[feat] Completely redesign test output (#63)

repobee_junit4/_java.py

@@ -60,6 +60,19 @@ def generate_classpath(*paths: pathlib.Path, classpath: str = "") -> str:
     return classpath
 
 
+def fqn_from_file(java_filepath: pathlib.Path) -> str:
+    """Extract the expected fully qualified class name for the given java file.
+
+    Args:
+        java_filepath: Path to a .java file.
+    """
+    if not java_filepath.suffix == ".java":
+        raise ValueError("{} not a path to a .java file".format(java_filepath))
+    package = extract_package(java_filepath)
+    simple_name = java_filepath.name[:-len(java_filepath.suffix)]
+    return fqn(package, simple_name)
+
+
 def extract_package(class_: pathlib.Path) -> str:
     """Return the name package of the class. An empty string
     denotes the default package.

repobee_junit4/_junit4_runner.py

@@ -12,6 +12,7 @@
 from repobee_plug import Status
 
 from repobee_junit4 import _java
+from repobee_junit4 import _output
 
 
 LOGGER = daiquiri.getLogger(__file__)
@@ -66,22 +67,6 @@ def _generate_default_security_policy(classpath: str) -> str:
     return _DEFAULT_SECURITY_POLICY_TEMPLATE.format(junit4_jar_path=path)
 
 
-def get_num_failed(test_output: bytes) -> int:
-    """Get the amount of failed tests from the error output of JUnit4."""
-    decoded = test_output.decode(encoding=sys.getdefaultencoding())
-    match = re.search(r"Failures: (\d+)", decoded)
-    # TODO this is a bit unsafe, what if there is no match?
-    return int(match.group(1))
-
-
-def parse_failed_tests(test_output: bytes) -> str:
-    """Return a list of test failure descriptions, excluding stack traces."""
-    decoded = test_output.decode(encoding=sys.getdefaultencoding())
-    return re.findall(
-        r"^\d\) .*(?:\n(?!\s+at).*)*", decoded, flags=re.MULTILINE
-    )
-
-
 def _extract_conforming_package(test_class, prod_class):
     """Extract a package name from the test and production class.
     Raise if the test class and production class have different package
@@ -103,28 +88,23 @@ def run_test_class(
     test_class: pathlib.Path,
     prod_class: pathlib.Path,
     classpath: str,
-    verbose: bool = False,
     security_policy: Optional[pathlib.Path] = None,
-) -> Tuple[str, str]:
+) -> _output.TestResult:
     """Run a single test class on a single production class.
 
     Args:
         test_class: Path to a Java test class.
         prod_class: Path to a Java production class.
         classpath: A classpath to use in the tests.
-        verbose: Whether to output more failure information.
     Returns:
-        ()
+        The completed process.
     """
     package = _extract_conforming_package(test_class, prod_class)
 
     prod_class_dir = _java.extract_package_root(prod_class, package)
     test_class_dir = _java.extract_package_root(test_class, package)
 
-    test_class_name = test_class.name[
-        : -len(test_class.suffix)
-    ]  # remove .java
-    test_class_name = _java.fqn(package, test_class_name)
+    test_class_name = _java.fqn_from_file(test_class)
 
     classpath = _java.generate_classpath(
         test_class_dir, prod_class_dir, classpath=classpath
@@ -147,23 +127,4 @@ def run_test_class(
         command, stdout=subprocess.PIPE, stderr=subprocess.PIPE
     )
 
-    return _extract_results(proc, test_class_name, verbose)
-
-
-def _extract_results(
-    proc: subprocess.CompletedProcess, test_class_name: str, verbose: bool
-) -> Tuple[str, str]:
-    """Extract and format results from a completed test run."""
-    if proc.returncode != 0:
-        status = Status.ERROR
-        msg = "Test class {} failed {} tests".format(
-            test_class_name, get_num_failed(proc.stdout)
-        )
-        if verbose:
-            msg += os.linesep + os.linesep.join(
-                parse_failed_tests(proc.stdout)
-            )
-    else:
-        msg = "Test class {} passed!".format(test_class_name)
-        status = Status.SUCCESS
-    return status, msg
+    return _output.TestResult(test_class=test_class, proc=proc)

repobee_junit4/_output.py

@@ -0,0 +1,106 @@
+"""Utility methods for processing and formatting output.
+
+.. module:: _output
+    :synopsis: Plugin that tries to compile all .java files in a repo.
+
+.. moduleauthor:: Simon Larsén
+"""
+import sys
+import os
+import re
+import subprocess
+import collections
+
+from typing import Tuple
+
+from repobee_plug import Status
+
+from repobee_junit4 import _java
+
+from colored import bg, style
+
+
+SUCCESS_COLOR = bg("dark_green")
+FAILURE_COLOR = bg("yellow")
+
+
+class TestResult(
+    collections.namedtuple("TestResult", "test_class proc".split())
+):
+    @property
+    def fqn(self):
+        return _java.fqn_from_file(self.test_class)
+
+    @property
+    def success(self):
+        return self.proc.returncode == 0
+
+    @property
+    def status(self):
+        return Status.SUCCESS if self.success else Status.ERROR
+
+    @property
+    def num_failed(self):
+        return _get_num_failed(self.proc.stdout)
+
+    @property
+    def num_passed(self):
+        return _get_num_passed(self.proc.stdout)
+
+    @property
+    def test_failures(self):
+        return _parse_failed_tests(self.proc.stdout)
+
+    def pretty_result(self, verbose: bool) -> str:
+        """Format this test as a pretty-printed message."""
+        title_color = SUCCESS_COLOR if self.success else FAILURE_COLOR
+        num_passed = self.num_passed
+        num_failed = self.num_failed
+        msg = test_result_header(
+            self.fqn, num_passed + num_failed, num_passed, title_color
+        )
+        if not self.success and verbose:
+            msg += os.linesep + os.linesep.join(self.test_failures)
+        return msg
+
+
+def _get_num_failed(test_output: bytes) -> int:
+    """Get the amount of failed tests from the error output of JUnit4."""
+    decoded = test_output.decode(encoding=sys.getdefaultencoding())
+    match = re.search(r"Failures: (\d+)", decoded)
+    return int(match.group(1)) if match else 0
+
+
+def _get_num_tests(test_output: bytes) -> int:
+    """Get the total amount of tests. Only use this if there were test failures!"""
+    decoded = test_output.decode(encoding=sys.getdefaultencoding())
+    match = re.search(r"Tests run: (\d+)", decoded)
+    return int(match.group(1)) if match else 0
+
+
+def _get_num_passed(test_output: bytes) -> int:
+    """Get the amount of passed tests from the output of JUnit4."""
+    decoded = test_output.decode(encoding=sys.getdefaultencoding())
+    match = re.search(r"OK \((\d+) tests\)", decoded)
+    if not match:  # there were failures
+        return _get_num_tests(test_output) - _get_num_failed(test_output)
+    return int(match.group(1))
+
+
+def _parse_failed_tests(test_output: bytes) -> str:
+    """Return a list of test failure descriptions, excluding stack traces."""
+    decoded = test_output.decode(encoding=sys.getdefaultencoding())
+    return re.findall(
+        r"^\d\) .*(?:\n(?!\s+at).*)*", decoded, flags=re.MULTILINE
+    )
+
+
+def test_result_header(
+    test_class_name: str, num_tests: int, num_passed: int, title_color: bg
+) -> str:
+    """Return the header line for a test result."""
+    test_results = "Passed {}/{} tests".format(num_passed, num_tests)
+    msg = "{}{}{}: {}".format(
+        title_color, test_class_name, style.RESET, test_results,
+    )
+    return msg

repobee_junit4/junit4.py

@@ -10,8 +10,9 @@
 pre-specified reference tests on production classes that are dynamically
 discovered in student repositories. See the README for more details.
 
-.. module:: javac
-    :synopsis: Plugin that tries to compile all .java files in a repo.
+.. module:: junit4
+    :synopsis: Plugin that runts JUnit4 test classes on students' production
+        code.
 
 .. moduleauthor:: Simon Larsén
 """
@@ -33,6 +34,7 @@
 from repobee_junit4 import _java
 from repobee_junit4 import _junit4_runner
 from repobee_junit4 import _exception
+from repobee_junit4 import _output
 from repobee_junit4 import SECTION
 
 LOGGER = daiquiri.getLogger(__file__)
@@ -86,17 +88,15 @@ def act_on_cloned_repo(
                 )
 
             compile_succeeded, compile_failed = self._compile_all(path)
-            tests_succeeded, tests_failed = self._run_tests(compile_succeeded)
+            test_results = self._run_tests(compile_succeeded)
 
-            msg = self._format_results(
-                itertools.chain(tests_succeeded, tests_failed, compile_failed)
+            has_failures = compile_failed or any(
+                map(lambda r: not r.success, test_results)
             )
 
-            status = (
-                Status.ERROR
-                if tests_failed or compile_failed
-                else Status.SUCCESS
-            )
+            msg = self._format_results(test_results, compile_failed)
+
+            status = Status.ERROR if compile_failed else (Status.WARNING if has_failures else Status.SUCCESS)
             return plug.HookResult(SECTION, status, msg)
         except _exception.ActError as exc:
             return exc.hook_result
@@ -321,67 +321,60 @@ def _find_test_classes(self, master_name) -> List[pathlib.Path]:
 
         return test_classes
 
-    def _format_results(self, hook_results: Iterable[plug.HookResult]):
-        """Format a list of plug.HookResult tuples as a nice string.
+    def _format_results(self, test_results, compile_failed):
 
-        Args:
-            hook_results: A list of plug.HookResult tuples.
-        Returns:
-            a formatted string
-        """
-        backgrounds = {
-            Status.ERROR: bg("red"),
-            Status.WARNING: bg("yellow"),
-            Status.SUCCESS: bg("dark_green"),
-        }
-
-        def test_result_string(status, msg):
-            return "{}{}:{} {}".format(
-                backgrounds[status],
-                status,
-                style.RESET,
-                _truncate_lines(msg) if self._verbose else msg,
-            )
+        compile_error_messages = [
+            "{}Compile error:{} {}".format(bg("red"), style.RESET, res.msg)
+            for res in compile_failed
+        ]
+        test_messages = [
+            res.pretty_result(self._verbose or self._very_verbose)
+            for res in test_results
+        ]
 
-        return os.linesep.join(
+        msg = os.linesep.join(
             [
-                test_result_string(status, msg)
-                for _, status, msg, _ in hook_results
+                msg if self._very_verbose else _truncate_lines(msg)
+                for msg in compile_error_messages + test_messages
             ]
         )
+        if test_messages:
+            num_passed = sum([res.num_passed for res in test_results])
+            num_failed = sum([res.num_failed for res in test_results])
+            total = num_passed + num_failed
+            msg = (
+                "Passed {}/{} tests{}".format(num_passed, total, os.linesep)
+                + msg
+            )
+        return msg
 
     def _run_tests(
         self, test_prod_class_pairs: ResultPair
-    ) -> Tuple[List[plug.HookResult], List[plug.HookResult]]:
+    ) -> _output.TestResult:
         """Run tests and return the results.
 
         Args:
             test_prod_class_pairs: A list of tuples on the form
             ``(test_class_path, prod_class_path)``
 
         Returns:
-            A tuple of lists ``(succeeded, failed)`` containing HookResult
-            tuples.
+            A TestResult for each test class run.
         """
-        succeeded = []
-        failed = []
+        results = []
         classpath = self._generate_classpath()
         with _junit4_runner.security_policy(
             classpath, active=not self._disable_security
         ) as security_policy:
             for test_class, prod_class in test_prod_class_pairs:
-                status, msg = _junit4_runner.run_test_class(
+                test_class_name = _java.fqn_from_file(test_class)
+                test_result = _junit4_runner.run_test_class(
                     test_class,
                     prod_class,
                     classpath=classpath,
-                    verbose=self._verbose or self._very_verbose,
                     security_policy=security_policy,
                 )
-                if status != Status.SUCCESS:
-                    failed.append(plug.HookResult(SECTION, status, msg))
-                else:
-                    succeeded.append(plug.HookResult(SECTION, status, msg))
-            return succeeded, failed
+                results.append(test_result)
+            return results
 
     def _generate_classpath(self, *paths: pathlib.Path) -> str:
         """