Merge pull request watplugin#319 from RailKill/improve-test-detection

Improve test detection and include broken scripts in results

addons/WAT/filesystem/filesystem.gd

@@ -16,21 +16,22 @@ var changed: bool = false setget _set_filesystem_changed
 var built: bool = false setget ,_get_filesystem_built # CSharpScript
 var build_function: FuncRef
 var index = {} # Path / Object
+var test_validator: Reference
 
 # Initialize/Save meta
-
 func _init(_build_function = null) -> void:
 	build_function = _build_function
 	tagged = TaggedTests.new(Settings)
 	failed = FailedTests.new()
+	test_validator = Validator.new()
 
 func _set_filesystem_changed(has_changed: bool) -> void:
 	changed = has_changed
 	if has_changed or ClassDB.class_exists("CSharpScript"):
 		built = false
 
 func _get_filesystem_built() -> bool:
-	# If it is not Mono, automatically return true because it is irrelevant to GDScript.
+	# If not Mono, return true because it is irrelevant to GDScript.
 	return built or not Engine.is_editor_hint() or not ClassDB.class_exists("CSharpScript")
 
 func _recursive_update(testdir: TestDirectory) -> void:
@@ -55,11 +56,12 @@ func _recursive_update(testdir: TestDirectory) -> void:
 			index[sub_testdir.path] = sub_testdir
 			pass
 
-		elif dir.file_exists(absolute) and Validator.is_valid_test(absolute):
+		elif dir.file_exists(absolute):
 			var test_script: TestScript = _get_test_script(absolute)
-			testdir.tests.append(test_script)
-			test_script.dir = testdir.path
-			index[test_script.path] = test_script
+			if test_script:
+				testdir.tests.append(test_script)
+				test_script.dir = testdir.path
+				index[test_script.path] = test_script
 
 		relative = dir.get_next()
 
@@ -73,7 +75,7 @@ func _recursive_update(testdir: TestDirectory) -> void:
 
 func update(testdir: TestDirectory = _get_root()) -> void:
 	_recursive_update(testdir)
-	# The changed attribute should be set to false after the update otherwise it is redundant.
+	# Set "changed" to false after the update, otherwise it is redundant.
 	changed = false
 
 func _get_root() -> TestDirectory:
@@ -84,20 +86,26 @@ func _get_root() -> TestDirectory:
 	return root
 
 func _get_test_script(p: String) -> TestScript:
-	var test: Node = load(p).new()
-	var test_script: TestScript = TestScript.new()
-	test_script.path = p
-	test_script.names = test.get_test_methods()
-	for m in test_script.names:
-		var test_method: TestMethod = TestMethod.new()
-		test_method.path = p
-		test_method.name = m
-		test_script.methods.append(test_method)
-		index[test_script.path+m] = test_method
-	if p.ends_with(".gd") or p.ends_with(".gdc"):
-		test_script.time = YieldCalculator.calculate_yield_time(load(p), test_script.names.size())
-	test.free()
+	test_validator.load_path(p, changed)
+	var test_script: TestScript = null
+	if test_validator.is_valid_test():
+		test_script = TestScript.new(p, test_validator.get_load_error())
+		var script_instance = test_validator.script_instance
+		if script_instance:
+			test_script.names = script_instance.get_test_methods()
+			# Skip scripts with 0 defined test methods if validator allows.
+			if test_validator.skip_empty and test_script.names.empty():
+				return null
+			for m in test_script.names:
+				var test_method: TestMethod = TestMethod.new()
+				test_method.path = p
+				test_method.name = m
+				test_script.methods.append(test_method)
+				index[test_script.path+m] = test_method
+			if p.ends_with(".gd") or p.ends_with(".gdc"):
+				test_script.time = YieldCalculator.calculate_yield_time(
+						test_validator.script_resource, test_script.names.size())
 	return test_script
-	
+
 func clear() -> void:
 	index.clear()

addons/WAT/filesystem/method.gd

@@ -4,6 +4,10 @@ var path: String
 var dir: String setget ,_get_path
 var name: String setget ,_get_sanitized_name
 
+func _init(method_path: String = "", method_name: String = ""):
+	path = method_path
+	name = method_name
+
 # Method Name != test name
 func get_tests() -> Array:
 	return [{"dir": dir, "name": name, "path": self.path, "methods": [name], "time": 0.0}]

addons/WAT/filesystem/script.gd

@@ -6,6 +6,14 @@ var path: String setget ,_get_path
 var methods: Array # TestMethods
 var names: Array # MethodNames
 var time: float = 0.0 # YieldTime
+var parse: int # stores error code (int) of last load()
+
+# Constructor for script.gd reference.
+# script_path: Resource path of the script.
+# load_result: Error code when resource path is reloaded.
+func _init(script_path: String = "", load_result: int = OK):
+	path = script_path
+	parse = load_result
 
 func _get_sanitized_name() -> String:
 	var n: String = path.substr(path.find_last("/") + 1)
@@ -19,4 +27,4 @@ func _get_path() -> String:
 	return path.replace("///", "//") # 
 
 func get_tests() -> Array:
-	return [{"dir": dir, "name": self.name, "path": self.path, "methods": names, "time": time}]
+	return [{"dir": dir, "name": self.name, "path": self.path, "methods": names, "time": time, "parse": parse}]

addons/WAT/filesystem/validator.gd

@@ -1,23 +1,82 @@
 extends Reference
+
+# To search broken script source for WAT.Test usage.
+const WAT_TEST_PATTERN = "(\\bextends\\s+WAT.Test\\b)" + \
+	"|(\\bextends\\b\\s+\\\"res:\\/\\/addons\\/WAT\\/test\\/test.gd\\\")" + \
+	"|(class\\s\\w[\\w<>]+\\s*:\\s*WAT.Test[\\s\\{])"
+
+var path: String setget load_path
+var regex: RegEx
+var script_resource: Script setget ,get_script_resource
+var script_instance setget ,get_script_instance
+# If true, test scripts with 0 defined test methods should be skipped.
+var skip_empty: bool = true
+
+func _init(regex_pattern = WAT_TEST_PATTERN):
+	regex = RegEx.new()
+	regex.compile(regex_pattern)
+
+# Frees the script_instance on destroy.
+func _notification(what: int) -> void:
+	if what == NOTIFICATION_PREDELETE and is_instance_valid(script_instance):
+		# Cannot call _clear_resource() at predelete stage.
+		script_instance.free()
+
+# Clears the script resource and frees the script instance.
+func _clear_resource() -> void:
+	if is_instance_valid(script_instance):
+		script_instance.free()
+	script_instance = null
+	script_resource = null
+
+# Checks if the Script's source code is matching the compiled regex pattern.
+func _is_matching_regex_pattern() -> bool:
+	return true if script_resource and \
+			regex.search(script_resource.source_code) else false
+
+func _is_valid_gdscript() -> bool:
+	return path.ends_with(".gd") and path != "res://addons/WAT/test/test.gd"
 
-static func is_valid_test(p: String) -> bool:
-	return _is_valid_gdscript(p) or _is_valid_compiled_gdscript(p) or _is_valid_csharp(p)
-
-static func _is_valid_gdscript(p: String) -> bool:
-	return p.ends_with(".gd") and p != "res://addons/WAT/test/test.gd" and load(p).get("IS_WAT_TEST")
+func _is_valid_compiled_gdscript() -> bool:
+	return path.ends_with(".gdc") and path != "res://addons/WAT/test/test.gdc"
 
-static func _is_valid_compiled_gdscript(p: String) -> bool:
-	return p.ends_with(".gdc") and p != "res://addons/WAT/test/test.gdc" and load(p).get("IS_WAT_TEST")
-
-static func _is_valid_csharp(p: String) -> bool:
-	# TODO: This requires extra checking for invalid or uncompiled csharp scripts
-	# Any errors about no method or new function new found exists here
-	if p.ends_with(".cs") and not "addons/WAT" in p and load(p).has_method("new"):
-		var test = load(p).new()
-		if test.get("IS_WAT_TEST"):
-			test.free()
-			return true
-		else:
-			test.free()
-			return false
-	return false
+func _is_valid_csharp() -> bool:
+	return path.ends_with(".cs") and not "addons/WAT" in path
+
+func _is_valid_file() -> bool:
+	return _is_valid_gdscript() or _is_valid_compiled_gdscript() or \
+			(ClassDB.class_exists("CSharpScript") and _is_valid_csharp())
+
+func is_valid_test() -> bool:
+	return get_script_resource() and get_script_instance() or \
+			get_load_error() == ERR_PARSE_ERROR and _is_matching_regex_pattern()
+
+# Returns error code during resource load.
+func get_load_error() -> int:
+	var error = ERR_SKIP
+	if script_resource:
+		# Script resource with 0 methods signify parse error / uncompiled.
+		# Loaded scripts always have at least one method from its base class.
+		error = OK if not script_resource.get_script_method_list().empty() \
+				else ERR_PARSE_ERROR
+	return error
+
+func get_script_instance():
+	# Create script_instance if no errors are found. Performed once and stored.
+	if not script_instance and get_load_error() == OK and \
+			(script_resource.get("IS_WAT_TEST") if not _is_valid_csharp() \
+			else _is_matching_regex_pattern()):
+		script_instance = script_resource.new()
+	return script_instance
+
+func get_script_resource() -> Script:
+	return script_resource
+
+func load_path(resource_path: String, refresh: bool = true) -> void:
+	_clear_resource()
+	path = resource_path
+	if _is_valid_file():
+		# .cs scripts should load from cache due to how it is compiled.
+		# .gd scripts need to be reloaded on filesystem update.
+		script_resource = ResourceLoader.load(path, "Script",
+				refresh and not _is_valid_csharp())

addons/WAT/runner/test_controller.gd

@@ -12,27 +12,49 @@ signal results_received
 func run(metadata: Dictionary) -> void:
 	_dir = metadata["dir"]
 	_path = metadata["path"]
-	var methods = metadata["methods"]
-	_test = load(_path).new().setup(_dir, _path, methods)
+	var _methods = metadata["methods"]
 
-	# Signals leak in C# so we're just using a direct connection and GetParent Calls
-
-	_test.connect("test_method_started", self, "_on_test_method_started")
-	_test.connect("described", self, "_on_test_method_described")
-	_test.connect("asserted", self, "_on_asserted")
-	_test.connect("test_method_finished", self, "_on_test_method_finished")
-	_test.connect("test_script_finished", self, "_on_test_script_finished")
-	_on_test_script_started(metadata)
-	# We need to wait for the object itself to emit the signal (since we..
-	# ..cannot yield for C# so we defer the call to run so we have time to..
-	# ..to setup our yielding rather than deal with a race condition)
-	call_deferred("add_child", _test)
-	var results = yield(self, "results_received") # test_script_finished
-	_test.queue_free()
-	return results
+	# Default test results to -1 if there is a parse error, so the CLI or GUI will..
+	# ..know that an error had occurred.
+	var test_results = {
+		total = -1,
+		passed = -1,
+		context = metadata["name"],
+		methods = _methods,
+		success = false,
+		path = _path,
+		time_taken = metadata["time"],
+		dir = _dir
+	}
+
+	# If there was a parsing error, skip the test case.
+	if metadata.get("parse", OK) == OK:
+		# Signals leak in C# so we're just using a direct connection and GetParent Calls
+		_test = load(_path).new().setup(_dir, _path, _methods)
+		_test.connect("test_method_started", self, "_on_test_method_started")
+		_test.connect("described", self, "_on_test_method_described")
+		_test.connect("asserted", self, "_on_asserted")
+		_test.connect("test_method_finished", self, "_on_test_method_finished")
+		_test.connect("test_script_finished", self, "_on_test_script_finished")
+		_on_test_script_started(metadata) # Needs to be after _test is loaded.
+
+		# We need to wait for the object itself to emit the signal (since we..
+		# ..cannot yield for C# so we defer the call to run so we have time to..
+		# ..to setup our yielding rather than deal with a race condition)
+		call_deferred("add_child", _test)
+		test_results = yield(self, "results_received") # test_script_finished
+		_test.queue_free()
+		# Reset for the next case.
+		_test = null
+	else:
+		_on_test_script_started(metadata)
+		# Nothing to call_deferred here, so just yield for next idle_frame.
+		yield(get_tree(), "idle_frame")
+		_on_test_script_finished(test_results)
+	return test_results
 
 func _on_test_script_started(data: Dictionary) -> void:
-	data["title"] = _test.title()
+	data["title"] = _test.title() if is_instance_valid(_test) else data["name"]
 	if results != null:
 		results.on_test_script_started(data)
 

addons/WAT/ui/cli.gd

@@ -110,7 +110,7 @@ func _threads() -> int:
 
 
 func _display(cases: Dictionary) -> void:
-	cases.seconds = OS.get_ticks_msec() / 1000
+	cases.seconds = stepify(OS.get_ticks_msec() / 1000.0, 0.001)
 	_time_taken = cases.seconds
 	print("""
 	-------RESULTS-------
@@ -123,14 +123,21 @@ func _display(cases: Dictionary) -> void:
 func _display_failures(case) -> void:
 	# We could create this somewhere else?
 	print("%s (%s)" % [case.context, case.path])
-	for method in case.methods:
-		if not method.success:
-			print("\n  %s" % method.context)
-			for assertion in method.assertions:
-				if not assertion.success:
-					print("\t%s" % assertion.context, "\n\t  (EXPECTED: %s) | (RESULTED: %s)" % [assertion.expected, assertion.actual])
+	match case.total:
+		-1:
+			print("\n  Parse Error (check syntax or broken dependencies)")
+		0:
+			print("\n  No Test Methods Defined")
+		_:
+			for method in case.methods:
+				if not method.success:
+					print("\n  %s" % method.context)
+					for assertion in method.assertions:
+						if not assertion.success:
+							print("\t%s" % assertion.context, 
+								"\n\t  (EXPECTED: %s) | (RESULTED: %s)" % \
+								[assertion.expected, assertion.actual])
 
 func _quit() -> void:
 	_filesystem.clear()
 	get_tree().quit()
-