Skip to content

Upgrade to test runner v3 #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Sep 18, 2023
Merged

Upgrade to test runner v3 #61

merged 15 commits into from
Sep 18, 2023

Conversation

sanderploegsma
Copy link
Contributor

@sanderploegsma sanderploegsma commented Sep 4, 2023

What started off as an attempt to speed up the test runner by dropping Maven in favor of directly compiling solutions using Java's built-in JavaCompiler resulted in almost a complete rewrite:

Programmatic compilation of solutions

The test runner now compiles the submitted solutions directly. This loses the runtime overhead of running a Maven process in the background. This is a big time-saver with little extra complexity IMO.

Programmatic invocation of JUnit

With the solutions being compiled by the test runner, we should also programmatically run JUnit. The test runner contains both junit-jupiter-engine and junit-vintage-engine to support both JUnit 4 and JUnit 5 tests, which is useful if we want to gradually adopt JUnit 5 in the exercises.

Capturing of stdout

I configured JUnit to capture stdout while running tests, and made sure the captured output ends up in the JSON output of the test runner. This allows students to add print statements to their solutions in the online editor to debug failing tests if they choose.

V3 compatibility

I came up with the following solution to allow for the task_id to be determined in order to support the v3 spec of the test runner: we can use JUnit's @Tag annotation to annotate each concept exercise test with the appropriate task ID, and then retrieve the annotations from the JUnit test execution results.

The concept exercise tests should then contain tests like this:

public class ConceptExerciseTest {
    @Test
    @Tag("task:1")
    public void testForTask1() {
        // ...
    }

    @Test
    @Tag("task:2")
    public void testForTask2() {
        // ...
    }
}

And it would result in the following output:

{
    "version": 3,
    "status": "pass",
    "tests": [
        {
            "name": "testForTask1()",
            "status": "pass",
            "task_id": 1
        },
        {
            "name": "testForTask2()",
            "status": "pass",
            "task_id": 2
        }
    ]
}

Unfortunately there is nothing of the sort in JUnit 4. It supports categories but since these are class markers and not string markers they would have to be defined separately in every test class, and it would have to rely on some sort of naming convention for the test runner to be able to correctly determine which marker belongs to which task ID. It's possible, but I figured using JUnit 5's @Tag is a lot easier.

@sanderploegsma sanderploegsma marked this pull request as ready for review September 4, 2023 19:15
@sanderploegsma sanderploegsma requested a review from a team as a code owner September 4, 2023 19:15
@sanderploegsma
Copy link
Contributor Author

@ErikSchierboom you’ll like this I think 😃

Not sure whether a similar approach will help the Kotlin test runner though as you already mentioned that using kotlinc didn’t show a major difference.

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this PR! Such great work. I might be worth you checking the Kotlin test runner, it could be that I just failed in my JVM skills (which admittedly are bad)

@sanderploegsma sanderploegsma requested a review from a team September 5, 2023 07:53
@sanderploegsma
Copy link
Contributor Author

@ErikSchierboom thanks!

Just wondering, does this PR fall in the size:large or size:huge category in your opinion? 😄

@ErikSchierboom
Copy link
Member

Well, it depends how long you were working on it :D

@sanderploegsma sanderploegsma merged commit 1262485 into main Sep 18, 2023
@sanderploegsma sanderploegsma deleted the remove-maven branch September 18, 2023 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:size/large Large amount of work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants