Introduce JUnit5 #2493

RoiEXLab · 2017-10-13T21:37:44Z

See #935 and #951 for more information.

This PR introduces the JUnit5 Runner now that eclipse officially supports JUnit5 in the latest 4.7.1a build.
This means you are highly encouraged to write new tests using the new namespace (importing the test annotation from junit5), and migrate the junit 4 test cases every now and then.

This is still experimental, it is working for me, but I really want to hear some feedback...

RoiEXLab · 2017-10-13T22:23:48Z

It seems that the current setup does not integrate with the jacocotestreport really well.
Do not merge this PR until we find a solution for this.

ssoloff · 2017-10-14T03:28:45Z

It seems that the current setup does not integrate with the jacocotestreport really well.

Is junit-team/junit5#1024 related?

ssoloff · 2017-10-14T03:35:09Z

build.gradle

@@ -11,6 +20,7 @@ plugins {
 }

 apply from: 'gradle/scripts/yaml.gradle'
+apply plugin: 'org.junit.platform.gradle.plugin'


Ugh, but at least the JUnit team is aware of the issue: junit-team/junit5#768. However, it appears to have been pushed back to 5.1 as of 14 hours ago. 😞

Note that there's a workaround listed in the issue, but I'm not sure if it's worth introducing that just to keep build.gradle clean.

Nit: Pasting the buildscript configuration from the JUnit docs introduced mixed tabs/spaces into this file.

DanVanAtta

tests do run on intellij FWIW, though had a result that was a good bit slower (~30s) than compared to master. The tests are overall too slow to effectively evaluate this changing, I think this is blocked until we get our tests running in a reasonable 15s ~ 20s ballpark. Then we can more easily see if there is a real performance difference. Adding 30s to a test run is a non-started, so I'd like to verify that result does not carry over once we fix the test execution speed.

DanVanAtta · 2017-10-14T06:35:36Z

It's really good to see your tenacity to get this working @RoiEXLab. It's valuable to get to a newer technology when we can.

RoiEXLab · 2017-10-14T06:46:02Z

Is junit-team/junit5#1024 related?

@ssoloff
I think so, seems like it.

RoiEXLab · 2017-10-15T20:35:57Z

@ssoloff
junit-team/junit5#1024 provides a workaround for this issue, but I have no idea how to apply it, pasting it into the buildscript isn't working 😅

afterEvaluate {
    val junitPlatformTest : JavaExec by tasks
    jacoco {
        applyTo(junitPlatformTest)
    }
    task<JacocoReport>("jacocoJunit5TestReport") {
        executionData(junitPlatformTest)
        sourceSets(java.sourceSets["main"])
        sourceDirectories = files(java.sourceSets["main"].allSource.srcDirs)
        classDirectories = files(java.sourceSets["main"].output)
    }
}

I think this is kotlin code, any suggestions on how to apply it?
You should have push access to this branch, so in case a fix is really obvious there's no need to create a PR

codecov-io · 2017-10-15T21:42:36Z

Codecov Report

Merging #2493 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #2493      +/-   ##
============================================
+ Coverage     20.56%   20.56%   +<.01%     
- Complexity     5848     5860      +12     
============================================
  Files           831      831              
  Lines         73614    73618       +4     
  Branches      12400    12401       +1     
============================================
+ Hits          15136    15141       +5     
  Misses        56389    56389              
+ Partials       2089     2088       -1

Impacted Files	Coverage Δ	Complexity Δ
...trategy/engine/lobby/client/ui/TimespanDialog.java	`41.46% <0%> (-1.04%)`	`3% <0%> (ø)`
.../framework/startup/login/ClientLoginValidator.java	`46.93% <0%> (-0.98%)`	`9% <0%> (ø)`
...ine/framework/map/download/DownloadMapsWindow.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
.../strategy/triplea/ai/proAI/ProNonCombatMoveAI.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...amework/map/download/MapDownloadProgressPanel.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...es/strategy/triplea/printgenerator/SetupFrame.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...va/games/strategy/engine/framework/GameRunner.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...mes/strategy/triplea/ai/proAI/util/ProMatches.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...trategy/engine/delegate/DefaultDelegateBridge.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...ames/strategy/triplea/settings/SettingsWindow.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1263005...2dcdb4a. Read the comment docs.

ssoloff · 2017-10-15T21:49:03Z

@RoiEXLab I pushed a commit to your branch that implements the workaround in the issue you referenced.

Some other things I noted while playing around here:

As @DanVanAtta said, tests are running MUCH slower.
We need to suppress/redirect stdout/stderr to ensure we don't blow up the Travis logs. Ideally, we want the same behavior as before where stdout/stderr are captured in the test results stored under build/test-results/junit-platform/TEST-*.xml.
We should generate a JUnit HTML report in addition to the XML report, as was done with JUnit 4. This makes it easier for devs to review detailed results when running tests from the command line.
I think all the Enclosed tests are being run twice. Probably because the hack we added to ensure they only run once doesn't apply to the new junitPlatformTest task. I believe JUnit 5 has built-in support for nested tests, so it may be better to just convert them all incrementally.

I suspect (2) and (3) probably just require changing the junitPlatformTest task configuration, as needed. And a quick fix for (4), until we can convert the tests, may just require a similar configuration change:

junitPlatformTest {
    exclude '**/*Tests.class'
}

But, unfortunately, I agree with @DanVanAtta that (1) is kind of a show-stopper until we can figure out how to improve the performance. Maybe it's just due to the fact that the vast majority of the tests have to run through the vintage test runner, so there's extra indirection there? Dunno...

RoiEXLab · 2017-10-15T21:56:34Z

@ssoloff It's possible to update all tests at once to compile with junit 5 with some basic find and replace. (replacing imports is sufficient)
I didn't do that because junit5 has reversed parameters for assertEquals and my regex knowledge is not advanced enough to automatically change that as well.
We could get a more accurate benchmark this way.

ssoloff · 2017-10-15T22:04:14Z

I didn't do that because junit5 has reversed parameters for assertEquals

According to the JUnit 5 Javadocs, the order is expected followed by actual, which is the same as in JUnit 4, and I believe has been that way since at least JUnit 3. Are you talking about something different?

We may be using the API incorrectly (I've observed that in a few places), but it will be the same incorrect usage between JUnit 4 and 5. The only real problem is that if an assertion fails, the message printed will contain reversed values for expected and actual, and we can just fix those incrementally as we revisit tests.

ssoloff · 2017-10-15T23:07:55Z

@RoiEXLab

For issue (4), I pushed another commit that prevents the Enclosed tests from running twice.

For issue (2), there appears to be no way to redirect stdout/stderr at this time (see junit-team/junit5#780). 😞

For issue (3), there is a known limitation that prevents the JUnit 5 runner's results from being included in the standard Gradle test report. 😞

RoiEXLab · 2017-10-16T08:48:21Z

According to the JUnit 5 Javadocs, the order is expected followed by actual, which is the same as in JUnit 4, and I believe has been that way since at least JUnit 3. Are you talking about something different?

@ssoloff Interesing I could swear they were reversed at some point.

In this case I will upgrade them at once.

DanVanAtta · 2017-10-18T02:26:35Z

Isn't there a hamcrest style API/library built into Junit5 that avoids the expected and actual ordering problem by being type safe? It is one of the perhaps really nice features of hamcrest that you do not have to worry about it so much.

DanVanAtta · 2017-10-18T03:07:18Z

Running all tests (no integration) from IntelliJ, got the following results (all in seconds):

I switched over to Junit5 and did the same but only three times with the following results:

3m 0.880s
3m 34.805s
2m 56s

I remember when we had Junit5 previously for a bit that it was a bit slower, but not so drastically. Previously as well IntelliJ was running Junit5 by essentially running the build script and reporting back the results. Checking gradle builds as well on my system, the tests seem to still be running slow. I'm afraid this might be a nail in the coffin for the moment.

RoiEXLab · 2017-10-18T08:19:04Z

There's no longer an assertThat method by JUnit 5.
We have to use the one by hamcrest itself.
I believe (and hope) this performance issue is caused by the junit 4/5 duality and will disappear once we migrated everything, I will check...

RoiEXLab · 2017-10-18T20:19:12Z

I updated all tests to JUnit5.

@DanVanAtta I ran the tests, and on my machine they run equally fast using JUnit 5 and 4 (maximum 3 seconds difference), would be nice if you could verify the same.

Now there are just 2 (maybe 3) issues left.
For one the integration tests are not getting picked up. The integTest task runs fine, but does nothing.
And it seems about 150 tests less are being executed, I'd assume they are not getting picked up due to file filters or similar, some investigation will be needed.

For whatever reason javadoc errors for the integTests showed up, I have no idea why, shouldn't the test classes just get ignored?
@ssoloff Ideas?

DanVanAtta · 2017-10-19T03:09:20Z

good work, can confirm performance is there again:

gradle via CLI first run
1st run: 1m 20s total, but: "Test run finished after 26777 ms:""
2nd run: Test run finished after 24203 ms


intellij runs:
18s 614 
18s 278
18s 149

ssoloff · 2017-10-19T04:15:30Z

For one the integration tests are not getting picked up. The integTest task runs fine, but does nothing.

I'm guessing this is due to the JUnit 5 test task no longer being of type Test. We'll have to look into if there is a new type provided by the JUnit 5 Gradle plugin that we should extend for integTest. Worst case, we can always keep the integration tests as JUnit 4 tests for now.

ssoloff · 2017-10-19T04:56:43Z

And it seems about 150 tests less are being executed, I'd assume they are not getting picked up due to file filters or similar, some investigation will be needed.

My experience seems to be different. Running on master (which this branch is currently up-to-date with minus one commit that has no code changes), I get a unit test count of 924. Running on this branch, I get a unit test count of 926. Not sure where those extra two tests are coming from. I've been unsuccessful in locating them so far. I'm going to have to just parse the test results looking for the differences (which is made more difficult by the fact that JUnit 4 puts each test class's results in a separate file, while JUnit 5 puts them all in a single file).

ssoloff

Thanks for slogging through the conversions, @RoiEXLab. It required more work than just a simple search-and-replace. 😄

ssoloff · 2017-10-19T05:00:13Z

src/test/java/games/strategy/engine/config/lobby/LobbyPropertyReaderTest.java

  public void setup() throws IOException {
-    final File testFile = tempFolderRule.newFile();
+    final File testFile = tempFolderRule.newFile("testname");


Recommend changing the argument to getClass().getName().

ssoloff · 2017-10-19T05:00:26Z

src/test/java/games/strategy/engine/framework/map/download/DownloadUtilsTests.java

    public void setUp() throws Exception {
-      file = temporaryFolder.newFile();
+      file = temporaryFolder.newFile("filename1123");


Recommend changing the argument to getClass().getName().

ssoloff · 2017-10-19T05:00:39Z

src/test/java/games/strategy/engine/framework/map/download/FileSystemStrategyTest.java

  public void setUp() throws Exception {
    testObj = new FileSystemAccessStrategy();
    final String text = DownloadFileProperties.VERSION_PROPERTY + " = 1.2";
-    mapFile = temporaryFolder.newFile();
+    mapFile = temporaryFolder.newFile("someothertestname");


Recommend changing the argument to getClass().getName().

ssoloff · 2017-10-19T05:01:34Z

src/test/java/games/strategy/test/EqualsPredicateTests.java

@@ -105,7 +108,8 @@ public void shouldReturnFalseWhenObjectsAreNotEqual() {
          new FakeNestedClass(new FakeClass(-42))));
    }

-    private static final class FakeNestedClass {
+    @Nested
+    private final class FakeNestedClass {


This class does not contain tests, and thus does not need to be annotated with @Nested.

ssoloff · 2017-10-19T05:01:44Z

src/test/java/games/strategy/test/EqualsPredicateTests.java

@@ -159,7 +164,8 @@ public void shouldReturnFalseWhenOwnerObjectsAreNotEqual() {
      assertFalse(equalsPredicate.test(owner1, owner2));
    }

-    private static final class FakeOwnerClass {
+    @Nested
+    private final class FakeOwnerClass {


This class does not contain tests, and thus does not need to be annotated with @Nested.

ssoloff · 2017-10-19T05:01:50Z

src/test/java/games/strategy/test/EqualsPredicateTests.java

@@ -168,7 +174,8 @@ public void shouldReturnFalseWhenOwnerObjectsAreNotEqual() {
      }
    }

-    private static final class FakeOwneeClass {
+    @Nested
+    private final class FakeOwneeClass {


This class does not contain tests, and thus does not need to be annotated with @Nested.

ssoloff · 2017-10-19T05:02:20Z

src/test/java/games/strategy/test/EqualsPredicateTests.java

@@ -179,7 +186,8 @@ public void shouldReturnFalseWhenOwnerObjectsAreNotEqual() {
    }
  }

-  private static final class FakeClass {
+  @Nested
+  private final class FakeClass {


This class does not contain tests, and thus does not need to be annotated with @Nested.

It also can remain static because it's a child of the top-level class.

ssoloff · 2017-10-19T05:09:17Z

For whatever reason javadoc errors for the integTests showed up, I have no idea why, shouldn't the test classes just get ignored?

You need to update the JavadocMethod rule configuration to recognize the new JUnit 5 annotations. In config/checkstyle/checkstyle.xml, you need to change this line:

<property name="allowedAnnotations" value="Override, Test, BeforeClass, AfterClass, Before, After"/>

ssoloff · 2017-10-19T06:10:54Z

My experience seems to be different. Running on master (which this branch is currently up-to-date with minus one commit that has no code changes), I get a unit test count of 924. Running on this branch, I get a unit test count of 926.

Found the differences:

The following top-level tests in g.s.triplea.delegate.MatchesTests are run in JUnit 5 but are not being run in JUnit 4:

testAlways
testCountMatches
testGetMatches
testGetNMatches
testGetNMatches_ShouldThrowExceptionWhenMaxIsNegative
testNever

They are not run in JUnit 4 because the top-level class is declared Enclosed, which means that all tests must be in a nested class. After converting the nested classes to Nested in JUnit 5, these tests are now being correctly discovered and run.

The remaining differences are due to an erroneous @Test annotation on the g.s.triplea.delegate.DelegateTest#setUp() method (introduced prior to this PR). Because this class has four subclasses, JUnit 4 reports five occurrences of this "test." For whatever reason, JUnit 5 only reports one occurrence of this "test." This results in a net increase of four tests for the JUnit 4 run.

@RoiEXLab Please change @Test on this setUp() method to @BeforeEach as part of this PR:

diff --git a/src/test/java/games/strategy/triplea/delegate/DelegateTest.java b/src/test/java/games/strategy/triplea/delegate/DelegateTest.java
index e4d12c4..1a5378d 100644
--- a/src/test/java/games/strategy/triplea/delegate/DelegateTest.java
+++ b/src/test/java/games/strategy/triplea/delegate/DelegateTest.java
@@ -3,6 +3,7 @@ package games.strategy.triplea.delegate;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import games.strategy.engine.data.GameData;
@@ -78,7 +79,7 @@ public class DelegateTest {
   protected UnitType carrier;
   protected Resource pus;
 
-  @Test
+  @BeforeEach
   public void setUp() throws Exception {
     gameData = TestMapGameData.DELEGATE_TEST.getGameData();
     british = GameDataTestUtil.british(gameData);

Thus +6 tests for JUnit 5 and +4 tests for JUnit 4 ends up being +2 tests for JUnit 5, which is 926 - 924.

RoiEXLab · 2017-10-19T08:56:59Z

@ssoloff I looked into the integTest problem.
I noticed pretty fast that there's currently no way to define an additional Task to be a junit5 task.
However there's a way to execute it in a seperate task using the junit5 console launcher api.
It's essentially an API with a main method, which automatically runs test in the specified directories.
I couldn't get it to really work, but I was able to invoke the console launcher so far:

diff --git a/build.gradle b/build.gradle
index cd9a86a..bad0c12 100644
--- a/build.gradle
+++ b/build.gradle
@@ -59,7 +59,7 @@ sourceSets {
         java.srcDir 'src/integ_test/java'
         resources.srcDir 'src/integ_test/resources'

-        compileClasspath = sourceSets.main.output + sourceSets.test.output + configurations.testRuntime
+        compileClasspath = sourceSets.main.output + sourceSets.test.output + configurations.testRuntime + configurations.junitPlatform
         runtimeClasspath = output + compileClasspath
     }
 }
@@ -111,19 +111,22 @@ dependencies {
     testRuntime 'org.junit.jupiter:junit-jupiter-engine:5.0.1'
 }

-task integTest(type: Test) {
+task integTest(type: junitPlatformTest.getClass()) {
     group = LifecycleBasePlugin.VERIFICATION_GROUP
     description = 'Runs the integration tests.'

-    testClassesDirs = sourceSets.integTest.output.classesDirs
     classpath = sourceSets.integTest.runtimeClasspath

-    binResultsDir = file("$buildDir/integration_test_results/binary/integTest")
+    //binResultsDir = file("$buildDir/integration_test_results/binary/integTest")

-    reports {
+    /*reports {
         reports.html.destination = file("${reports.html.destination}/$name")
         reports.junitXml.destination = file("${reports.junitXml.destination}/$name")
-    }
+    }*/
+    outputs.dir file("$buildDir/integration_test_results/binary/integTest")
+    main = org.junit.platform.console.ConsoleLauncher.class.getName()
+
+    args = ['-d', sourceSets.integTest.java.srcDirs]

     mustRunAfter tasks.test
 }

TL;DR There's no easy way to do it, but there is a way. See junit-team/junit5#874 for more information

RoiEXLab · 2017-10-19T09:12:39Z

src/integ_test/java/games/strategy/engine/chat/ChatIntegrationTest.java

-    waitFor(this::serverNodeToDisconnect);
+  @Test
+  public void shouldBeAbleToChatAcrossMultipleNodes() {
+    assertTimeout(Duration.ofSeconds(15), () -> {


There's a assertTimeoutPreemptively method, which fails instantly after the timeout limit is reached.
This method always lets the test methods finish first. Should we use the other method?
The same for the other test cases where this method is used.

I just learned that this is the Junit4 behaviour... Changing that now
https://github.com/junit-team/junit4/wiki/Timeout-for-tests

Agreed, It looks like the rule-of-thumb is to use assertTimeout() for things like performance tests (e.g. this code that never blocks needs to run within a certain amount of time), while assertTimeoutPreemptively() should be used for anything that can possibly block.

RoiEXLab · 2017-10-19T09:22:54Z

When using JUnit 5, we can drop the catch exception library as JUnit 5 has a nice exception API on it's own.

NullPointerException e = assertThrows(NullPointerException.class, () -> Preconditions.checkNotNull(null));
assertEquals("something", e.getMessage());
//or something like that

ssoloff · 2017-10-19T15:26:21Z

When using JUnit 5, we can drop the catch exception library as JUnit 5 has a nice exception API on it's own.

Yup, I believe you mentioned that a few months ago, so I've been looking forward to switching to JUnit 5. 😄 This PR is getting pretty big, though. It might be easier to just do that in a follow-up PR, if you'd like.

ssoloff · 2017-10-19T15:28:00Z

build.gradle

@@ -95,15 +105,12 @@ dependencies {
    testCompile 'eu.codearte.catch-exception:catch-exception:2.0.0-ALPHA-1'
    testCompile 'nl.jqno.equalsverifier:equalsverifier:2.3.3'
    testCompile 'org.hamcrest:java-hamcrest:2.0.0.0'
-    testCompile 'org.mockito:mockito-core:2.10.0'
-    testCompile 'junit:junit:4.12'
+    testCompile 'org.mockito:mockito-core:2.8.47'


BTW, was just curious what triggered the need to downgrade Mockito?

Bad merge conflict resolution, will fix that ^^

ssoloff · 2017-10-19T15:34:41Z

src/test/java/games/strategy/triplea/delegate/PlaceDelegateTest.java

-  public void setUp() throws Exception {
-    super.setUp();
+  @BeforeEach
+  public void setupPlaceDelegate() {


Does JUnit 5 make guarantees on the order @BeforeEach methods will be called? That is, does it ensure that a @BeforeEach method in the superclass will be called before a @BeforeEach method in a subclass?

EDIT: Nevermind, answered my own question:

@BeforeEach methods are inherited from superclasses as long as they are not overridden. Furthermore, @BeforeEach methods from superclasses will be executed before @BeforeEach methods in subclasses.

Yes, I should have provided a link to the JUnit docs

RoiEXLab · 2017-10-19T15:48:02Z

Yup, I believe you mentioned that a few months ago, so I've been looking forward to switching to JUnit 5. 😄 This PR is getting pretty big, though. It might be easier to just do that in a follow-up PR, if you'd like.

@ssoloff Of course, I planned to do that in a follow-up PR, just wanted to mention it.

RoiEXLab · 2017-10-20T13:04:14Z

@ssoloff @DanVanAtta I figured out how to run our integrationTests with junit 5.
Worth noting is that we now explicitly require integration tests to match ^.*IntegrationTests?$ (otherwhise the integrationTest task runs the normal tests as well, because they are included in the classpath as well)

This PR is now ready for a final review, I'd prefer minor issues to be adressed in a follo-up PR though, to avoid merge conflicts as the diff is pretty large

ssoloff

There's a minor issue when running the integration tests. It is scanning the entire classpath instead of just the integTest source set's classpath. There's really nothing we can do about that because we're running JUnit outside of Gradle at this point.

The problem is that it's picking up a unit test class that happens to have the suffix IntegrationTest (ProxyableObjectInputOutputStreamIntegrationTest), so that the integTest task has seven more tests running using JUnit 5 (68) than with JUnit 4 (61). The test class in question has that particular suffix because it was testing the integration between ProxyableObjectOutputStream and ObjectInputStream, but it is still a "fast" test (because everything is done in memory), which is why it is included in the test source set.

I think the easiest thing at this point is to just rename that test. For lack of a better idea, my suggestion would be to name it ProxyableObjectInputOutputTest.

ssoloff · 2017-10-20T17:32:30Z

build.gradle

@@ -106,24 +106,21 @@ dependencies {
    testCompile 'nl.jqno.equalsverifier:equalsverifier:2.3.3'
    testCompile 'org.hamcrest:java-hamcrest:2.0.0.0'
    testCompile 'org.mockito:mockito-core:2.11.0'
-    testCompile 'org.junit.platform:junit-platform-launcher:1.0.1'
+    testRuntime 'org.junit.platform:junit-platform-launcher:1.0.1'


Nit: Could you please move this down one line so we keep the different configurations grouped together? It's easy to miss the difference between testCompile and testRuntime when scanning the list. In fact, maybe an extra line of whitespace between them would be helpful, just as we did between compile and testCompile above.

ssoloff · 2017-10-20T17:33:24Z

build.gradle

-    }
+    main = 'org.junit.platform.console.ConsoleLauncher'
+    args '--details', 'none'
+    args "--reports-dir=$project.buildDir/integ-test-results/junit-platform"


Nit: The project. qualifier is superfluous here; $buildDir should be sufficient and is consistent with the rest of the build script.

ssoloff · 2017-10-20T18:18:09Z

@RoiEXLab I missed your request to defer minor issues until a follow-up PR. I consider my suggestions to be minor. If you want to wait, that's fine with me. I'll change my review to Approved, but leave the PR open for @DanVanAtta.

@ssoloff

Given that @ssoloff already approved this PR, I'm going ahead and merge this in order to not block any further work. Please go ahead and reply with potential concerns on the follow-up PR.

RoiEXLab · 2017-10-22T08:15:19Z

This PR has been merged.
For eclipse users this means they have to update to Oxygen.1A (4.7.1a), to have JUnit 5 support from within the IDE (otherwhise they have to use gradle to do it: gradlew clean junitPlatformTest)

ssoloff · 2017-10-23T00:36:30Z

otherwhise they have to use gradle to do it: gradlew clean junitPlatformTest

Note that you can still run gradlew clean test. The JUnit 5 Gradle plugin apparently configures the junitPlatformTest task to be a dependency of the test task.

RoiEXLab requested review from ssoloff and DanVanAtta October 13, 2017 21:37

RoiEXLab changed the title ~~Introduce JUnit5~~ [DO NOT MERGE] Introduce JUnit5 Oct 13, 2017

ssoloff reviewed Oct 14, 2017

View reviewed changes

DanVanAtta previously requested changes Oct 14, 2017

View reviewed changes

RoiEXLab changed the title ~~[DO NOT MERGE] Introduce JUnit5~~ Introduce JUnit5 Oct 15, 2017

RoiEXLab and others added 5 commits October 18, 2017 11:35

Introduce JUnit5

ef49bcb

Fix spaces

9cb8598

Add workaround to get JaCoCo to work with JUnit 5 from Gradle

97473ea

Prevent Enclosed tests from running twice

0f9a6af

Migrate all tests to JUnit5

562c057

Use field

36739a8

ssoloff requested changes Oct 19, 2017

View reviewed changes

RoiEXLab added 2 commits October 19, 2017 10:04

Fix checkstyle

5eaf9a0

Improve Tests

0360f1b

RoiEXLab commented Oct 19, 2017

View reviewed changes

Use assertTimeoutPreemptively instead of assertTimeout

0966391

ssoloff reviewed Oct 19, 2017

View reviewed changes

RoiEXLab added 2 commits October 19, 2017 17:51

Fix mockito version

ded62e9

Fix integration Tests

b0620a9

Smaller improvements

2dcdb4a

ssoloff requested changes Oct 20, 2017

View reviewed changes

ssoloff approved these changes Oct 20, 2017

View reviewed changes

RoiEXLab assigned DanVanAtta Oct 20, 2017

RoiEXLab merged commit 5432fd5 into triplea-game:master Oct 22, 2017

RoiEXLab deleted the junit5 branch October 22, 2017 08:13

This was referenced Oct 22, 2017

Junit cleanup #2521

Merged

Get rid of catch exception dependency #2522

Merged

ssoloff unassigned DanVanAtta Feb 27, 2018

Introduce JUnit5 #2493

Introduce JUnit5 #2493

Conversation

RoiEXLab commented Oct 13, 2017

RoiEXLab commented Oct 13, 2017

ssoloff commented Oct 14, 2017

Choose a reason for hiding this comment

DanVanAtta left a comment

Choose a reason for hiding this comment

DanVanAtta commented Oct 14, 2017

RoiEXLab commented Oct 14, 2017

RoiEXLab commented Oct 15, 2017

codecov-io commented Oct 15, 2017 • edited Loading

Codecov Report

ssoloff commented Oct 15, 2017

RoiEXLab commented Oct 15, 2017

ssoloff commented Oct 15, 2017

ssoloff commented Oct 15, 2017

RoiEXLab commented Oct 16, 2017

DanVanAtta commented Oct 18, 2017

DanVanAtta commented Oct 18, 2017

RoiEXLab commented Oct 18, 2017

RoiEXLab commented Oct 18, 2017

DanVanAtta commented Oct 19, 2017

ssoloff commented Oct 19, 2017

ssoloff commented Oct 19, 2017

ssoloff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssoloff commented Oct 19, 2017

ssoloff commented Oct 19, 2017

RoiEXLab commented Oct 19, 2017 • edited Loading

RoiEXLab Oct 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoiEXLab commented Oct 19, 2017 • edited Loading

ssoloff commented Oct 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoiEXLab commented Oct 19, 2017

RoiEXLab commented Oct 20, 2017

ssoloff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssoloff commented Oct 20, 2017

RoiEXLab commented Oct 22, 2017

ssoloff commented Oct 23, 2017

codecov-io commented Oct 15, 2017 •

edited

Loading

RoiEXLab commented Oct 19, 2017 •

edited

Loading

RoiEXLab Oct 19, 2017 •

edited

Loading

RoiEXLab commented Oct 19, 2017 •

edited

Loading