[Proposition] Adopt JUnit 5 & AssertJ

[echebbi]

Proposal

Tests are currently written with JUnit 4 and I believe we should move to JUnit 5:

• JUnit 5 makes easier to parameterize a test
• JUnit 5 makes easier to organize tests (e.g. through @Nested and @DisplayName):

I believe this is important because some test classes are becoming heavy (BuildTest is 1k lines, EvalTest is 1.3k and TypeValidatorTest 1.6k) which makes harder and harder to browse existing tests and can lead to duplicated tests.

Moreover, I suggest adopting AssertJ which fluent API makes tests much more readable:

@Test @DisplayName("executes all its tasks")
void executes_all_its_tasks() {
    split.run(context);
    assertThat(split.branches()).allMatch(Activity::hasRun);
}

AssertJ also provides SoftAssertions, a way to test several predicates at once. We currently have a lot of tests that stop early in case of failure and do not check all their predicates which hence could benefit from this API, e.g.:

ale-lang/tests/org.eclipse.emf.ecoretools.ale.tests/src/org/eclipse/emf/ecoretools/ale/core/validation/test/TypeValidatorTest.java

Lines 1841 to 1847 in cfa611d

	assertEquals(msg.toString(), 6, msg.size());
	assertMsgEquals(ValidationMessageLevel.ERROR, 241, 260, "[Set(java.lang.Integer)] cannot be removed from [Sequence(ecore::EBoolean)] (expected [Sequence(ecore::EBoolean),ecore::EBoolean])\n--------------------------------------------------\nMake sure both collections hold the same type", msg.get(0));
	assertMsgEquals(ValidationMessageLevel.ERROR, 283, 305, "[Sequence(java.lang.String)] cannot be removed from [Set(ecore::EClass)] (expected [Set(ecore::EClass),ecore::EClass])\n--------------------------------------------------\nMake sure both collections hold the same type", msg.get(1));
	assertMsgEquals(ValidationMessageLevel.ERROR, 414, 433, "[Set(java.lang.Integer)] cannot be removed from [Sequence(ecore::EBoolean)] (expected [Sequence(ecore::EBoolean),ecore::EBoolean])\n--------------------------------------------------\nMake sure both collections hold the same type", msg.get(2));
	assertMsgEquals(ValidationMessageLevel.ERROR, 452, 474, "[Sequence(java.lang.String)] cannot be removed from [Set(ecore::EClass)] (expected [Set(ecore::EClass),ecore::EClass])\n--------------------------------------------------\nMake sure both collections hold the same type", msg.get(3));
	assertMsgEquals(ValidationMessageLevel.ERROR, 500, 519, "[Set(java.lang.Integer)] cannot be removed from [Sequence(ecore::EString)] (expected [Sequence(ecore::EString),ecore::EString])\n--------------------------------------------------\nMake sure both collections hold the same type", msg.get(4));
	assertMsgEquals(ValidationMessageLevel.ERROR, 539, 566, "[Sequence(java.lang.Boolean)] cannot be removed from [Sequence(ecore::EInt)] (expected [Sequence(ecore::EInt),ecore::EInt])\n--------------------------------------------------\nMake sure both collections hold the same type", msg.get(5));

To be done

• Add JUnit 5 & AssertJ to the target platform (warning: they are in Orbit, which may slow down first Maven build)
• Make current tests use JUnit 5 and AssertJ

Discussion

@dvojtise do you see any reason to stick to JUnit 4?

[dvojtise]

There is no particular reason to stick to JUnit 4

If someone migrates the test suites I'll be happy to merge :-)

[echebbi]

Great, I'll try to find some time then. By the way, we may have already discuss this but can we use snake_case to name JUnit tests? I usually try to use long, explicit and meaningful names which are hard to read with camel case:

class ExecutionTest {

    @Test
    void when_launched_executes_its_tasks_in_sequential_order() {}

    @Test
    void while_running_when_canceled_stops_running() {}

    @Test
    void while_running_when_canceled_has_canceled_state() {}

}

Names are a bit shorter with JUnit 5, but I still find snake_case to be more readable (full example here):

class ExecutionTest {

    @Nested @DisplayName("when running")
    class WhenRunning {

        @BeforeEach
        void runExecution() {}

        @Nested @DisplayName("when canceled")
        class WhenCanceled {

            @BeforeEach
            void cancelExecution() {}

            @Test
            void stops_running() {}

            @Test
            void has_canceled_state() {}
        }
    }

}

[dvojtise]

you're right snake case looks nice for test names, I usually use camelCase mainly because I reuse the general rule for method names but for tests, the snake case looks better.