Make test runner return a TestResult, rather than Optional

Author: JustinMeimar

dragon_runner/harness.py

@@ -15,13 +15,6 @@ def __init__(self, config: Config, cli_args: CLIArgs):
         self.cli_args: CLIArgs = cli_args
         self.failures: List[TestResult] = []
 
-    def post_run_log(self):
-        """Report failures to stdout."""
-        if self.failures != []:
-            log(f"Failure Summary: ({len(self.failures)} tests)") 
-            for result in self.failures:
-                result.log()
-
     def process_test_result(self, test_result: Optional[TestResult], counters: Dict[str, int]):
         """
         Process each test result.
@@ -43,7 +36,10 @@ def pre_executable_hook(self):
 
     def post_executable_hook(self):
         """Hook to run after iterating through an executable"""
-        pass
+        if self.failures != []:
+            log(f"Failure Summary: ({len(self.failures)} tests)") 
+            for result in self.failures:
+                result.log()
 
     def iterate(self):
         """
@@ -70,7 +66,7 @@ def iterate(self):
                         counters = {"pass_count": 0, "test_count": 0}
                         self.pre_subpackage_hook(spkg)
                         for test in spkg.tests:
-                            test_result: Optional[TestResult] = tc_runner.run(test, exe)
+                            test_result: TestResult = tc_runner.run(test, exe)
                             self.process_test_result(test_result, counters)
                         self.post_subpackage_hook(counters)
                         log("Subpackage Passed: ", counters["pass_count"], "/", counters["test_count"], indent=3)
@@ -100,9 +96,7 @@ def process_test_result(self, test_result: Optional[TestResult], counters: Dict[
         """
         Override the hook for regular run-specific implementation of counting passes
         """
-        if not test_result:
-            log("[ERROR] Failed to receive test result", indent=7)
-        elif test_result.did_pass:
+        if test_result.did_pass:
             counters["pass_count"] += 1
             test_result.log(args=self.cli_args)
         else:
@@ -126,10 +120,7 @@ def process_test_result(self,
         toolchain_name = context["toolchain_name"]
         a_pkg_name = context["a_pkg_name"]
 
-        if not test_result:
-            log(f"Failed to run test {test.stem}")
-            context["exit_status"] = False
-        elif test_result.did_pass:
+        if test_result.did_pass:
             print(Fore.GREEN + '.' + Fore.RESET, end='')
             counters['pass_count'] += 1
         else:
@@ -251,6 +242,11 @@ def post_executable_hook(self):
                 indent=4)
         self.leak_tests = []
         self.test_count = 0 # reset for each executable
+        
+        if self.failures != []:
+            log(f"Failure Summary: ({len(self.failures)} tests)") 
+            for result in self.failures:
+                result.log()
 
     def process_test_result(self, test_result: Optional[TestResult], counters: Dict[str, int]):
         """
@@ -286,9 +282,7 @@ def process_test_result(self, test_result: Optional[TestResult], counters: Dict[
         """
         Override the hook for regular run-specific implementation of counting passes
         """
-        if not test_result:
-            log("Failed to receive test result")
-        elif test_result.did_pass:
+        if test_result.did_pass:
             counters["pass_count"] += 1
             test_result.log(args=self.cli_args)
         else:

dragon_runner/main.py

@@ -14,7 +14,7 @@ def main():
     args: CLIArgs = parse_cli_args()
     log(args, level=1)
 
-    # dragon-runner can also be used as a frontend for grading scripts
+    # dragon-runner can also be used as a loader for grading & other scripts
     if args.is_script_mode():
         print(f"Use dragon-runner as a loader for script: {args.mode}")
         loader = Loader(args.mode, args.script_args)

dragon_runner/runner.py

@@ -69,10 +69,10 @@ class TestResult:
     execution time, and error information.
     """
     __test__ = False  # pytest gets confused when classes start with 'Test' 
-    def __init__(self, test:TestFile): 
+    def __init__(self, test:TestFile, did_pass:bool=False): 
         # required fields 
         self.test = test
-        self.did_pass: bool = False
+        self.did_pass: bool = did_pass
         self.did_timeout: bool = False 
         self.error_test: bool = False
         self.memory_leak: bool = False
@@ -186,13 +186,13 @@ def resolve_command(self, step: Step, params: MagicParams) -> Command:
             command.args[0] = os.path.abspath(exe)
         return command
 
-    def run(self, test: TestFile, exe: Executable) -> Optional[TestResult]: 
+    def run(self, test: TestFile, exe: Executable) -> TestResult: 
         """
         run each step of the toolchain for a given test and executable
         """
         input_file = test.path
         expected = test.expected_out if isinstance(test.expected_out, bytes) else b'' 
-        tr = TestResult(test=test)
+        tr = TestResult(test=test, did_pass=False)
 
         for index, step in enumerate(self.tc):
 
@@ -241,7 +241,7 @@ def run(self, test: TestFile, exe: Executable) -> Optional[TestResult]:
                     tr.memory_leak = True 
 
             if child_process.returncode != 0 and \
-                child_process.returncode not in self.reserved_exit_codes:
+               child_process.returncode not in self.reserved_exit_codes:
                 """
                 A step in the toolchain has returned a non-zero exit status. If "allowError"
                 is specified in the config, we can perform a lenient diff based on CompileTime
@@ -296,6 +296,9 @@ def run(self, test: TestFile, exe: Executable) -> Optional[TestResult]:
                 If $OUTPUT is not supplied, we create a temporary pipe.
                 """
                 input_file = output_file or make_tmp_file(child_process.stdout)
+        
+        # this code should be unreachable for well-defined toolchains 
+        return tr
 
     @staticmethod
     def replace_env_vars(cmd: Command) -> Command:

tests/configs/ValgrindConfig.json

@@ -1,7 +1,8 @@
 {
   "testDir": "../packages/MemoryLeaks",
   "testedExecutablePaths": {
-    "clang": "/usr/bin/clang"
+    "clang": "/usr/bin/clang",
+    "clang2": "/usr/bin/clang"
   },
   "toolchains": {
     "LLVM": [