[feat] Add option to run student test classes instead of reference te…

…sts (#57)

repobee_junit4/_exception.py

@@ -0,0 +1,26 @@
+"""Exceptions for the junit4 plugin.
+
+.. module:: _exception
+    :synopsis: Exceptions for the junit4 plugin.
+
+.. moduleauthor:: Simon Larsén
+"""
+
+import repobee_plug as plug
+
+from repobee_junit4 import SECTION
+
+
+class ActError(plug.PlugError):
+    """Raise if something goes wrong in act_on_clone_repo."""
+
+    def __init__(self, hook_result):
+        self.hook_result = hook_result
+
+
+class JavaError(ActError):
+    """Raise if something goes wrong with Java files."""
+
+    def __init__(self, msg):
+        res = plug.HookResult(hook=SECTION, status=plug.Status.ERROR, msg=msg)
+        super().__init__(res)

repobee_junit4/_java.py

@@ -14,13 +14,15 @@
 import os
 import sys
 import subprocess
+import collections
 
 from typing import Iterable, Tuple, Union, List
 
 import repobee_plug as plug
 from repobee_plug import Status
 
 from repobee_junit4 import SECTION
+from repobee_junit4 import _exception
 
 
 def is_abstract_class(class_: pathlib.Path) -> bool:
@@ -187,6 +189,71 @@ def pairwise_compile(
     return succeeded, failed
 
 
+def get_student_test_classes(
+    path: pathlib.Path, reference_test_classes: List[pathlib.Path]
+) -> List[pathlib.Path]:
+    """Return paths to all files that match the test classes in the
+    provided list. Raises if there is more than one or no matches for any
+    of the files.
+
+    Args:
+        path: Path to the repository worktree.
+        reference_test_classes: A list of paths to reference test classes.
+            These are assumed to be unique.
+    Returns:
+        A list of paths to test classes corresponding to the ones in the input
+        list, but in the student repository.
+    """
+    filenames = {f.name for f in reference_test_classes}
+    matches = [file for file in path.rglob("*") if file.name in filenames]
+    _check_exact_matches(reference_test_classes, matches)
+    return matches
+
+
+def _check_exact_matches(
+    reference_test_classes: List[pathlib.Path],
+    student_test_classes: List[pathlib.Path],
+) -> None:
+    """Check that for every path in reference_test_classes, there is a path in
+    student_test_classes with the same filename and the same package.
+    """
+
+    def by_fqn(path):
+        pkg = extract_package(path)
+        return fqn(pkg, path.name)
+
+    duplicates = _extract_duplicates(student_test_classes)
+    if duplicates:
+        raise _exception.JavaError(
+            "Duplicates of the following test classes found in student repo: "
+            + ", ".join(duplicates)
+        )
+    if len(student_test_classes) < len(reference_test_classes):
+        reference_filenames = {f.name for f in reference_test_classes}
+        student_filenames = {f.name for f in student_test_classes}
+        raise _exception.JavaError(
+            "Missing the following test classes in student repo: "
+            + ", ".join(reference_filenames - student_filenames)
+        )
+    package_mismatch = []
+    for ref, match in zip(
+        sorted(reference_test_classes, key=by_fqn),
+        sorted(student_test_classes, key=by_fqn),
+    ):
+        expected_package = extract_package(ref)
+        actual_package = extract_package(match)
+        if actual_package != expected_package:
+            package_mismatch.append((ref, expected_package, actual_package))
+    if package_mismatch:
+        errors = ", ".join(
+            "Student's {} expected to have package {}, but had {}".format(
+                ref.name, expected, actual
+            )
+            for ref, expected, actual in package_mismatch
+        )
+        raise _exception.JavaError("Package statement mismatch: " + errors)
+
+
 def _pairwise_compile(test_class, classpath, java_files):
     """Compile the given test class together with its production class
     counterpoint (if it can be found). Return a tuple of (status, msg).
@@ -217,6 +284,11 @@ def _pairwise_compile(test_class, classpath, java_files):
     return status, msg, prod_class_path
 
 
+def _extract_duplicates(files: List[pathlib.Path]) -> List[pathlib.Path]:
+    counts = collections.Counter([f.name for f in files])
+    return [path for path, count in counts.items() if count > 1]
+
+
 def _get_matching_prod_classes(test_class, package, java_files):
     """Find all production classes among the Java files that math the test
     classes name and the package.

repobee_junit4/junit4.py

@@ -20,7 +20,9 @@
 import argparse
 import configparser
 import pathlib
-from typing import Union, Iterable, Tuple, List
+import collections
+from typing import Union, Iterable, Tuple, List, Any
+
 
 import daiquiri
 from colored import bg, style
@@ -30,6 +32,7 @@
 
 from repobee_junit4 import _java
 from repobee_junit4 import _junit4_runner
+from repobee_junit4 import _exception
 from repobee_junit4 import SECTION
 
 LOGGER = daiquiri.getLogger(__file__)
@@ -39,13 +42,6 @@
 DEFAULT_LINE_LIMIT = 150
 
 
-class _ActException(Exception):
-    """Raise if something goes wrong in act_on_clone_repo."""
-
-    def __init__(self, hook_result):
-        self.hook_result = hook_result
-
-
 class JUnit4Hooks(plug.Plugin):
     def __init__(self):
         self._master_repo_names = []
@@ -57,6 +53,7 @@ def __init__(self):
         self._verbose = False
         self._very_verbose = False
         self._disable_security = False
+        self._run_student_tests = False
 
     def act_on_cloned_repo(
         self, path: Union[str, pathlib.Path]
@@ -101,7 +98,8 @@ def act_on_cloned_repo(
                 else Status.SUCCESS
             )
             return plug.HookResult(SECTION, status, msg)
-        except _ActException as exc:
+        except _exception.ActError as exc:
+            print(exc)
             return exc.hook_result
         except Exception as exc:
             return plug.HookResult(SECTION, Status.ERROR, str(exc))
@@ -135,6 +133,7 @@ def parse_args(self, args: argparse.Namespace) -> None:
             if args.disable_security
             else self._disable_security
         )
+        self._run_student_tests = args.run_student_tests
 
     def clone_parser_hook(
         self, clone_parser: configparser.ConfigParser
@@ -208,6 +207,15 @@ def clone_parser_hook(
             action="store_true",
         )
 
+        clone_parser.add_argument(
+            "--junit4-run-student-tests",
+            help="Run test classes found in the student repos instead of "
+            "those from the reference tests directory. Only tests that exist "
+            "in the reference tests directory will be searched for.",
+            dest="run_student_tests",
+            action="store_true",
+        )
+
     def config_hook(self, config_parser: configparser.ConfigParser) -> None:
         """Look for hamcrest and junit paths in the config, and get the classpath.
 
@@ -235,7 +243,12 @@ def _compile_all(
         """
         java_files = list(path.rglob("*.java"))
         master_name = self._extract_master_repo_name(path)
-        test_classes = self._find_test_classes(master_name)
+        reference_test_classes = self._find_test_classes(master_name)
+        test_classes = (
+            _java.get_student_test_classes(path, reference_test_classes)
+            if self._run_student_tests
+            else reference_test_classes
+        )
         compile_succeeded, compile_failed = _java.pairwise_compile(
             test_classes, java_files, classpath=self._generate_classpath()
         )
@@ -264,7 +277,7 @@ def _extract_master_repo_name(self, path: pathlib.Path) -> str:
                 )
             )
             res = plug.HookResult(SECTION, Status.ERROR, msg)
-            raise _ActException(res)
+            raise _exception.ActError(res)
 
     def _find_test_classes(self, master_name) -> List[pathlib.Path]:
         """Find all test classes (files ending in ``Test.java``) in directory
@@ -285,7 +298,7 @@ def _find_test_classes(self, master_name) -> List[pathlib.Path]:
                     master_name, self._reference_tests_dir
                 ),
             )
-            raise _ActException(res)
+            raise _exception.ActError(res)
 
         test_classes = [
             file
@@ -302,7 +315,7 @@ def _find_test_classes(self, master_name) -> List[pathlib.Path]:
                     test_dir
                 ),
             )
-            raise _ActException(res)
+            raise _exception.ActError(res)
 
         return test_classes
 

tests/integration_tests/repos/student-with-bad-tests-week-10/README.md

@@ -0,0 +1 @@
+This repo should pass all tests

tests/integration_tests/repos/student-with-bad-tests-week-10/src/Fibo.java

@@ -0,0 +1,23 @@
+/**
+ * Class for calculating Fibonacci numbers.
+ */
+
+public class Fibo {
+    private long prev;
+    private long current;
+
+    public Fibo() {
+        prev = 0;
+        current = 1;
+    }
+
+    /**
+     * Generate the next Fibonacci number.
+     */
+    public long next() {
+        long ret = prev;
+        prev = current;
+        current = ret + current;
+        return ret;
+    }
+}

tests/integration_tests/repos/student-with-bad-tests-week-10/src/FiboTest.java

@@ -0,0 +1,38 @@
+import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.*;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class FiboTest {
+    @Test
+    public void correctlyGeneratesFirst10Numbers() {
+        Fibo f = new Fibo();
+        long[] expected = {0, 1, 1, 2, 3, 5, 8, 13, 21, 34};
+        long[] actual = new long[10];
+
+        for (int i = 0; i < 10; i++) {
+            actual[i] = f.next();
+        }
+
+        assertThat(actual, equalTo(expected));
+    }
+
+    @Test
+    public void correctlyGeneratesFiftiethNumber() {
+        // note that the first number is counted as the 0th
+        Fibo f = new Fibo();
+
+        for (int i = 0; i < 50; i++) {
+            f.next();
+        }
+
+        assertThat(f.next(), equalTo(12586269025l));
+    }
+
+    @Test
+    public void failingTest() {
+        fail("Student wrote a bad test");
+    }
+}

tests/integration_tests/repos/student-with-duplicate-tests-week-10/FiboTest.java

@@ -0,0 +1,33 @@
+import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.*;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class FiboTest {
+    @Test
+    public void correctlyGeneratesFirst10Numbers() {
+        Fibo f = new Fibo();
+        long[] expected = {0, 1, 1, 2, 3, 5, 8, 13, 21, 34};
+        long[] actual = new long[10];
+
+        for (int i = 0; i < 10; i++) {
+            actual[i] = f.next();
+        }
+
+        assertThat(actual, equalTo(expected));
+    }
+
+    @Test
+    public void correctlyGeneratesFiftiethNumber() {
+        // note that the first number is counted as the 0th
+        Fibo f = new Fibo();
+
+        for (int i = 0; i < 50; i++) {
+            f.next();
+        }
+
+        assertThat(f.next(), equalTo(12586269025l));
+    }
+}

tests/integration_tests/repos/student-with-duplicate-tests-week-10/README.md

@@ -0,0 +1 @@
+This repo should pass all tests

tests/integration_tests/repos/student-with-duplicate-tests-week-10/src/Fibo.java

@@ -0,0 +1,23 @@
+/**
+ * Class for calculating Fibonacci numbers.
+ */
+
+public class Fibo {
+    private long prev;
+    private long current;
+
+    public Fibo() {
+        prev = 0;
+        current = 1;
+    }
+
+    /**
+     * Generate the next Fibonacci number.
+     */
+    public long next() {
+        long ret = prev;
+        prev = current;
+        current = ret + current;
+        return ret;
+    }
+}