[feat] Tweak verbosity levels of compile error output (#66)

repobee_junit4/_junit4_runner.py

@@ -5,12 +5,10 @@
 import subprocess
 import os
 import contextlib
-from typing import Tuple, Optional
+from typing import Optional
 
 import daiquiri
 
-from repobee_plug import Status
-
 from repobee_junit4 import _java
 from repobee_junit4 import _output
 

repobee_junit4/_output.py

@@ -8,21 +8,20 @@
 import sys
 import os
 import re
-import subprocess
 import collections
 
-from typing import Tuple
+from colored import bg, style
 
 from repobee_plug import Status
 
 from repobee_junit4 import _java
 
-from colored import bg, style
-
-
 SUCCESS_COLOR = bg("dark_green")
 FAILURE_COLOR = bg("yellow")
 
+DEFAULT_LINE_LENGTH_LIMIT = 150
+DEFAULT_MAX_LINES = 10
+
 
 class TestResult(
     collections.namedtuple("TestResult", "test_class proc".split())
@@ -72,7 +71,9 @@ def _get_num_failed(test_output: bytes) -> int:
 
 
 def _get_num_tests(test_output: bytes) -> int:
-    """Get the total amount of tests. Only use this if there were test failures!"""
+    """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
@@ -101,6 +102,65 @@ def test_result_header(
     """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,
+        title_color, test_class_name, style.RESET, test_results
     )
     return msg
+
+
+def format_results(test_results, compile_failed, verbose, very_verbose):
+    def format_compile_error(res):
+        msg = "{}Compile error:{} {}".format(bg("red"), style.RESET, res.msg)
+        if very_verbose:
+            return msg
+        elif verbose:
+            return _truncate_lines(msg)
+        else:
+            return msg.split("\n")[0]
+
+    def format_test_result(res):
+        msg = res.pretty_result(verbose or very_verbose)
+        if very_verbose:
+            return msg
+        elif verbose:
+            return _truncate_lines(msg, max_lines=sys.maxsize)
+        else:
+            return msg.split("\n")[0]
+
+    compile_error_messages = list(map(format_compile_error, compile_failed))
+    test_messages = list(map(format_test_result, test_results))
+    msg = os.linesep.join(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 _truncate_lines(
+    string: str,
+    max_len: int = DEFAULT_LINE_LENGTH_LIMIT,
+    max_lines: int = DEFAULT_MAX_LINES,
+):
+    """Truncate lines to max_len characters."""
+    trunc_msg = " #[...]# "
+    if max_len <= len(trunc_msg):
+        raise ValueError(
+            "max_len must be greater than {}".format(len(trunc_msg))
+        )
+
+    effective_len = max_len - len(trunc_msg)
+    head_len = effective_len // 2
+    tail_len = effective_len // 2
+
+    def truncate(s):
+        if len(s) > max_len:
+            return s[:head_len] + trunc_msg + s[-tail_len:]
+        return s
+
+    lines = [truncate(line) for line in string.split(os.linesep)]
+    if len(lines) > max_lines:
+        lines = lines[: max_lines - 1] + [trunc_msg]
+    return os.linesep.join(lines)

repobee_junit4/junit4.py

@@ -16,17 +16,14 @@
 
 .. moduleauthor:: Simon Larsén
 """
-import itertools
 import os
 import argparse
 import configparser
 import pathlib
-import collections
-from typing import Union, Iterable, Tuple, List, Any
+from typing import Union, Tuple, List
 
 
 import daiquiri
-from colored import bg, style
 
 import repobee_plug as plug
 from repobee_plug import Status
@@ -41,8 +38,6 @@
 
 ResultPair = Tuple[pathlib.Path, pathlib.Path]
 
-DEFAULT_LINE_LIMIT = 150
-
 
 class JUnit4Hooks(plug.Plugin):
     def __init__(self):
@@ -94,9 +89,15 @@ def act_on_cloned_repo(
                 map(lambda r: not r.success, test_results)
             )
 
-            msg = self._format_results(test_results, compile_failed)
+            msg = _output.format_results(
+                test_results, compile_failed, self._verbose, self._very_verbose
+            )
 
-            status = Status.ERROR if compile_failed else (Status.WARNING if has_failures else Status.SUCCESS)
+            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,33 +322,6 @@ def _find_test_classes(self, master_name) -> List[pathlib.Path]:
 
         return test_classes
 
-    def _format_results(self, test_results, compile_failed):
-
-        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
-        ]
-
-        msg = os.linesep.join(
-            [
-                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
     ) -> _output.TestResult:
@@ -366,7 +340,6 @@ def _run_tests(
             classpath, active=not self._disable_security
         ) as security_policy:
             for test_class, prod_class in test_prod_class_pairs:
-                test_class_name = _java.fqn_from_file(test_class)
                 test_result = _junit4_runner.run_test_class(
                     test_class,
                     prod_class,
@@ -415,9 +388,8 @@ def _check_jars_exist(self):
         for raw_path in (junit_path, hamcrest_path):
             if not pathlib.Path(raw_path).is_file():
                 raise plug.PlugError(
-                    "{} is not a file, please check the filepath you specified".format(
-                        raw_path
-                    )
+                    "{} is not a file, please check the filepath you "
+                    "specified".format(raw_path)
                 )
 
     def _parse_from_classpath(self, filename: str) -> pathlib.Path:
@@ -437,25 +409,3 @@ def _parse_from_classpath(self, filename: str) -> pathlib.Path:
                 )
             )
         return matches[0] if matches else None
-
-
-def _truncate_lines(string: str, max_len: int = DEFAULT_LINE_LIMIT):
-    """Truncate lines to max_len characters."""
-    trunc_msg = " #[...]# "
-    if max_len <= len(trunc_msg):
-        raise ValueError(
-            "max_len must be greater than {}".format(len(trunc_msg))
-        )
-
-    effective_len = max_len - len(trunc_msg)
-    head_len = effective_len // 2
-    tail_len = effective_len // 2
-
-    def truncate(s):
-        if len(s) > max_len:
-            return s[:head_len] + trunc_msg + s[-tail_len:]
-        return s
-
-    return os.linesep.join(
-        [truncate(line) for line in string.split(os.linesep)]
-    )

tests/integration_tests/repos/compile-error-week-10/src/BadClass.java

@@ -1,5 +1,5 @@
 public class BadClass {
-    for (int i = 0; i < 10; i++) {
+    for (int i = 0; i < 10; i++, i--) {
         System.out.println(i);
     }
 }

tests/integration_tests/test_end_to_end.py

@@ -170,7 +170,9 @@ def test_handles_missing_student_tests(self):
         )
 
     def test_converts_generic_exception_to_hook_result(self, default_hooks):
-        """Test that a generic Exception raised during execution is converted to a hook result."""
+        """Test that a generic Exception raised during execution is converted
+        to a hook result.
+        """
         msg = "Some error message"
 
         def _raise_exception(*args, **kwargs):
@@ -232,10 +234,13 @@ def test_fail_repo(self, default_hooks):
             in result.msg
         )
 
-    def test_fail_repo_verbose(self):
+    @pytest.mark.parametrize(
+        "hooks",
+        [setup_hooks(verbose=True), setup_hooks(very_verbose=True)],
+        ids=["verbose", "very_verbose"],
+    )
+    def test_fail_repo_verbose(self, hooks):
         """Test verbose output on repo that fails tests."""
-        hooks = setup_hooks(verbose=True)
-
         expected_verbose_msg = """1) isPrimeFalseForComposites(PrimeCheckerTest)
 java.lang.AssertionError: 
 Expected: is <false>
@@ -318,7 +323,47 @@ def test_error_result_on_compile_error(self, default_hooks):
         result = default_hooks.act_on_cloned_repo(str(COMPILE_ERROR_REPO))
 
         assert result.status == Status.ERROR
-        assert "error: illegal start of type" in result.msg
+        assert "Compile error" in result.msg
+        assert len(result.msg.split("\n")) == 1
+
+    def test_amount_of_lines_in_compile_error_is_truncated_in_verbose_mode(
+        self,
+    ):
+        hooks = setup_hooks(verbose=True)
+
+        result = hooks.act_on_cloned_repo(str(COMPILE_ERROR_REPO))
+
+        assert result.status == Status.ERROR
+        assert len(result.msg.split(os.linesep)) == _output.DEFAULT_MAX_LINES
+
+    def test_full_compile_error_shown_in_very_verbose_mode(self):
+        hooks = setup_hooks(very_verbose=True)
+        expected_error_msg_lines = """
+BadClass.java:2: error: illegal start of type
+    for (int i = 0; i < 10; i++, i--) {
+    ^
+BadClass.java:2: error: illegal start of type
+    for (int i = 0; i < 10; i++, i--) {
+                        ^
+BadClass.java:2: error: <identifier> expected
+    for (int i = 0; i < 10; i++, i--) {
+                             ^
+BadClass.java:2: error: <identifier> expected
+    for (int i = 0; i < 10; i++, i--) {
+                                  ^
+4 errors
+""".strip().split(
+            "\n"
+        )
+
+        result = hooks.act_on_cloned_repo(str(COMPILE_ERROR_REPO))
+
+        result_lines = result.msg.strip().split("\n")
+        assert result.status == Status.ERROR
+        # the absolute path to BadClass will differ depending on the test
+        # environment so asserting the following is about as good as it gets
+        assert len(result_lines) == len(expected_error_msg_lines)
+        assert result_lines[-1] == expected_error_msg_lines[-1]
 
     def test_runs_correctly_when_paths_include_whitespace(self, default_hooks):
         result = default_hooks.act_on_cloned_repo(DIR_PATHS_WITH_SPACES)
@@ -410,8 +455,8 @@ def test_verbose_output_is_truncated(self, monkeypatch):
         hooks = setup_hooks(verbose=True)
         line_length = 20
         monkeypatch.setattr(
-            "repobee_junit4.junit4._truncate_lines",
-            partial(junit4._truncate_lines, max_len=line_length),
+            "repobee_junit4._output._truncate_lines",
+            partial(_output._truncate_lines, max_len=line_length),
         )
 
         result = hooks.act_on_cloned_repo(FAIL_REPO)
@@ -429,8 +474,8 @@ def test_very_verbose_output_not_truncated(self, monkeypatch):
         hooks = setup_hooks(very_verbose=True)
         line_length = 20
         monkeypatch.setattr(
-            "repobee_junit4.junit4._truncate_lines",
-            partial(junit4._truncate_lines, max_len=line_length),
+            "repobee_junit4._output._truncate_lines",
+            partial(_output._truncate_lines, max_len=line_length),
         )
 
         result = hooks.act_on_cloned_repo(FAIL_REPO)

tests/unit_tests/test_junit4.py

@@ -186,7 +186,6 @@ def test_defaults_are_kept_if_not_specified_in_args(
         expected_junit_path = JUNIT_PATH
         expected_rtd = RTD
         expected_disable_security = False
-        expected_run_student_tests = False
 
         junit4_hooks._ignore_tests = expected_ignore_tests
         junit4_hooks._hamcrest_path = expected_hamcrest_path
@@ -238,7 +237,9 @@ def test_raises_if_classpath_junit_jar_does_not_exist(self, junit4_hooks):
         """
         junit_path = "/no/jar/on/this/classpath/" + _junit4_runner.JUNIT_JAR
         junit4_hooks._hamcrest_path = HAMCREST_PATH
-        junit4_hooks._classpath = os.pathsep.join(["/garbage/path/", junit_path, HAMCREST_PATH])
+        junit4_hooks._classpath = os.pathsep.join(
+            ["/garbage/path/", junit_path, HAMCREST_PATH]
+        )
         args = Args()
 
         with pytest.raises(plug.PlugError) as exc_info: