Adds JUnit 5 test support #3322

reinhapa · 2020-03-25T18:59:03Z

Implements the #3321 feature.

dizzzz · 2020-03-25T19:56:40Z

rerunning tests as in travis all tests failed.

dizzzz

LGTM, now lets convince travis

reinhapa · 2020-03-25T20:07:50Z

@dizzzz I got some trouble getting the problem that travis seems to have while building... :-(

dizzzz · 2020-03-26T07:00:51Z

appveyor and travis are consistent, interestingly:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-dependency-plugin:3.1.2:analyze-only (analyze) on project exist-ant: Dependency problems found -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :exist-ant
Command exited with code 1
Running "on_failure" scripts

dizzzz · 2020-03-26T07:01:51Z

https://www.baeldung.com/junit-5-migration

<dependency>
    <groupId>org.junit.vintage</groupId>
    <artifactId>junit-vintage-engine</artifactId>
    <version>${junit5.vintage.version}</version>
    <scope>test</scope>
</dependency>

is missing for the sub-pomfiles?

(sorry for closing)

dizzzz · 2020-03-28T09:11:50Z

hmmm

adamretter · 2020-03-28T10:23:09Z

I took a look into this PR this morning and tried a few things. Unfortunately it is not a straight-forward thing to move eXist-db to Junit 5, for three reasons:

We use JUnit rules for starting and stopping the eXist-db server for tests. It is not clear if these would still work in JUnit5, we would have to test, or migrate them to the equivalent JUnit5 approach - https://junit.org/junit5/docs/current/user-guide/#migrating-from-junit4
We make use of Google's JUnit Toolbox which enables us to run lots of JUnit 4 test methods in parallel. JUnit 5 has something itself built in for parallel execution which we could migrate to, but it is marked as "experimental" - https://junit.org/junit5/docs/current/user-guide/#writing-tests-parallel-execution
eXist-db's XQueryTestRunner and XMLTestRunner which execute XQSuite tests via JUnit uses JUnit 4's Runner APIs which don't exist in JUnit5. Instead we would need to rewrite these substantially to use JUnit5's TestFactory API - https://junit.org/junit5/docs/current/user-guide/#writing-tests-dynamic-tests

Now obviously this can all be solved given enough time and resource, but it is going to be quite involved I think. Also, I wonder if now is the right time to move to JUnit 5, as some of the APIs we want to use e.g. parallelism, are marked as experimental.

I am happy for @reinhapa and others to take this forward if they still want to, but personally I probably would not have much time to offer for this as I guess I don't yet see what the benefit of doing so would be?

dizzzz

see Adams remarks :-(

reinhapa · 2020-03-28T17:16:45Z

@adamretter the goal of this PR is not the migration of the existing test to the JUnit 5 API . I should enable to use write JUnit 5 tests in addition to the existing ones.

1. We use JUnit rules for starting and stopping the eXist-db server for tests. It is not clear if these would still work in JUnit5, we would have to test, or migrate them to the equivalent JUnit5 approach - https://junit.org/junit5/docs/current/user-guide/#migrating-from-junit4

As my existing experience in other project shows,is that there the existing tests can be run using the JUnit Vintage test engine as stated in the migration guide.

2. We make use of Google's JUnit Toolbox which enables us to run lots of JUnit 4 test methods in parallel. JUnit 4 has something itself built in for parallel execution which we could migrate to, but it is marked as "experimental" - https://junit.org/junit5/docs/current/user-guide/#writing-tests-parallel-execution

I would suggest to leave the existing tests as they are, until we get the tools running also as JUnit 5 test. I will take a look into this task as soon as we have migrated the test execution to the new JUnit test engines though.

3. eXist-db's `XQueryTestRunner` and `XMLTestRunner` which execute XQSuite tests via JUnit uses JUnit 4's Runner APIs which don't exist in JUnit5. Instead we would need to rewrite these substantially to use JUnit5's TestFactory API - https://junit.org/junit5/docs/current/user-guide/#writing-tests-dynamic-tests

Here the same approach applies as the existing JUnit tests.

Now obviously this can all be solved given enough time and resource, but it is going to be quite involved I think. Also, I wonder if now is the right time to move to JUnit 5, as some of the APIs we want to use e.g. parallelism, are marked as experimental.

In my opinion this a good time to do so and may give those experimental parts a try and help the JUnit team to improve there code too.

I am happy for @reinhapa and others to take this forward if they still want to, but personally I probably would not have much time to offer for this as I guess I don't yet see what the benefit of doing so would be?

From my part I'm happy if I get some support according the build infrastructure and the way to run the build locally in a way that matches the build environment in order to capture the build failures locally.

adamretter · 2020-03-28T18:02:55Z

@reinhapa Okay... I understand the scope. What I don't understand is how we can have JUnit4 tests which have a dependency on JUnit 4 jar files (especially the JUnit4 runner), run on the JUnit4 engine (even with the vintage dependency).

reinhapa · 2020-03-28T18:23:36Z

@adamretter the existing JUnit 4 test still need the existing JUnit 4 dependency the execution is done with using the JUnit Vintage test engine dependency, which is a one test engine implementation of the JUnit 5 platform (https://junit.org/junit5/docs/current/user-guide/#overview-what-is-junit-5)

We could implement our own test engine for our XML tests not using any other API than the test engine API itself

reinhapa · 2020-04-07T13:15:23Z

Opened an issue for the current error on junit-team/junit

joewiz · 2020-04-08T20:41:57Z

All 4 linux builds failed on Travis (only the macOS build succeeded) - restarted one linux build to test the waters.

exist-core/src/main/java/org/exist/test/runner/XSuite.java

exist-core/pom.xml

adamretter · 2021-07-14T11:36:24Z

exist-core/src/main/java/org/exist/test/runner/AbstractTestRunner.java

+                }
+            } else {
+                try {
+                    embeddedServer.restart();


This seems like every call to getBrokerPool() will restart the database. This seems like it would be very expensive.

Is it that this method is not really named correctly, or that we don't want to restart the database on every getBrokerPool call?

I need to understand this a bit better...

I can do a try leaving this away...

Seem that when letting this away it leads to non available security manager service though..

dizzzz

Rebase required, after pulling in Adams last PR (which needs to be rebased)

reinhapa · 2021-07-30T13:57:30Z

@dizzzz I will rebase as soon as #3985 is merged too..

exist-core/pom.xml

dizzzz · 2021-08-01T13:28:12Z

exist-core/src/test/java/org/exist/test/runner/AbstractTestRunnerTest.java

@@ -72,6 +71,6 @@ public Description getDescription() {
        };

        final RunNotifier notifier = new RunNotifier();
-        runner.run(notifier);
+        assertDoesNotThrow(() -> runner.run(notifier));


dizzzz

Looks great! Only one compile/test/... scope question

Signed-off-by: Patrick Reinhart <patrick@reini.net>

sonarcloud · 2021-08-01T16:21:51Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

dizzzz requested a review from adamretter March 25, 2020 19:54

dizzzz approved these changes Mar 25, 2020

View reviewed changes

dizzzz closed this Mar 26, 2020

dizzzz reopened this Mar 26, 2020

reinhapa force-pushed the junit5 branch from 4aa6ec1 to 1e97cdb Compare March 26, 2020 21:39

adamretter added the in progress Issue is actively being worked upon label Mar 26, 2020

dizzzz requested changes Mar 28, 2020

View reviewed changes

reinhapa force-pushed the junit5 branch from 9758689 to 53ebeaa Compare April 8, 2020 13:07

reinhapa requested a review from dizzzz April 8, 2020 13:16

dizzzz approved these changes Apr 8, 2020

View reviewed changes

joewiz added enhancement new features, suggestions, etc. and removed in progress Issue is actively being worked upon labels Apr 8, 2020

joewiz added this to the eXist-5.3.0 milestone Apr 8, 2020

reinhapa force-pushed the junit5 branch 2 times, most recently from 3b0f475 to 03fead0 Compare April 11, 2020 07:12

reinhapa marked this pull request as draft April 11, 2020 07:17

reinhapa force-pushed the junit5 branch 3 times, most recently from 58ac2f9 to cd8f2d8 Compare April 13, 2020 14:15

reinhapa force-pushed the junit5 branch 2 times, most recently from 3db5cb5 to 7bff31b Compare July 13, 2021 14:42

reinhapa marked this pull request as ready for review July 13, 2021 14:43

reinhapa force-pushed the junit5 branch 3 times, most recently from 52c7c42 to 19c3053 Compare July 13, 2021 18:03

dizzzz reviewed Jul 14, 2021

View reviewed changes

exist-core/src/main/java/org/exist/test/runner/XSuite.java Outdated Show resolved Hide resolved

dizzzz approved these changes Jul 14, 2021

View reviewed changes

adamretter reviewed Jul 14, 2021

View reviewed changes

exist-core/pom.xml Show resolved Hide resolved

adamretter reviewed Jul 14, 2021

View reviewed changes

exist-core/pom.xml Show resolved Hide resolved

adamretter reviewed Jul 14, 2021

View reviewed changes

reinhapa force-pushed the junit5 branch 3 times, most recently from aa76a7f to 19daf2d Compare July 19, 2021 15:41

adamretter mentioned this pull request Jul 23, 2021

Fix issues with XSuite test descriptions #3985

Merged

reinhapa force-pushed the junit5 branch from 19daf2d to 246f9cb Compare July 26, 2021 18:23

dizzzz requested changes Jul 30, 2021

View reviewed changes

reinhapa force-pushed the junit5 branch from 910012c to b3f982f Compare July 30, 2021 15:59

reinhapa requested a review from dizzzz July 30, 2021 16:02

dizzzz reviewed Aug 1, 2021

View reviewed changes

exist-core/pom.xml Outdated Show resolved Hide resolved

dizzzz reviewed Aug 1, 2021

View reviewed changes

dizzzz approved these changes Aug 1, 2021

View reviewed changes

[feature] add JUnit 5 support

4f410f3

Signed-off-by: Patrick Reinhart <patrick@reini.net>

reinhapa force-pushed the junit5 branch from b3f982f to 4f410f3 Compare August 1, 2021 15:53

adamretter modified the milestones: eXist-6.0.0, eXist-5.3.1 Aug 1, 2021

adamretter merged commit a907406 into eXist-db:develop Aug 2, 2021

reinhapa deleted the junit5 branch August 2, 2021 17:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds JUnit 5 test support #3322

Adds JUnit 5 test support #3322

reinhapa commented Mar 25, 2020

dizzzz commented Mar 25, 2020

dizzzz left a comment

reinhapa commented Mar 25, 2020

dizzzz commented Mar 26, 2020

dizzzz commented Mar 26, 2020 •

edited

dizzzz commented Mar 28, 2020

adamretter commented Mar 28, 2020 •

edited by dizzzz

dizzzz left a comment

reinhapa commented Mar 28, 2020

adamretter commented Mar 28, 2020 •

edited

reinhapa commented Mar 28, 2020

reinhapa commented Apr 7, 2020

joewiz commented Apr 8, 2020

adamretter Jul 14, 2021

reinhapa Jul 14, 2021

reinhapa Jul 15, 2021

dizzzz left a comment

reinhapa commented Jul 30, 2021

dizzzz Aug 1, 2021

dizzzz left a comment

sonarcloud bot commented Aug 1, 2021

Adds JUnit 5 test support #3322

Adds JUnit 5 test support #3322

Conversation

reinhapa commented Mar 25, 2020

dizzzz commented Mar 25, 2020

dizzzz left a comment

Choose a reason for hiding this comment

reinhapa commented Mar 25, 2020

dizzzz commented Mar 26, 2020

dizzzz commented Mar 26, 2020 • edited

dizzzz commented Mar 28, 2020

adamretter commented Mar 28, 2020 • edited by dizzzz

dizzzz left a comment

Choose a reason for hiding this comment

reinhapa commented Mar 28, 2020

adamretter commented Mar 28, 2020 • edited

reinhapa commented Mar 28, 2020

reinhapa commented Apr 7, 2020

joewiz commented Apr 8, 2020

adamretter Jul 14, 2021

Choose a reason for hiding this comment

reinhapa Jul 14, 2021

Choose a reason for hiding this comment

reinhapa Jul 15, 2021

Choose a reason for hiding this comment

dizzzz left a comment

Choose a reason for hiding this comment

reinhapa commented Jul 30, 2021

dizzzz Aug 1, 2021

Choose a reason for hiding this comment

dizzzz left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Aug 1, 2021

dizzzz commented Mar 26, 2020 •

edited

adamretter commented Mar 28, 2020 •

edited by dizzzz

adamretter commented Mar 28, 2020 •

edited