Separate tester and layout logic

[kalabukdima]

Current state

Right now there is a CourseDriver responsible for handling layout and providing an interface to find public/private/etc directories. It looks like it is designed to abstract away from the directory structure of the course (how tasks are organized into directories and where the tests and author solutions are located).

There are also multiple testers that accept paths to directories provided by the course driver and check the given solution. Let's look closer at how they do it.

CppTester

Build stage

• Checks that the solution doesn't contain forbidden_regexp.
• Copies modifications in allowed files in the solution to the public tests dir.
• Copies needed files (copy_to_build list) from the public tests dir directly to the build dir.
• Runs cmake -G Ninja -DGRADER=YES -DENABLE_PRIVATE_TESTS=YES -DCMAKE_BUILD_TYPE=${config.build_type} ${root} in the build dir, where ${root} is the path to repo containing public tests (with modifications from the previous steps).
• Runs ninja -v ${target} for every target in config.tests.
• Runs ${root}/run-clang-format.py.
• Runs clang-tidy for all .cpp files in public tests dir.

Run tests stage

• Runs every target built on the build stage.
• Either checks that it doesn't crash or that it crashes depending on the value of config.is_crash_me.

PythonTester (simplified)

Build stage

• Copies all files from the solution to the build dir, except test_files and blacklisted files.
• Checks that files in the build dir don't contain forbidden_regexp
• Copies public tests from the public tests dir to the build dir.
• Copies private tests from the private tests dir to the build dir.

Run tests stage

• Runs flake8 in the build dir.
• Runs mypy in the build dir.
• Runs pytest in the build dir.

MakeTester

Build stage

• Copies all files from the solution to the build dir.
• Copies files specified in public_test_files from public tests dir to the build dir.
• Copies files specified in private_test_files from private tests dir to the build dir.

Run tests stage

• Runs make -B in the build dir.

And there is the wrapping script that creates runs the tester with the course driver using student's solution in the grade mode and author's solution in the check mode.

Problems

I see multiple problems with this approach:

• The tester defines how public, private and solution files are merged before testing instead of leaving that responsibility to the course driver (right now all the testers do it in completely different ways). It makes the tester and the course driver tightly coupled.
• MakeTester was probably meant to be flexible and support testing any language but it turns out to be the least flexible of them because it is not possible to access common files located outside of the task folder.
• There are a lot of undocumented requirements for the course repository to be used with any checker. For example, with the CppTester it's not clear what are CMake files supposed to do with the GRADER and ENABLE_PRIVATE_TESTS variables. Private tests location is ignored by the checker and it's responsibility of the course repository to find and use them.
• In the existing C++ course repositories there is also a variable TEST_SOLUTION in the CMake scripts that allows testing author solutions, but it's behavior differs from running checker check. It should probably be removed in favor of checker check.

Proposal

I think that a better approach would be to have the course driver (or another layer on top of it) prepare the repository with the solution and leave only the "testing" for the tester. By testing I mean checking the correctness of the given task directory populated with the solution and test files. For example, for Python it would just mean checking forbidden expressions, running flake8, mypy and pytest in the task dir. For C++ it would mean checking forbidden expressions, compiling task's targets and running them.

It would also allow moving the testing logic from the checker to the course repo and provide easier support for any future language (see #35).

The following merging strategy should be sufficient for all existing courses:

• Copy everything from private tests to public tests dir;
• Copy files allowed by the allow_change pattern from the solution to public tests dir (ensure that pattern rules are flexible enough to exclude some files from globbing);
• Run tester in public tests dir and allow it to move everything to the temporary build dir if needed.

Open questions:

• Can the logic of "merging" tests and solutions always be defined by the layout alone or do we have to control it separately?