[PATCH] Upgrade to JUnit 5

[Don-vip]

JUnit 5 is now supported by all major IDEs and quite stable. JUnit 4 has not seen any release in the last 4 years, so it's time to upgrade.

As a first step I'll try to run our tests with the "vintage" engine (JUnit 5 running in JUnit 4 compatibility mode), before upgrading the tests.

​https://ant.apache.org/manual/Tasks/junitlauncher.html

[Don-vip]

First problem: ​https://github.com/jacoco/jacoco/issues/673

[Don-vip]

We need this in Ant: ​https://github.com/apache/ant/pull/67

[taylor.smock]

Allow JUnit 5 tests to use JOSMTestRules

[taylor.smock]

I'd like to be able to start using JUnit 5 in plugin tests. The above patch should not have any affect with current tests (since those methods are never called by JUnit 4).

Example usage:

    @RegisterExtension
    public JOSMTestRules test = new JOSMTestRules().projection();

EDIT:
JUnit 5 jars will need to be downloaded to test/lib/junit for ant test

$ curl -O "https://search.maven.org/remotecontent?filepath=org/junit/jupiter/junit-jupiter/5.5.2/junit-jupiter-5.5.2.jar"
$ curl -O "https://search.maven.org/remotecontent?filepath=org/junit/jupiter/junit-jupiter-api/5.5.2/junit-jupiter-api-5.5.2.jar"

[taylor.smock]

Replying to Don-vip:

We need this in Ant: ​https://github.com/apache/ant/pull/67

​https://ant.apache.org/manual/Tasks/junitlauncher.html#fork -- Since Ant 1.10.6 (May 7, 2019)

Replying to Don-vip:

First problem: ​​https://github.com/jacoco/jacoco/issues/673

Apparently not going to be fixed (they really want to keep Java 5 compatibility):

Ant 1.10.x depends on Java 8, while the baseline for JaCoCo is Java 5. So currently we cannot add implementation and tests for it without major hacks in our build. To manage expectations I close this with WONTFIX.

It does look like there is a workaround though (see ​https://github.com/jacoco/jacoco/issues/673#issuecomment-499738017 )

[taylor.smock]

Modify build.xml to use junitlauncher instead of junit. (jacoco output not working, looks like it is a problem with the destfile syntax).

[taylor.smock]

Script to get the required libraries for junitlauncher:

#!/usr/bin/env sh

libraries=(
        # Common
        'https://search.maven.org/remotecontent?filepath=org/junit/platform/junit-platform-commons/1.5.2/junit-platform-commons-1.5.2.jar'
        'https://search.maven.org/remotecontent?filepath=org/junit/platform/junit-platform-engine/1.5.2/junit-platform-engine-1.5.2.jar'
        'https://search.maven.org/remotecontent?filepath=org/junit/platform/junit-platform-launcher/1.5.2/junit-platform-launcher-1.5.2.jar'
        'https://search.maven.org/remotecontent?filepath=org/opentest4j/opentest4j/1.2.0/opentest4j-1.2.0.jar'
        # JUnit 5
        'https://search.maven.org/remotecontent?filepath=org/junit/jupiter/junit-jupiter-api/5.5.2/junit-jupiter-api-5.5.2.jar'
        'https://search.maven.org/remotecontent?filepath=org/junit/jupiter/junit-jupiter-engine/5.5.2/junit-jupiter-engine-5.5.2.jar'
        # Junit 4 (also needs junit.jar from junit4, already there)
        'https://search.maven.org/remotecontent?filepath=org/junit/vintage/junit-vintage-engine/5.5.2/junit-vintage-engine-5.5.2.jar'
        )

for lib in ${libraries[@]}; do curl -O "${lib}"; done

[taylor.smock]

When I said "(jacoco output not working, looks like it is a problem with the destfile syntax)", I was wrong -- I was running the tests under Java 13, where jacoco is explicitly disabled.

That being said, is the jacoco.exec supposed to have coverage information? It doesn't look like it is actually working, with the original build.xml (both the original and modified build.xml give me a coverage of two instructions in all of JOSM with jacoco.exec, and neither produce any other jacoco*.exec files).

[Don-vip]

Yes, jacoco coverage works with Java 8: https://josm.openstreetmap.de/sonar/dashboard?id=josm

[taylor.smock]

I haven't been able to get jacoco coverage working properly in a fresh VM (Fedora 31 with openjdk 1.8.0) with a fresh copy of JOSM (from svn)

What OS/OS version is the test server running?

I've been using ant test-unit -Dosm.username=<+TEST_USER+> -Dosm.password=<+TEST_PASSWORD+> (with a real user/password on the osm sandbox server).

If that isn't the command that leads to the appropriate junit coverage output, please let me know (based off of <echo> statements, testITsuffix is never set, which means that the jacoco.exec file is overwritten by unit/functional/performance tests, but none of them should have 2 lines of coverage).

To test the coverage, I've been loading it into Eclipse, using the JOSM/src as the target directory (I didn't see anything that indicated that it should be a lower directory, e.g. src/org/openstreetmap/josm/)

[taylor.smock]

Add script to get jar files (for easy downloading and testing)

[taylor.smock]

Replying to taylor.smock:

I haven't been able to get jacoco coverage working properly in a fresh VM (Fedora 31 with openjdk 1.8.0) with a fresh copy of JOSM (from svn)

I ran ant test-html -Dosm.username=<+TEST_USER+> -Dosm.password=<+TEST_PASSWORD+> and I got coverage information (using the html instead of jacoco.exec). I also just took a look at the docs for SonarQube coverage ( ​https://docs.sonarqube.org/latest/analysis/coverage/ ), and it looks like sonar.coverage.jacoco.xmlReportPaths is the supported way to get coverage information of java instead of parsing the jacoco.exec file.

It looks like jacoco.exec and jacocoIT.exec are generated on Debian with the original build.xml.

[Don-vip]

I'm pretty sure the jacoco.exec files work as expected.

The SonarQube project configuration is as follows:

Key: sonar.jacoco.reportPaths:
- target/jacoco.exec
- target/jacoco-it.exec

Server is running current Ubuntu LTS.

JOSM Jenkins job is:

ant clean test-clean test-html dist-optimized-report distmac distwin spotbugs checkstyle pmd javadoc taginfo

with JDK8 (from Ubuntu: /usr/lib/jvm/java-8-openjdk-amd64), Ant 1.10.5 (downloaded; I just upgraded manually to Ant 1.10.7) and following properties:

test-it.notRequired=true
test-perf.notRequired=true
osm.username=xxx
osm.password=yyy
glass.platform=Monocle
monocle.platform=Headless
prism.order=sw

Jenkins jacoco configuration is:

path to exec: **/**.exec
inclusions: org/openstreetmap/josm/**/*.class
path to class: build
path to source: src

Then the Jenkins Sonar job runs SonarQube Scanner 3.2.0.1227 (downloaded; I just upgraded manually to 4.2.0.1873) with the same JDK8 and following properties:

# required metadata
sonar.projectKey=josm
sonar.projectName=JOSM
sonar.projectVersion=$SVN_REVISION

# path to source directories (required)
sonar.sources=src,data,images,resources,styles,linux,windows,macosx,scripts
sonar.sourceEncoding=UTF-8
sonar.java.source=1.8

# path to test source directories (optional)
sonar.tests=test/functional,test/performance,test/unit

# path to project binaries (optional), for example directory of Java bytecode
sonar.java.binaries=build
sonar.java.test.binaries=build,test/build/unit,test/build/functional,test/build/performance
sonar.java.test.libraries=test/lib/**/*.jar

sonar.scm.url=scm:svn:http://josm.openstreetmap.de/svn/trunk

# Exclude all COTS and classes in the 'org/openstreetmap/josm/gui/mappaint/mapcss/parsergen' package
sonar.inclusions=src/org/openstreetmap/josm/**/*.java,**/*.groovy,**/*.xml,**/*.xsd,**/*.css,**/*.osm,**/*.mapcss,**/*.properties,**/*.json,**/*.js,**/*.cfg
sonar.exclusions=src/org/openstreetmap/josm/gui/mappaint/mapcss/parsergen/**/*.java,src/org/openstreetmap/josm/data/imagery/types/*.java,data/overpass-turbo-ffs.js,data/overpass-wizard.js,data/validator/opening_hours.js,macosx/JOSM.app/Contents/Info.plist_template.xml

# Plugins
sonar.findbugs.timeout=1200000
sonar.jacoco.reportPath=test/jacoco.exec
sonar.jacoco.itReportPath=test/jacocoIT.exec
sonar.junit.reportsPath=test/report
sonar.scm-stats.period1=365
sonar.scm-stats.period2=90
sonar.scm-stats.period3=7
sonar.trac.url=https://josm.openstreetmap.de
sonar.trac.username.secured=xxx
sonar.trac.password.secured=yyy

This setup is working fine for years.

[taylor.smock]

It seems like something would be broken:

sonar.jacoco.itReportPath=test/jacocoIT.exec
target/jacoco-it.exec

Anyway, thank you for the information. I'll set up a VM for testing with Ubuntu 18.04 LTS, and once I can reproduce the jacoco{,IT}.exec files, I'll apply the patch and see if it works.

If I don't figure out what is going on with the junitlauncher + jacoco:agent ant configuration before 19.12, is there any chance of merging the first patch that added the JUnit 5 compatibility for plugin tests?

[Don-vip]

Replying to taylor.smock:

If I don't figure out what is going on with the junitlauncher + jacoco:agent ant configuration before 19.12, is there any chance of merging the first patch that added the JUnit 5 compatibility for plugin tests?

Sure, I just want to test it first but I have very little time this week.

[taylor.smock]

Replying to Don-vip:

Sure, I just want to test it first but I have very little time this week.

That's fair.

Have a good (and hopefully enjoyable) week.

[taylor.smock]

Output jacoco.exec files (if:set needed to be fixed). jacoco.exec files appear to have information (secondary testing would be nice, since my version of eclipse is refusing to load them). The reports appear to work properly, but the jacoco.exec file is not loading in Eclipse (this hasn't changed from the previous version, AFAICT). Since the jacoco.exec files are used in report generation, the data is there.

[taylor.smock]

@Don-vip: I would appreciate a second opinion on the jacoco.exec files -- the data appears to be there, but I haven't been able to get Eclipse to load it (as a sanity check). The report generation works using the same files, and the reports look OK.

[taylor.smock]

OK. I just ran a comparison between the report from junit 4 and junit 5, and I got the following differences (see the junit_difference.py file, junit4/junit5 reports have to be copied to specific directories):

TEST-org.openstreetmap.josm.gui.mappaint.mapcss.ChildOrParentSelectorTest.xml
report-junit4: {'tests': '7', 'skipped': '4'}
report-junit5: {'tests': '3', 'skipped': '4'}
TEST-org.openstreetmap.josm.actions.JoinAreasActionTest.xml
report-junit4: {'tests': '3', 'skipped': '1'}
report-junit5: {'tests': '2', 'skipped': '1'}
TEST-org.openstreetmap.josm.tools.PlatformHookOsxTest.xml
report-junit4: {'tests': '8', 'skipped': '1'}
report-junit5: {'tests': '8', 'aborted': '1'}
TEST-org.openstreetmap.josm.data.imagery.WMTSTileSourceTest.xml
report-junit4: {'tests': '17', 'skipped': '1'}
report-junit5: {'tests': '16', 'skipped': '1'}
TEST-org.openstreetmap.josm.gui.dialogs.relation.actions.AbstractRelationEditorActionTest.xml
report-junit4: {'tests': '1', 'skipped': '1'}
report-junit5: {}
TEST-org.openstreetmap.josm.gui.layer.gpx.ChooseTrackVisibilityActionTest.xml
report-junit4: {'tests': '1', 'skipped': '1'}
report-junit5: {'skipped': '1'}
TEST-org.openstreetmap.josm.io.MultiFetchServerObjectReaderTest.xml
report-junit4: {'tests': '5', 'skipped': '5'}
report-junit5: {'tests': '5', 'aborted': '5'}
TEST-org.openstreetmap.josm.io.OsmServerBackreferenceReaderTest.xml
report-junit4: {'tests': '6', 'skipped': '6'}
report-junit5: {'tests': '6', 'aborted': '6'}
TEST-org.openstreetmap.josm.tools.OverpassTurboQueryWizardTest.xml
report-junit4: {'tests': '11', 'skipped': '1'}
report-junit5: {'tests': '10', 'skipped': '1'}
TEST-org.openstreetmap.josm.io.audio.AudioPlayerTest.xml
report-junit4: {'tests': '1', 'skipped': '1'}
report-junit5: {'skipped': '1'}
TEST-org.openstreetmap.josm.io.UploadStrategySelectionPanelTest.xml
report-junit4: {'tests': '1', 'skipped': '1'}
report-junit5: {'skipped': '1'}
TEST-org.openstreetmap.josm.data.validation.routines.EmailValidatorTest.xml
report-junit4: {'tests': '22', 'skipped': '1'}
report-junit5: {'tests': '21', 'skipped': '1'}

Beyond that, it looks like JUnit5 removes the "skipped" tests from the "tests" count, so there appears to be no real difference as far as the test outputs go.

That being said, the HTML outputs do have differences, and I'm not certain why probably due to differences in how they handle skipped tests.

JUnit	Tests	Failures	Errors	Skipped	Success rate	Time
4	2168	0	0	24	100.00%	1392.075
5	2156	0	0	11	100.00%	1378.424

So JUnit5 appears to have 2167 tests while JUnit4 has 2168 tests. The single difference is likely from org.openstreetmap.josm.gui.dialogs.relation.actions.AbstractRelationEditorActionTest, which only has skipped tests in JUnit4, which means JUnit5 is not counting them.

[taylor.smock]

Fix an issue where non-unit tests (i.e., functional tests) didn't have JOSMTestRules in their classpath.

[taylor.smock]

Show tests that have different numbers of passed/failed tests

[Don-vip]

Jacoco *.exec file support has just been removed from SonarQube: ​https://jira.sonarsource.com/browse/MMF-1651
I will probably need to change something to load XML reports instead.

[Don-vip]

In 15703/josm:

see #16567 - generate Jacoco CSV and XML reports

[taylor.smock]

Update to ensure patch applies cleanly (see r15703)

[taylor.smock]

Is there anything I need to do?

[Don-vip]

Replying to taylor.smock:

Is there anything I need to do?

No? I still need to find enough time to complete my testing and commit the changes. Sorry, I'm a bit overloaded right now.

[taylor.smock]

Fair enough. I figured I would double check -- I haven't seen any issues in my own testing, but different systems are different, and I'm usually testing on a heavily customized JOSM codebase. Its worked for me(tm) on a clean codebase (with just the patch applied), but sometimes software works differently on different systems.

And I completely understand waiting to find a solid chunk of time for testing this (it is kind of important for the test suite to keep working).

[Don-vip]

Descoping to release 20.03 early to address #18798, as we receive a lot of duplicates.

[taylor.smock]

Update for Ivy dependencies

[Klumbumbus]

Milestone renamed

[taylor.smock]

I've been running svn up; cd core; ant clean dist test for the past month or two on weekdays (in my morning). The only consistent test failure I've seen with this patch (on Mac) is this:

ShortcutTest    testMakeTooltip Failure expected:<[<html>Foo Bar <font size='-2'>(Shift+J)</font>&nbsp;</html>]> but was:<[Foo Bar (⇧+J)]>

And that isn't a true failure, just a case where Shift is replaced by ⇧.

EDIT: I was always checking the html output, which wasn't being regenerated (I needed to be calling ant test-html).

[taylor.smock]

Fix ant test-perf (failing since the jacoco agent was added, even when coverage was disabled)

[simon04]

This patch causes plenty of deprecation warnings…

[taylor.smock]

The deprecation warnings are fairly new. At the time of the original patch, they did not exist.

My original plan was to do a in-situ upgrade to JUnit5, then start switching tests over to using JUnit5 (to remove most dependencies on JUnit4).

In light of the deprecation notices, I was going to still do the "simple" upgrade to JUnit5, but then fix deprecation warnings, and then start switching tests over to using JUnit5.

Note: I was intending on leaving the dependency on JUnit4 (JOSMTestRules) in for a little while, to give plugin authors some time to switch to JUnit5. This is to avoid a situation where multiple plugins have to upgrade all at the same time.

[taylor.smock]

Remove unnecessary modification from JOSMTestRules

[taylor.smock]

Fix (most) deprecation warnings. There is still one in ExpectedRootException and OptionParserTest.

[taylor.smock]

I've gone through and fixed most deprecation warnings.

The 16567.deprecated.1.patch does depend on the 16567.12.patch (where I added new assert methods, I used the JUnit5 API, not the JUnit4 API).

I'm going to have to do more work on ExpectedRootException -- currently only used in LayerManagerTest.java, and maybe some plugins. [EDIT: Not used in any plugins in the `plugins` directory]

I'm also going to have to do some work on OptionParserTest#testMultipleShort -- the current (in-tree) implementation stops at the first parser.parseOptions(Arrays.asList("-ft=arg", "x")), so I have to figure out how to actually run the remainder of the test (note: fairly easy to avoid stopping due to an expected exception, but the other assertions are failing).

[taylor.smock]

ExpectedRootException has been deprecated, and LayerManagerTest has been modified to account for that

[taylor.smock]

All deprecation warnings are fixed, but there is a new failing test. The failing test is OptionParserTest#testMultipleShort. The reason it is failing is that the first parser.parseOpts was throwing an exception. This ensured that the rest of the test was never used.

[taylor.smock]

@simon04: Any advice on how to handle the failing test? I'm inclined to either remove everything after the point where the test initially was stopping, or rewriting the test to pass today using the current code to indicate expected results.

[simon04]

In 16608/josm:

see #16567 - Fix OptionParserTest.testMultipleShort

[taylor.smock]

Update for testMultipleShort changes

[simon04]

In 16617/josm:

fix #16567 - Upgrade to JUnit 5 (patch by taylor.smock)

I applied attachment:16567.12.patch​​

[simon04]

In 16618/josm:

see #16567 - Fix deprecations related to JUnit 5 (patch by taylor.smock)

I applied attachment:16567.deprecated.4.patch​​

[simon04]

Jenkins needs to be updated – https://josm.openstreetmap.de/jenkins/job/JOSM/6529/jdk=JDK8/testReport/org.openstreetmap.josm.gui.layer/TMSLayerTest/testTMSLayer/ only reports

java.lang.reflect.InvocationTargetException

when it should report

ReportedException [thread=Thread[Timeout runner,5,], exception=java.lang.reflect.InvocationTargetException
, methodWarningFrom=null]
	at org.openstreetmap.josm.tools.bugreport.BugReport.intercept(BugReport.java:173)
	at org.openstreetmap.josm.gui.util.GuiHelper.runInEDTAndWaitWithException(GuiHelper.java:245)
	at org.openstreetmap.josm.gui.layer.LayerManager.addLayer(LayerManager.java:217)
	at org.openstreetmap.josm.gui.layer.LayerManager.addLayer(LayerManager.java:206)
	at org.openstreetmap.josm.gui.layer.TMSLayerTest.test(TMSLayerTest.java:53)
	at org.openstreetmap.josm.gui.layer.TMSLayerTest.testTMSLayer(TMSLayerTest.java:66)
	at org.openstreetmap.josm.testutils.JOSMTestRules$TimeoutThread.run(JOSMTestRules.java:756)
Caused by: java.lang.reflect.InvocationTargetException
	at java.desktop/java.awt.EventQueue.invokeAndWait(EventQueue.java:1367)
	at java.desktop/java.awt.EventQueue.invokeAndWait(EventQueue.java:1342)
	at java.desktop/javax.swing.SwingUtilities.invokeAndWait(SwingUtilities.java:1480)
	at org.openstreetmap.josm.gui.util.GuiHelper.runInEDTAndWaitWithException(GuiHelper.java:243)
	... 5 more
Caused by: java.lang.NullPointerException
	at org.openstreetmap.josm.gui.layer.imagery.TileCoordinateConverter.getScaleFactor(TileCoordinateConverter.java:186)
	at org.openstreetmap.josm.gui.layer.AbstractTileSourceLayer.getScaleFactor(AbstractTileSourceLayer.java:359)
	at org.openstreetmap.josm.gui.layer.AbstractTileSourceLayer.getBestZoom(AbstractTileSourceLayer.java:370)
	at org.openstreetmap.josm.gui.layer.AbstractTileSourceLayer.initTileSource(AbstractTileSourceLayer.java:281)
	at org.openstreetmap.josm.gui.layer.AbstractTileSourceLayer.initializeIfRequired(AbstractTileSourceLayer.java:579)
	at org.openstreetmap.josm.gui.layer.AbstractTileSourceLayer.attachToMapView(AbstractTileSourceLayer.java:555)
	at org.openstreetmap.josm.gui.MapView.layerAdded(MapView.java:342)
	at org.openstreetmap.josm.gui.layer.LayerManager.fireLayerAdded(LayerManager.java:458)
	at org.openstreetmap.josm.gui.layer.LayerManager.realAddLayer(LayerManager.java:233)
	at org.openstreetmap.josm.gui.layer.MainLayerManager.realAddLayer(MainLayerManager.java:284)
	at org.openstreetmap.josm.gui.layer.LayerManager.lambda$addLayer$0(LayerManager.java:217)
	at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:306)
	at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:770)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:715)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:704)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
	at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:740)
	at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

[simon04]

In 16633/josm:

see #16567 - Fix deprecations related to JUnit 5 (in HttpClientTest)

[taylor.smock]

Ensure that after is not run until after the tests complete

[taylor.smock]

I just started porting over the MapWithAI unit tests over to JUnit5. In the process, I discovered that after was being called before the tests were actually run.

I'm currently tracking down a StackOverflow bug as well, but that might just be code that I'm using in the plugin.

[taylor.smock]

Add -Djunit.jupiter.extensions.autodetection.enabled=true as a JVM argument. This automatically registers the JMockit JUnit5 extension, which clears mocks between tests (otherwise, you can have a ton of mocks for the same code)

[simon04]

In 16658/josm:

see #16567 - JUnit 5: call JOSMTestRules.after (patch by taylor.smock)

[simon04]

In 16659/josm:

see #16567 - JUnit 5: set -Djunit.jupiter.extensions.autodetection.enabled=true to clear JMockit mocks (patch by taylor.smock)

[taylor.smock]

Replying to simon04:

In 16659/josm:

see #16567 - JUnit 5: set -Djunit.jupiter.extensions.autodetection.enabled=true to clear JMockit mocks (patch by taylor.smock)

#18200 is going to be a blocker for moving completely to JUnit5. The -Djunit.jupiter.extensions.autodetection.enabled=true is only relevant for v1.46+ of JMockit, and we are currently using v1.44. The JMockit changelog claims that it Added support for JUnit 5.4.0. However, in my testing, I found that 1.47+ worked, while 1.46 did not actually remove the mockers when Djunit.jupiter.extensions.autodetection.enabled=true.

[Klumbumbus]

Probably changes here broke the imagery integration test on jenkins: https://josm.openstreetmap.de/jenkins/job/JOSM-Imagery-Integration/1700/

https://josm.openstreetmap.de/log/josm/?action=stop_on_copy&mode=stop_on_copy&rev=16619&stop_rev=16614&limit=100&verbose=on&sfp_email=&sfph_mail=

[simon04]

Yes, very likely: Running …

$ ant -Dtest-perf.notRequired=true -Dtest.notRequired=true '-Ddefault-junitIT-includes=**/ImageryPreferenceTestIT.class' test-clean test-html

yields

$ cat test/report/TEST-org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT.txt
Testcase: org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT
Test: testImageryEntryValidity[cyclosm] took 1 sec(s)
Test: testImageryEntryValidity[osm-cambodia_laos_thailand_vietnam-bilingual] took 1 sec(s)
Test: testImageryEntryValidity[Bing] took 2 sec(s)
Tests run: 3, Failures: 0, Skipped: 0, Aborted: 0, Time elapsed: 4 sec(s)

but an empty

$ cat test/report/TEST-org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT.xml
<?xml version="1.0"?>
<testsuite name="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" time="4.466" timestamp="2020-06-21T08:14:17" tests="3" failures="0" skipped="0" aborted="0">
  <properties><!-- ... --></properties>
  <!-- here should be the test results!!! -->
</testsuite>

---

For testing, I restricted the imagery tests to 3:

test/unit/org/openstreetmap/josm/gui/preferences/imagery/ImageryPreferenceTestIT.java

@@ -128 +128 @@
         ImageryLayerInfo.instance.load(false);
         return ImageryLayerInfo.instance.getDefaultLayers()
                 .stream()
+                .limit(3)
                 .map(x -> new Object[] {x.getId(), x})
                 .collect(Collectors.toList());
     }

---

The error might be related to org.apache.tools.ant.taskdefs.optional.junitlauncher.LegacyXmlResultFormatter not correctly reporting parameterized tests…
Now we're in the middle of JUnit 4 tests, JUnit 5 test runner, Ant test reporter :-(

[simon04]

Maybe using the ConsoleLauncher from JUnit 5 resolves this issue? – ​https://github.com/junit-team/junit5-samples/blob/d435084257338103436e240a1dd8cbadd815fdf9/junit5-jupiter-starter-ant/build.xml#L49-L61

[taylor.smock]

Implement AfterAllCallback and BeforeAllCallback for instances where there is a static JOSMTestRules, modify ImageryPreferenceTestIT to be a full JUnit5 test, add junit-jupiter-params as an Ivy dependency

[taylor.smock]

There is some odd behavior when running the parameterized test in Eclipse. The test likes to be collapsed except when running another parameterized test, or one of the parameters has caused a failure. So for the first x tests, it is collapsing/expanding on every parameter.

In any case, I think I've fixed the issue, but I am waiting on the test to actually complete (I didn't limit it to 3 imagery layers). It will probably finish in 30-80 minutes (full test runs on my home machine takes ~86 minutes).

[taylor.smock]

Replying to taylor.smock:

In any case, I think I've fixed the issue, but I am waiting on the test to actually complete (I didn't limit it to 3 imagery layers). It will probably finish in 30-80 minutes (full test runs on my home machine takes ~86 minutes).

OK. It took 90 minutes (I think I may need to look into @Timeout if we want to decrease test run time, but we already allow it to take 40 minutes), but there is data in the xml file, and there is data in the txt file. The xml looks good (note: I ran it through xmllint --format and edited for brevity)

<?xml version="1.0" ?><testsuite name="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" time="5397.281" timestamp="2020-06-21T12:41:33" tests="887" failures="57" skipped="0" aborted="29">
<.../>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[193]" time="0.533"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[700]" time="8.179"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[833]" time="3.198"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[506]" time="0.831"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[639]" time="0.971"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[748]" time="2.493"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[615]" time="0.631">
    <aborted message="Assumption failed: {IE={OSMIE Townlands[IE] ('Ireland) - https://tile.openstreetmap.ie/townland/{zoom}/{x}/{y}.png - TMS=[https://tile.openstreetmap.ie/townland/7/61/41.png -&gt; java.net.ConnectException: Connection refused (Connection refused), https://tile.openstreetmap.ie/townland/12/1956/1326.png -&gt; java.net.ConnectException: Connection refused (Connection refused)]}}" type="org.opentest4j.TestAbortedException"><![CDATA[org.opentest4j.TestAbortedException: Assumption failed: {IE={OSMIE Townlands[IE] ('Ireland) - https://tile.openstreetmap.ie/townland/{zoom}/{x}/{y}.png - TMS=[https://tile.openstreetmap.ie/townland/7/61/41.png -> java.net.ConnectException: Connection refused (Connection refused), https://tile.openstreetmap.ie/townland/12/1956/1326.png -> java.net.ConnectException: Connection refused (Connection refused)]}}
    ...stacktrace...
    </aborted>
  </testcase>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[91]" time="5.099"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[724]" time="8.458"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[857]" time="0.854"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[192]" time="1.305"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[723]" time="7.297"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[832]" time="0.947"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[505]" time="0.825"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[529]" time="0.82"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[638]" time="0.807"/>
  <testcase classname="org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT" name="testImageryEntryValidity(String, ImageryInfo)[747]" time="120.349">
    <failure message="{PL={Przemy&#x15B;l: Ortophotomap (aerial image)[PL] ('Poland) - http://przemysl.geoportal2.pl/map/geoportal/wms.php?FORMAT=image/jpeg&amp;VERSION=1.1.1&amp;SERVICE=WMS&amp;REQUEST=GetMap&amp;LAYERS=ortofotomapa&amp;STYLES=&amp;SRS={proj}&amp;WIDTH={width}&amp;HEIGHT={height}&amp;BBOX={bbox} - WMS=[http://przemysl.geoportal2.pl/map/geoportal/wms.php?FORMAT=image/jpeg&amp;VERSION=1.1.1&amp;SERVICE=WMS&amp;REQUEST=GetMap&amp;LAYERS=ortofotomapa&amp;STYLES=&amp;SRS=EPSG:2179&amp;WIDTH=512&amp;HEIGHT=512&amp;BBOX=8390252.4918224,-33995034.6879584,48465269.1774009,6079981.9976201 -&gt; java.net.SocketTimeoutException: connect timed out, http://przemysl.geoportal2.pl/map/geoportal/wms.php?FORMAT=image/jpeg&amp;VERSION=1.1.1&amp;SERVICE=WMS&amp;REQUEST=GetMap&amp;LAYERS=ortofotomapa&amp;STYLES=&amp;SRS=EPSG:2179&amp;WIDTH=512&amp;HEIGHT=512&amp;BBOX=8409820.3710634,5512513.4996309,8429388.2503044,5532081.3788719 -&gt; java.net.SocketTimeoutException: connect timed out]}} ==&gt; expected: &lt;true&gt; but was: &lt;false&gt;" type="org.opentest4j.AssertionFailedError"><![CDATA[org.opentest4j.AssertionFailedError: {PL={Przemy�~[l: Ortophotomap (aerial image)[PL] ('Poland) - http://przemysl.geoportal2.pl/map/geoportal/wms.php?FORMAT=image/jpeg&VERSION=1.1.1&SERVICE=WMS&REQUEST=GetMap&LAYERS=ortofotomapa&STYLES=&SRS={proj}&WIDTH={width}&HEIGHT={height}&BBOX={bbox} - WMS=[http://przemysl.geoportal2.pl/map/geoportal/wms.php?FORMAT=image/jpeg&VERSION=1.1.1&SERVICE=WMS&REQUEST=GetMap&LAYERS=ortofotomapa&STYLES=&SRS=EPSG:2179&WIDTH=512&HEIGHT=512&BBOX=8390252.4918224,-33995034.6879584,48465269.1774009,6079981.9976201 -> java.net.SocketTimeoutException: connect timed out, http://przemysl.geoportal2.pl/map/geoportal/wms.php?FORMAT=image/jpeg&VERSION=1.1.1&SERVICE=WMS&REQUEST=GetMap&LAYERS=ortofotomapa&STYLES=&SRS=EPSG:2179&WIDTH=512&HEIGHT=512&BBOX=8409820.3710634,5512513.4996309,8429388.2503044,5532081.3788719 -> java.net.SocketTimeoutException: connect timed out]}} ==> expected: <true> but was: <false>
... More output ...

[simon04]

@taylor.smock, thanks for looking into this issue!

The test names are not yet correct – tests are reported as name="testImageryEntryValidity(String, ImageryInfo)[193]" when they used to be name="testImageryEntryValidity[Berlin-2018]", see https://josm.openstreetmap.de/jenkins/job/JOSM-Imagery-Integration/1699/testReport/

[taylor.smock]

It turns out this is not supported (yet). Apparently the JUnit team has been planning to create a new test format that supports all of the new JUnit5 features. I would have expected parameterized tests to continue working as they had in JUnit4. I have a parameterized test in MapWithAI, and it works ok in Eclipse (like this one). I had never bothered looking at the xml output, so I didn't realize that it was (effectively) useless.

We could (theoretically) write our own test writer, but I really don't want to do that. Or extend the XML writer to write what we actually want it to write.

Links, so I don't forget them:

​https://github.com/junit-team/junit5/issues/373
​https://github.com/ota4j-team/opentest4j/issues/9
​https://junit.org/junit5/docs/current/user-guide/#launcher-api-listeners-custom (if we want to roll our own...)

[simon04]

In 16727/josm:

fix #16567 - Fix ImageryPreferenceTestIT for JUnit 5 (patch by taylor.smock)

[simon04]

@taylor.smock, thank you. I'm a bit puzzled that there are so many pitfalls (considering that JUnit has been released in ​2017-09)

I've submitted a bug in the Ant Bugzilla: Bug ​64564 – JUnitLauncher Task should support @ParameterizedTest (custom display name)

[Klumbumbus]

In the last time I already had the impression that the output of tests on jenkins is reduced. Now there is one test without any output at all, which is bad: https://josm.openstreetmap.de/jenkins/job/JOSM-Integration/lastCompletedBuild/jdk=JDK8/testReport/org.openstreetmap.josm.data.validation.routines/DomainValidatorTestIT/testIanaTldList/

[taylor.smock]

Replying to simon04:

@taylor.smock, thank you. I'm a bit puzzled that there are so many pitfalls (considering that JUnit has been released in ​2017-09)

I've submitted a bug in the Ant Bugzilla: Bug ​64564 – JUnitLauncher Task should support @ParameterizedTest (custom display name)

It has surprised me too. And not in a good way. I had been hoping that no major issues would crop up, but I was (a) not testing properly (i.e., not looking at the xml and txt files) and (b) assuming that JUnit 5 would have feature parity (at least!).

Replying to Klumbumbus:

In the last time I already had the impression that the output of tests on jenkins is reduced. Now there is one test without any output at all, which is bad: https://josm.openstreetmap.de/jenkins/job/JOSM-Integration/lastCompletedBuild/jdk=JDK8/testReport/org.openstreetmap.josm.data.validation.routines/DomainValidatorTestIT/testIanaTldList/

I've got a fix for this specific issue. I'll post that here in a minute or two.

[taylor.smock]

Use Logging.error + Logging.getLastErrorAndWarnings() to show output when JUnit 5 doesn't show system errors. Also use JUnit 5 Assertions instead of JUnit 4 Assert.

[Klumbumbus]

Another one without output: https://josm.openstreetmap.de/jenkins/job/JOSM-Integration/lastCompletedBuild/jdk=JDK8/testReport/org.openstreetmap.josm.gui.preferences.validator/ValidatorTagCheckerRulesPreferenceTestIT/testValidityOfAvailableRules/

[Klumbumbus]

Replying to simon04:

In 16727/josm:

fix #16567 - Fix ImageryPreferenceTestIT for JUnit 5 (patch by taylor.smock)

It is not yet fully fixed. The overview at https://josm.openstreetmap.de/jenkins/job/JOSM-Imagery-Integration/lastCompletedBuild/testReport/ should display the imagery name instead of "String, ImageryInfo".

[taylor.smock]

Replying to Klumbumbus:

Replying to simon04:

In 16727/josm:

fix #16567 - Fix ImageryPreferenceTestIT for JUnit 5 (patch by taylor.smock)

It is not yet fully fixed. The overview at https://josm.openstreetmap.de/jenkins/job/JOSM-Imagery-Integration/lastCompletedBuild/testReport/ should display the imagery name instead of "String, ImageryInfo".

That would be ideal. Unfortunately, that is (currently) the way that JUnit5 reports the test (see comment:53). That particular bug hasn't had any further activity on the ant bug tracker, but I have no idea how long it usually takes them to look at a bug and respond.

My patch, TBH, is a workaround so that we can at least figure out what the failing imagery was, although we have to actually read the logs. I, quite frankly, didn't expect JUnit5 to have a regression like this, and I didn't look at the parameterized tests outside of Eclipse (which does show parameters, albeit not the same way that JUnit4 shows parameters).

[simon04]

In 16765/josm:

see #16567 - Fix DomainValidatorTestIT for JUnit 5 (patch by taylor.smock)

[simon04]

Replying to simon04:

@taylor.smock, thank you. I'm a bit puzzled that there are so many pitfalls (considering that JUnit has been released in ​2017-09)

I've submitted a bug in the Ant Bugzilla: Bug ​64564 – JUnitLauncher Task should support @ParameterizedTest (custom display name)

I found a pull request resolving this issue: ​https://github.com/apache/ant/pull/121 Unfortunately it is pending since 2020-01…

[simon04]

In 17135/josm:

see #16567 - Update to JUnit 5.7.0

CHANGELOG: ​https://junit.org/junit5/docs/5.7.0/release-notes/

[Don-vip]

@Taylor I'm writing a simple test:

// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.gui.dialogs;

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.Date;

import javax.swing.JLabel;
import javax.swing.JList;

import org.junit.Rule;
import org.junit.jupiter.api.Test;
import org.openstreetmap.josm.data.coor.LatLon;
import org.openstreetmap.josm.data.notes.Note;
import org.openstreetmap.josm.data.notes.NoteComment;
import org.openstreetmap.josm.data.osm.User;
import org.openstreetmap.josm.gui.dialogs.NotesDialog.NoteRenderer;
import org.openstreetmap.josm.testutils.JOSMTestRules;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

/**
 * Unit tests of {@link NotesDialog}
 */
class NotesDialogTest {

    /**
     * Setup tests
     */
    @Rule
    @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
    public JOSMTestRules josmTestRules = new JOSMTestRules().preferences();

    @Test
    void testMultiLineNoteRendering() {
        Note note = new Note(LatLon.ZERO);
        note.setCreatedAt(new Date());
        note.addComment(new NoteComment(new Date(), User.createLocalUser(null), "foo\nbar\n\nbaz", null, false));
        assertEquals("foo; bar; baz",
                ((JLabel) new NoteRenderer().getListCellRendererComponent(new JList<>(), note, 0, false, false)).getText());
    }
}

which fails because the return value of "Config.getPref()" is null

java.lang.ExceptionInInitializerError
	at org.openstreetmap.josm.gui.dialogs.NotesDialog$NoteRenderer.getListCellRendererComponent(NotesDialog.java:262)
	at org.openstreetmap.josm.gui.dialogs.NotesDialogTest.testMultiLineNoteRendering(NotesDialogTest.java:40)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:686)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
	at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:205)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:201)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:137)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:71)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:248)
	at org.junit.platform.launcher.core.DefaultLauncher.lambda$execute$5(DefaultLauncher.java:211)
	at org.junit.platform.launcher.core.DefaultLauncher.withInterceptedStreams(DefaultLauncher.java:226)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:199)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:141)
	at org.eclipse.jdt.internal.junit5.runner.JUnit5TestReference.run(JUnit5TestReference.java:98)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:542)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:770)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:464)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)
Caused by: java.lang.NullPointerException: Cannot invoke "org.openstreetmap.josm.spi.preferences.IPreferences.getInt(String, int)" because the return value of "org.openstreetmap.josm.spi.preferences.Config.getPref()" is null
	at org.openstreetmap.josm.tools.ImageProvider$ImageSizes.<clinit>(ImageProvider.java:121)
	... 67 more

In debug I can see that JOSMTestRules.preferences() is called, but not JOSMTestRules.before().

Any idea what's wrong?

[taylor.smock]

Replying to Don-vip:

@Taylor I'm writing a simple test:

I don't (currently) have an idea as to what is wrong. I'll have to step through it tomorrow.

When I converted JOSMTestRules to work with JUnit5, I took care to not modify any existing code, IIRC. I think the only thing I really changed was adding a junit5 boolean, which conditionally executed after(), and the junit5 boolean shouldn't be touched by your code. Just out of curiosity, what happens when you use the JUnit 5 equivalents?

[Don-vip]

Replying to taylor.smock:

Just out of curiosity, what happens when you use the JUnit 5 equivalents?

I didn't try yet, what's the JUnit5 syntax to use?

[simon04]

Replying to Don-vip:

I didn't try yet, what's the JUnit5 syntax to use?

org.junit.jupiter.api.extension.RegisterExtension, see org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT for instance.

[taylor.smock]

Replying to Don-vip:

Replying to taylor.smock:

Just out of curiosity, what happens when you use the JUnit 5 equivalents?

I didn't try yet, what's the JUnit5 syntax to use?

// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.gui.dialogs;

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.Date;

import javax.swing.JLabel;
import javax.swing.JList;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.openstreetmap.josm.data.coor.LatLon;
import org.openstreetmap.josm.data.notes.Note;
import org.openstreetmap.josm.data.notes.NoteComment;
import org.openstreetmap.josm.data.osm.User;
import org.openstreetmap.josm.gui.dialogs.NotesDialog.NoteRenderer;
import org.openstreetmap.josm.testutils.JOSMTestRules;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

/**
 * Unit tests of {@link NotesDialog}
 */
class NotesDialogTest {

    /**
     * Setup tests
     */
    @RegisterExtension
    @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
    public JOSMTestRules josmTestRules = new JOSMTestRules().preferences();

    @Test
    void testMultiLineNoteRendering() {
        Note note = new Note(LatLon.ZERO);
        note.setCreatedAt(new Date());
        note.addComment(new NoteComment(new Date(), User.createLocalUser(null), "foo\nbar\n\nbaz", null, false));
        assertEquals("foo; bar; baz",
                ((JLabel) new NoteRenderer().getListCellRendererComponent(new JList<>(), note, 0, false, false)).getText());
    }
}

OK. I think I see what the problem was. @Rule isn't used by JUnit5, which means the code was never executed. I skimmed the code last night and missed that you were using the @Test annotation from JUnit5.

[Don-vip]

Replying to taylor.smock:

OK. I think I see what the problem was. @Rule isn't used by JUnit5, which means the code was never executed. I skimmed the code last night and missed that you were using the @Test annotation from JUnit5.

Ah ok we must either use only JUnit4 annotations or JUnit 5 ones, not a mix of them, right?

[taylor.smock]

Replying to Don-vip:

Replying to taylor.smock:

OK. I think I see what the problem was. @Rule isn't used by JUnit5, which means the code was never executed. I skimmed the code last night and missed that you were using the @Test annotation from JUnit5.

Ah ok we must either use only JUnit4 annotations or JUnit 5 ones, not a mix of them, right?

I'd stick with that. You can probably do something like this:

...
import org.junit.Rule;
import org.junit.jupiter.api.extension.RegisterExtension;
...
@Rule
@RegisterExtension
public JOSM TestRules josmTestRules = new JOSMTestRules().preferences();
@org.junit.Test
public void test4() {
...
}
@org.junit.jupiter.api.Test
void test5() {
...
}

But I think we should probably stick with one or the other per test file.

[Don-vip]

In 17275/josm:

see #16567 - upgrade almost all tests to JUnit 5, except those depending on WiremockRule

See ​https://github.com/tomakehurst/wiremock/issues/684

[Don-vip]

In 17276/josm:

see #16567 - fix obvious test errors + upgrade tests dependencies

[Don-vip]

In 17277/josm:

see #16567 - fix more obvious test errors

[Don-vip]

I don't understand the errors in tests using JMockit @Injectable and @Mocked annotations. In Eclipse, the tests run OK if I add the -javaagent parameter, as required by JMockit. I can reproduce the error when I remove the -javaagent argument, but we already have it in the build.xml, so I'm kind of lost here... Any clue someone?

[Don-vip]

In 17279/josm:

see #16567 - revert r17275 for CycleLayerActionTest / MinimapDialogTest

They depend on fakeImagery, which does not work yet with JUnit 5

[Don-vip]

In 17280/josm:

see #16567 - fix assertEquals parameters order

[GerdP]

What do I have to change in Eclipse to run unit tests?

[taylor.smock]

Replying to Don-vip:

I don't understand the errors in tests using JMockit @Injectable and @Mocked annotations. In Eclipse, the tests run OK if I add the -javaagent parameter, as required by JMockit. I can reproduce the error when I remove the -javaagent argument, but we already have it in the build.xml, so I'm kind of lost here... Any clue someone?

Which tests are failing?
I just ran ant test-html on Mac (took a little while), and the following tests had issues:

• MapCSSRendererTest (looks like UI differences, probably not an issue)
• ImageryPreferenceTestIT (I'm ignoring these)
• AuthenticationPreferenceTest (java.lang.NoClassDefFoundError, didn't see any mocks, from test/unit/org/openstreetmap/josm/gui/preferences/server/ServerAccessPreferenceTest.java)
• OverpassServerPreferenceTest (NoClassDefFoundError again, didn't see any mocks, from test/unit/org/openstreetmap/josm/gui/preferences/server/ServerAccessPreferenceTest.java)

[taylor.smock]

Replying to GerdP:

What do I have to change in Eclipse to run unit tests?

1) Change the test launcher to JUnit5

a) Down arrow next to Debug/Run/Coverage
b) Select {Debug,Run,Coverage} Configurations
c) Select the JUnit test(s) that you want to run under JUnit5
d) Change "Test runner" from "Junit 4" to "JUnit 5"
e) Apply

2) Add JUnit 5 to the classpath (may already be done)

a) Right click the JOSM project
b) Build Path -> Configure Build Path
c) Libraries
d) Remove JUnit 4
e) Add Library
f) JUnit
g) Set "JUnit 5" as the version
h) Finish and apply

Hopefully I didn't miss anything. :)

[GerdP]

My Eclipse doesn't know JUnit 5. It offers 3 and 4.

[taylor.smock]

Replying to GerdP:

My Eclipse doesn't know JUnit 5. It offers 3 and 4.

What version of Eclipse are you running? (Also, you can ping me on IRC if you want)

[GerdP]

Version: Neon.3 Release (4.6.3)
Build id: 20170314-1500

Don't know how to use IRC. Sorry for my ignorance.

[taylor.smock]

Replying to GerdP:

Version: Neon.3 Release (4.6.3)
Build id: 20170314-1500

Don't know how to use IRC. Sorry for my ignorance.

Don't worry about not knowing how to use IRC. I was just putting it out there to avoid spamming other people's inboxes.

You should update Eclipse. It looks like JUnit5 support was introduced in 4.7.1a ( see ​https://www.eclipse.org/community/eclipse_newsletter/2017/october/article5.php ).

[GerdP]

OK, let's see where that takes me...

[Don-vip]

@Gerd I waited three years after initial Eclipse support, it's time to update :D There's a new version of Eclipse every three months. Once you have a recent version, there is nothing special to do. Eclipse selects the correct JUnit version by itself.

[Don-vip]

Replying to taylor.smock:

Which tests are failing?

You can check on Jenkins:

• With Java 11, everything's fine. No error
• With Java 8, all tests involving JMockit injected arguments fail with message "Failed to resolve parameter..." I guess we should open a bug report to JMockit, but it's difficult to do so with the library author :(

[GerdP]

Replying to Don-vip:

@Gerd I waited three years after initial Eclipse support, it's time to update :D There's a new version of Eclipse every three months. Once you have a recent version, there is nothing special to do. Eclipse selects the correct JUnit version by itself.

Well, the latest Eclipse version requires JDK 11. Not sure how to set up everything without risking to produce binaries which don't work on JRE8. On my laptop I use a newer version of Eclipse that supports JUnit5, but it it not able to compile JOSM several tests.
No I have to configure my new Eclipse Version: 2020-09 (4.17.0) ...

[taylor.smock]

Replying to Don-vip:

You can check on Jenkins:

• With Java 11, everything's fine. No error
• With Java 8, all tests involving JMockit injected arguments fail with message "Failed to resolve parameter..." I guess we should open a bug report to JMockit, but it's difficult to do so with the library author :(

OK. I ran the tests locally with Java 11. I'll start looking at Java 8. I thought I had some tests in MapWithAI that were using mockers, and I do, but I haven't switched them over to JUnit 5 yet.

Anyway, on Mac I just ran JAVA_HOME=$(/usr/libexec/java_home -v 1.8) ant test-clean test '-Ddefault-junit-includes=**/PlatformHookWindowsTest.class' -Dglass.platform=Monocle -Dmonocle.platform=Headless -Dprism.order=sw; cat test/report/TEST-org.openstreetmap.josm.tools.PlatformHookWindowsTest.txt, which output

Testcase: org.openstreetmap.josm.tools.PlatformHookWindowsTest
Test: testGetRootKeystore() took 1 sec(s)
Test: testStartupHook() took 494 milli sec(s)
Test: testAfterPrefStartupHook() took 642 milli sec(s)
Test: testGetDefaultPrefDirectory() took 492 milli sec(s)
Test: testOpenUrlSuccess(Desktop) took 788 milli sec(s)
Test: testGetInstalledFonts() took 510 milli sec(s)
Test: testInitSystemShortcuts() took 598 milli sec(s)
Test: testGetDefaultCacheDirectory() took 656 milli sec(s)
Test: testOpenUrlFallback(Desktop, Runtime) took 917 milli sec(s)
Test: testGetDefaultStyle() took 536 milli sec(s)
Test: testGetOSDescription() took 593 milli sec(s)
Test: testGetAdditionalFonts() took 508 milli sec(s)
Tests run: 12, Failures: 0, Skipped: 0, Aborted: 0, Time elapsed: 8 sec(s)

Are we using Ubuntu 18.04, or have we updated to Ubuntu 20.04 for CI?

[Don-vip]

Replying to taylor.smock:

Are we using Ubuntu 18.04, or have we updated to Ubuntu 20.04 for CI?

We're running Ubuntu 18.04.5 LTS up-to-date. Yesterday I updated these packages and needed to commit r17278:

• openjdk-11-jre:amd64 (11.0.9+11-0ubuntu1~18.04.1) over (11.0.8+10-0ubuntu1~18.04.1)
• openjdk-8-jre:amd64 (8u272-b10-0ubuntu1~18.04) over (8u265-b01-0ubuntu2~18.04)

[Don-vip]

Replying to GerdP:

Well, the latest Eclipse version requires JDK 11. Not sure how to set up everything without risking to produce binaries which don't work on JRE8.

You can setup as many JDK as you want. You need Java 11 to run Eclipse but you can setup Java 8 as default JDK in compiler settings.

[taylor.smock]

Replying to Don-vip:

We're running Ubuntu 18.04.5 LTS up-to-date. Yesterday I updated these packages and needed to commit r17278:

• openjdk-11-jre:amd64 (11.0.9+11-0ubuntu1~18.04.1) over (11.0.8+10-0ubuntu1~18.04.1)
• openjdk-8-jre:amd64 (8u272-b10-0ubuntu1~18.04) over (8u265-b01-0ubuntu2~18.04)

Well, it doesn't happen on 20.04, so I'll have to make a VM Of 18.04 for testing.

[Don-vip]

In 17288/josm:

see #16567 - fix integration tests

[Don-vip]

In 17304/josm:

see #16567 - use our own patched version of JMockit to see if it solves the JUnit 5/Java 8 test failures

​https://github.com/JOSM/jmockit1/releases/tag/v1.49.a manually uploaded to https://josm.openstreetmap.de/nexus/

[Don-vip]

More details in text file:

Test: testSortData(Collections) took 121 milli sec(s) FAILED: Failed to resolve parameter [java.util.Collections arg0] in method [void org.openstreetmap.josm.data.ImageDataTest.testSortData(java.util.Collections)]: call site initialization exception
org.junit.jupiter.api.extension.ParameterResolutionException: Failed to resolve parameter [java.util.Collections arg0] in method [void org.openstreetmap.josm.data.ImageDataTest.testSortData(java.util.Collections)]: call site initialization exception
        at org.apache.tools.ant.taskdefs.optional.junitlauncher.LauncherSupport.launch(LauncherSupport.java:141)
        at org.apache.tools.ant.taskdefs.optional.junitlauncher.StandaloneLauncher.main(StandaloneLauncher.java:114)
Caused by: java.lang.BootstrapMethodError: call site initialization exception
        at java.lang.invoke.CallSite.makeSite(CallSite.java:341)
        at java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:307)
        at java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:297)
        ... 2 more
Caused by: java.lang.VerifyError: Bad local variable type
Exception Details:
  Location:
    org/junit/jupiter/engine/execution/ExecutableInvoker$$Lambda$1475.get$Lambda()Ljava/util/function/Predicate; @4: aload_0
  Reason:
    Type top (current frame, locals[0]) is not assignable to reference type
  Current Frame:
    bci: @4
    flags: { }
    locals: { }
    stack: { uninitialized 0, uninitialized 0 }
  Bytecode:
    0x0000000: bb00 0259 2a2b b700 15b0

        at sun.misc.Unsafe.defineAnonymousClass(Native Method)
        at java.lang.invoke.InnerClassLambdaMetafactory.spinInnerClass(InnerClassLambdaMetafactory.java:326)
        at java.lang.invoke.InnerClassLambdaMetafactory.buildCallSite(InnerClassLambdaMetafactory.java:194)
        at java.lang.invoke.LambdaMetafactory.metafactory(LambdaMetafactory.java:304)
        at java.lang.invoke.CallSite.makeSite(CallSite.java:302)
        ... 4 more

[Don-vip]

In Eclipse I can reproduce with a similar error, but not exactly the same:

Caused by: java.lang.VerifyError: Bad local variable type
Exception Details:
  Location:
    org/junit/jupiter/engine/descriptor/TestMethodTestDescriptor$$Lambda$280.<init>()V @5: aload_1
  Reason:
    Type top (current frame, locals[1]) is not assignable to reference type
  Current Frame:
    bci: @5
    flags: { }
    locals: { 'org/junit/jupiter/engine/descriptor/TestMethodTestDescriptor$$Lambda$280' }
    stack: { 'org/junit/jupiter/engine/descriptor/TestMethodTestDescriptor$$Lambda$280' }
  Bytecode:
    0x0000000: 2ab7 0010 2a2b b500 122a 2cb5 0014 2a2d
    0x0000010: b500 16b1                              

Stacktrace:

Thread [main] (Suspended (exception java.lang.VerifyError))	
	java.lang.invoke.InnerClassLambdaMetafactory.spinInnerClass() line: 326 [local variables unavailable]	
	java.lang.invoke.InnerClassLambdaMetafactory.buildCallSite() line: 194	
	java.lang.invoke.LambdaMetafactory.metafactory(java.lang.invoke.MethodHandles$Lookup, java.lang.String, java.lang.invoke.MethodType, java.lang.invoke.MethodType, java.lang.invoke.MethodHandle, java.lang.invoke.MethodType) line: 304	
	java.lang.invoke.LambdaForm$DMH.1514322932.invokeStatic_L6_L(java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object) line: not available	
	java.lang.invoke.LambdaForm$BMH.1121453612.reinvoke(java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object) line: not available	
	java.lang.invoke.LambdaForm$MH.128526626.invoke_MT(java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object) line: not available	
	java.lang.invoke.CallSite.makeSite(java.lang.invoke.MethodHandle, java.lang.String, java.lang.invoke.MethodType, java.lang.Object, java.lang.Class<?>) line: 302	
	java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(java.lang.Class<?>, java.lang.invoke.MethodHandle, java.lang.String, java.lang.invoke.MethodType, java.lang.Object, java.lang.Object[]) line: 307	
	java.lang.invoke.MethodHandleNatives.linkCallSite(java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object[]) line: 297	
	org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(org.junit.jupiter.engine.execution.JupiterEngineExecutionContext, org.junit.platform.engine.support.hierarchical.Node$DynamicTestExecutor) line: 206	
	org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(org.junit.jupiter.engine.execution.JupiterEngineExecutionContext, org.junit.platform.engine.support.hierarchical.Node$DynamicTestExecutor) line: 131	
	org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(org.junit.platform.engine.support.hierarchical.EngineExecutionContext, org.junit.platform.engine.support.hierarchical.Node$DynamicTestExecutor) line: 65	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.lambda$executeRecursively$5() line: 139	
	org.junit.platform.engine.support.hierarchical.NodeTestTask$$Lambda$214.103536485.execute() line: not available	
	org.junit.jupiter.engine.support.OpenTest4JAndJUnit4AwareThrowableCollector(org.junit.platform.engine.support.hierarchical.ThrowableCollector).execute(org.junit.platform.engine.support.hierarchical.ThrowableCollector$Executable) line: 73	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.lambda$executeRecursively$7(org.junit.platform.engine.support.hierarchical.EngineExecutionContext) line: 129	
	org.junit.platform.engine.support.hierarchical.NodeTestTask$$Lambda$213.572191680.invoke(org.junit.platform.engine.support.hierarchical.EngineExecutionContext) line: not available	
	org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor(org.junit.platform.engine.support.hierarchical.Node<C>).around(C, org.junit.platform.engine.support.hierarchical.Node.Invocation<C>) line: 137	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.lambda$executeRecursively$8() line: 127	
	org.junit.platform.engine.support.hierarchical.NodeTestTask$$Lambda$212.1007309018.execute() line: not available	
	org.junit.jupiter.engine.support.OpenTest4JAndJUnit4AwareThrowableCollector(org.junit.platform.engine.support.hierarchical.ThrowableCollector).execute(org.junit.platform.engine.support.hierarchical.ThrowableCollector$Executable) line: 73	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.executeRecursively() line: 126	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.execute() line: 84	
	org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService$$Lambda$218.2076287037.accept(java.lang.Object) line: not available	
	java.util.ArrayList<E>.forEach(java.util.function.Consumer<? super E>) line: 1259	
	org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(java.util.List<? extends org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutorService.TestTask>) line: 38	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.lambda$executeRecursively$5() line: 143	
	org.junit.platform.engine.support.hierarchical.NodeTestTask$$Lambda$214.103536485.execute() line: not available	
	org.junit.jupiter.engine.support.OpenTest4JAndJUnit4AwareThrowableCollector(org.junit.platform.engine.support.hierarchical.ThrowableCollector).execute(org.junit.platform.engine.support.hierarchical.ThrowableCollector$Executable) line: 73	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.lambda$executeRecursively$7(org.junit.platform.engine.support.hierarchical.EngineExecutionContext) line: 129	
	org.junit.platform.engine.support.hierarchical.NodeTestTask$$Lambda$213.572191680.invoke(org.junit.platform.engine.support.hierarchical.EngineExecutionContext) line: not available	
	org.junit.jupiter.engine.descriptor.ClassTestDescriptor(org.junit.platform.engine.support.hierarchical.Node<C>).around(C, org.junit.platform.engine.support.hierarchical.Node.Invocation<C>) line: 137	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.lambda$executeRecursively$8() line: 127	
	org.junit.platform.engine.support.hierarchical.NodeTestTask$$Lambda$212.1007309018.execute() line: not available	
	org.junit.jupiter.engine.support.OpenTest4JAndJUnit4AwareThrowableCollector(org.junit.platform.engine.support.hierarchical.ThrowableCollector).execute(org.junit.platform.engine.support.hierarchical.ThrowableCollector$Executable) line: 73	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.executeRecursively() line: 126	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.execute() line: 84	
	org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService$$Lambda$218.2076287037.accept(java.lang.Object) line: not available	
	java.util.ArrayList<E>.forEach(java.util.function.Consumer<? super E>) line: 1259	
	org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(java.util.List<? extends org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutorService.TestTask>) line: 38	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.lambda$executeRecursively$5() line: 143	
	org.junit.platform.engine.support.hierarchical.NodeTestTask$$Lambda$214.103536485.execute() line: not available	
	org.junit.jupiter.engine.support.OpenTest4JAndJUnit4AwareThrowableCollector(org.junit.platform.engine.support.hierarchical.ThrowableCollector).execute(org.junit.platform.engine.support.hierarchical.ThrowableCollector$Executable) line: 73	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.lambda$executeRecursively$7(org.junit.platform.engine.support.hierarchical.EngineExecutionContext) line: 129	
	org.junit.platform.engine.support.hierarchical.NodeTestTask$$Lambda$213.572191680.invoke(org.junit.platform.engine.support.hierarchical.EngineExecutionContext) line: not available	
	org.junit.jupiter.engine.descriptor.JupiterEngineDescriptor(org.junit.platform.engine.support.hierarchical.Node<C>).around(C, org.junit.platform.engine.support.hierarchical.Node.Invocation<C>) line: 137	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.lambda$executeRecursively$8() line: 127	
	org.junit.platform.engine.support.hierarchical.NodeTestTask$$Lambda$212.1007309018.execute() line: not available	
	org.junit.jupiter.engine.support.OpenTest4JAndJUnit4AwareThrowableCollector(org.junit.platform.engine.support.hierarchical.ThrowableCollector).execute(org.junit.platform.engine.support.hierarchical.ThrowableCollector$Executable) line: 73	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.executeRecursively() line: 126	
	org.junit.platform.engine.support.hierarchical.NodeTestTask<C>.execute() line: 84	
	org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutorService.TestTask) line: 32	
	org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor<C>.execute() line: 57	
	org.junit.jupiter.engine.JupiterTestEngine(org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine<C>).execute(org.junit.platform.engine.ExecutionRequest) line: 51	
	org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(org.junit.platform.engine.TestDescriptor, org.junit.platform.engine.EngineExecutionListener, org.junit.platform.engine.ConfigurationParameters, org.junit.platform.engine.TestEngine) line: 108	
	org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(org.junit.platform.launcher.core.LauncherDiscoveryResult, org.junit.platform.engine.EngineExecutionListener) line: 88	
	org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(org.junit.platform.launcher.core.InternalTestPlan, org.junit.platform.launcher.core.LauncherDiscoveryResult, org.junit.platform.launcher.TestExecutionListener) line: 54	
	org.junit.platform.launcher.core.EngineExecutionOrchestrator$$Lambda$170.1920387277.accept(java.lang.Object) line: not available	
	org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(org.junit.platform.engine.ConfigurationParameters, org.junit.platform.launcher.core.TestExecutionListenerRegistry, java.util.function.Consumer<org.junit.platform.launcher.TestExecutionListener>) line: 67	
	org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(org.junit.platform.launcher.core.InternalTestPlan, org.junit.platform.launcher.TestExecutionListener...) line: 52	
	org.junit.platform.launcher.core.DefaultLauncher.execute(org.junit.platform.launcher.core.InternalTestPlan, org.junit.platform.launcher.TestExecutionListener[]) line: 96	
	org.junit.platform.launcher.core.DefaultLauncher.execute(org.junit.platform.launcher.TestPlan, org.junit.platform.launcher.TestExecutionListener...) line: 84	
	org.eclipse.jdt.internal.junit5.runner.JUnit5TestReference.run(org.eclipse.jdt.internal.junit.runner.TestExecution) line: 98	
	org.eclipse.jdt.internal.junit.runner.TestExecution.run(org.eclipse.jdt.internal.junit.runner.ITestReference[]) line: 41	
	org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(java.lang.String[], java.lang.String, org.eclipse.jdt.internal.junit.runner.TestExecution) line: 542	
	org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(org.eclipse.jdt.internal.junit.runner.TestExecution) line: 770	
	org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run() line: 464	
	org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(java.lang.String[]) line: 210	

[Don-vip]

In 17310/josm:

see #16567 - rewrite ImageDataTest to get rid of problematic JMockit annotation

[Don-vip]

Interestingly, RenderingCLIAreaTest fails without using JMockit...

[Don-vip]

WTF, after Ubuntu upgrade that contains openjdk 8u275, everything's fine again. So it was likely a weird Java bug...

[Stereo]

The CI at ​https://github.com/openstreetmap/josm/actions use Ubuntu 18.04, macOS Catalina 10.15 and Windows Server 2019, with java 8, 11, 15 and 16-ea.

We can run the tests on anything that's listed on ​https://docs.github.com/en/free-pro-team@latest/actions/reference/specifications-for-github-hosted-runners#supported-runners-and-hardware-resources

16-ea builds seem to be failing right now.

[Don-vip]

Replying to Stereo:

16-ea builds seem to be failing right now.

See ticket:19724#comment:3

[Don-vip]

In 35642/osm:

see #16567 - see #20085 - JUnit to JUnitLauncher for plugins (patch by taylor.smock)

[Don-vip]

In 17360/josm:

see #16567 - print parameterized tests display name in failure messages

• https://josm.openstreetmap.de/ticket/16567#comment:53
• ​https://bz.apache.org/bugzilla/show_bug.cgi?id=64564
• ​https://github.com/apache/ant/pull/121

[simon04]

Nice, our commit is linked to the Apache Ant GitHub issue: ​https://github.com/apache/ant/pull/121#ref-commit-dfd4e3b

[Don-vip]

Replying to simon04:

Nice, our commit is linked to the Apache Ant GitHub issue: ​https://github.com/apache/ant/pull/121#ref-commit-dfd4e3b

This is a GitHub feature I love :)

[Don-vip]

Following my comment the feature has been ​implemented in Ant master:

                    <listener type="legacy-plain" useLegacyReportingName="false" />
                    <listener type="legacy-xml" useLegacyReportingName="false" />

Unfortunately I can't commit the new attributes right now as older versions of Ant fail on unknown tag:

BUILD FAILED
C:\SVN\josm\core\build.xml:505: The following error occurred while executing this line:
C:\SVN\josm\core\build.xml:434: listener doesn't support the "useLegacyReportingName" attribute

So until ant 1.10.10 is released and widely used in IDEs and Linux distributions, I'll tell Jenkins to patch build.xml on the fly, so that we can get our old reports back.

[Don-vip]

Done: ant 1.10.10alpha is now used for JOSM integration tests, and we have nice errors again:

org.openstreetmap.josm.gui.preferences.map.TaggingPresetPreferenceTestIT.Persian Presets - https://github.com/DearRude/IranianPresets/blob/master/zip/latast.zip

@Klumbumbus it will be easy again to find which extension is broken.

[Klumbumbus]

Great.

[Klumbumbus]

see https://josm.openstreetmap.de/jenkins/job/JOSM-Integration/5463/testReport/
For Rules, Plugins and possibly Styles there is still no information which one is the culprit. However I'm not sure if it was the same before the switch to JUnit 5.

[Don-vip]

In 17383/josm:

see #16567 - better assertion error messages for rules/plugins integration tests

[GerdP]

I've noticed that some unit tests call JOSMFixture.createUnitTestFixture().init(); in @BeforeEach, others do this only in @BeforeAll. The latter is much faster, so I wonder if the former is needed or not and if needed, what are the criteria?

[taylor.smock]

Replying to GerdP:

I've noticed that some unit tests call JOSMFixture.createUnitTestFixture().init(); in @BeforeEach, others do this only in @BeforeAll. The latter is much faster, so I wonder if the former is needed or not and if needed, what are the criteria?

It pretty much comes down to how the JUnit extension is registered (JOSMTestRules). If it is static, then it is essentially called in @BeforeAll. If it is not static, then it is essentially called in @BeforeEach. This is largely due to how I implemented JUnit5 support in JOSMTestRules -- I specifically wanted to keep current behavior w.r.t. JUnit4 to make it easier for people to port tests (especially the plugin users, and I don't anticipate being able to remove the JUnit4 support until mid to late 2021, at the earliest).

I suspect we could technically make modifications such that JOSMFixture is only called in the @BeforeAll, but that requires:
a) Making all JOSMTestRules static in our source code
b) Waiting on plugin authors to update their tests so that all of their JOSMTestRules are static

I'm pretty certain that we can have static JOSMTestRules have a beforeAll and a beforeEach behavior, and an afterAll and afterEach behavior. We just have to refactor the current code. Which I didn't do specifically to allow plugin authors a chance to update their unit tests to JUnit 5.

Plugins that I know have moved to JUnit 5:

• Mapillary
• MapWithAI (partial, mostly there)

Docs for @RegisterExtension:
​https://junit.org/junit5/docs/5.1.1/api/org/junit/jupiter/api/extension/RegisterExtension.html

It actually looks like, if JOSMTestRules is static, that beforeEach and beforeAll are both called. I'll see if that bears out with a debugger. :(

[GerdP]

Ok, sounds complicated. What's the difference between e.g. SelfIntersectingWayTest which uses @BeforeALl and SharpAnglesTest which uses @BeforeEach?

[taylor.smock]

Replying to GerdP:

Ok, sounds complicated. What's the difference between e.g. SelfIntersectingWayTest which uses @BeforeALl and SharpAnglesTest which uses @BeforeEach?

Um... I don't remember why I wasn't using JOSMTestRules in SharpAnglesTest. But the fixture should probably be in a @BeforeAll method. I haven't actually looked at the fixture, but it is also initialized in JOSMTestRules.

[Don-vip]

JOSMFixture is the old mechanism, you should avoid it for new tests. Everything should be doable with JOSMTestRules only.

[GerdP]

I try to understand the output of ImageryPreferenceTestIT. What is the purpose of this test? What are the expected actions to fix a problem reported by this test?

[Klumbumbus]

It tests if the source actually delivers an image and if the urls (attribution url, icon url,...) are working. In the last test https://josm.openstreetmap.de/jenkins/job/JOSM-Imagery-Integration/lastCompletedBuild/testReport/ 98 failed. Some might be false positives and actually work in JOSM, these could be put on ignorelist at IntegrationTestIgnores. Vincent also somtimes did report to the providers if there are incomplete ssl certificate chains or things like that.

[Klumbumbus]

If it is an real error it should be fixed on the subpages of wiki:/Maps

[GerdP]

Replying to Klumbumbus:

If it is an real error it should be fixed on the subpages of wiki:/Maps

OK, but how does one find out that it is a real error? Is this something that can be done by anybody?

[Klumbumbus]

Trying if the imagery works in JOSM / checking if an URL is not dead/404?

[Don-vip]

Can we please keep this ticket for tracking JUnit 5 migration?

[Don-vip]

In 17403/josm:

see #16567 - JUnit 5: enable parallel execution mode with command line parameters

See ​https://junit.org/junit5/docs/snapshot/user-guide/index.html#writing-tests-parallel-execution

[michael2402]

Does anyone have a list of which patches in this ticket are applied and which are not?

[taylor.smock]

Initial work on using {After,Before}{Each,All}Callbacks properly. Some tests are failing in Eclipse.

[taylor.smock]

Replying to michael2402:

Does anyone have a list of which patches in this ticket are applied and which are not?

• attachment:16567.12.patch​
• attachment:16567.deprecated.4.patch​
• attachment:16567.fixup1.patch​
• attachment:16567.fixup1.1.patch​

I don't know about anything else, but all attachments prior to attachment:16567.fixup1.1.patch​ is either in the above list, or was not applied.

[michael2402]

@taylor Only eclipse? Or also Jenkins/ANT?

About your patch:
Classes passed to ExtendWith should not have a global state. You should store all state in the ExtensionContext.

Checking the annotations in the JOSM Test rules is prone to errors, because users always have to remember to add JOSMTestRules to the class. And you should get the annotation from Junit, not by hand. What you should do is something like:

public class OverrideAssumeRevisionExtension extends BeforeEachCallback {
  public void beforeEach(ExtensionContext context) {
    // Get annotation from context.getElement()
    // Then recurse to context.getParent() if not available
  }
}

@ExtendWith(OverrideAssumeRevisionExtension.class)
@Retention(RetentionPolicy.RUNTIME)
@Target(value={TYPE,METHOD})
@interface OverrideAssumeRevision{
int value();
}

[taylor.smock]

Replying to michael2402:

@taylor Only eclipse? Or also Jenkins/ANT?

I've only run tests in eclipse so far. I'd expect at least one of them to pass properly in ant (which is why I called out Eclipse -- that specific test case failed with java.awt.HeadlessException, and I don't think I set eclipse up for headless tests).

About your patch:
Classes passed to ExtendWith should not have a global state. You should store all state in the ExtensionContext.

Checking the annotations in the JOSM Test rules is prone to errors, because users always have to remember to add JOSMTestRules to the class. And you should get the annotation from Junit, not by hand. What you should do is something like:

I thought the annotations having any effect were dependent upon users adding JOSMTestRules to the class (via @RegisterExtension).

public class OverrideAssumeRevisionExtension extends BeforeEachCallback {
  public void beforeEach(ExtensionContext context) {
    // Get annotation from context.getElement()
    // Then recurse to context.getParent() if not available
  }
}

@ExtendWith(OverrideAssumeRevisionExtension.class)
@Retention(RetentionPolicy.RUNTIME)
@Target(value={TYPE,METHOD})
@interface OverrideAssumeRevision{
int value();
}

I'll upload the changes I think you were trying to get at.

[taylor.smock]

Use @ExtendWith for Override annotations

[michael2402]

Replying to taylor.smock:

I'll upload the changes I think you were trying to get at.

The JOSMTestRules stuff is Junit4. I used it as an an ugly hack because what we acutally need was not possible with Junit4. See #12949. I did some more refactoring so that test rules did not always include everything but only the parts needed in #12977 #13005 #13017 #13018 #12974 #13033 #13037 #13045 #13047 #13048

When moving to Junit 5, we should get rid of JOSMTestRules all together. We should create new annotations for all the things the test rules changed, e.g.:

@UseI18n("en")
@UseJosmHome()
@UsePreferences()
@UseApi(APIType)
@UsePlatform()
@UseProjection("EPSG:3857")

You can put @ExtendWith on the annotation. Junit searches recursively, so if you mark a test with @OverrideAssumeRevision, it will automatically apply the @ExtendWith. So for each annotation, you create your own extension. You can also add @ExtendWith twice to use other dependend annotations (in the order in which they should be loaded), e.g. if Preferences requires JosmHome, then use:

@ExtendWith(UseJosmHomeExtension.class)
@ExtendWith(UsePreferences.class)
…
@interface UsePreferences {…}

You can then even pass parameters into the tests. For example, this would then work:

@Test
public void myTest(MainLayerManager ml) {
}

If you need more help, we can have a hangout / other video call, most of this is easier explained that way.

[taylor.smock]

OK. That makes a lot more sense than what I thought you were saying.

I've been trying to keep some backwards compatibility, so plugins with tests don't have to update from JUnit{3,4} to JUnit5, and then have to spend time figuring out how to fix their tests.

Splitting things out will definitely be clearer and better.

[michael2402]

Replying to taylor.smock:

I've been trying to keep some backwards compatibility, so plugins with tests don't have to update from JUnit{3,4} to JUnit5, and then have to spend time figuring out how to fix their tests.

Keep the old JOSMTestRules, don't touch them. Keep the JOSMFixture, don't touch it. Mark them as deprecated

And then write your extensions from scratch. We will have duplicate code for some time, but we won't maintain the old one for long, so it is not an issue. That way, we can start with clean extensions and clean up all the tests, plugin can then update whenever they like.

[taylor.smock]

Initial extensions replacing JOSMTestRules#preferences and JOSMTestRules#assumeRevision (Override should probably be dropped from filenames)

[taylor.smock]

Modify two tests to use the annotations, add i18n, Main, ApiType, Presets, Projection, ProjectionNadGrids, and Territories. Territories will probably be modified so that there can be different initialization types (e.g., full initialization or partial initialization).

[taylor.smock]

I probably won't get back to this for awhile (probably a month or so), so this post is largely to jog my memory. Or to help someone else if they decide to work on it in the meantime.

Unused annotations (with the two modified tests):

• OsmApiType
• AssumeRevision
• I18n
• Presets
• ProjectionNadGrids
• Territories

Behavior that I need to check:

• What happens if i18n (for example) can be used on both methods and types, and the extension implements BeforeAllCallback and BeforeEachCallback. Is BeforeEachCallback called prior to every method (my suspicion), or is it just called prior to annotated methods (probably not). So I need to figure out a good way to check and see if the most recent annotation is for a method or a class, and then either run the BeforeEachCallback or not based off of that.

Use case: Test localization in English and Spanish in the same test class.

[michael2402]

Replying to taylor.smock:

Behavior that I need to check:

• What happens if i18n (for example) can be used on both methods and types, and the extension implements BeforeAllCallback and BeforeEachCallback. Is BeforeEachCallback called prior to every method (my suspicion), or is it just called prior to annotated methods (probably not). So I need to figure out a good way to check and see if the most recent annotation is for a method or a class, and then either run the BeforeEachCallback or not based off of that.

Use case: Test localization in English and Spanish in the same test class.

No problem there if you only use beforeEach and afterEach. You can use:

@I18n("en") // This will get extended to @ExtendWith(I18nExtension)
public class MyTest {
  // BeforeAllCallback would called before all tests with the context of the class => sees only @I18n("en")

  @Test
  public void testEn() {
    // This test is extended with I18nExtension
    // I18nExtension#beforeEach is called with the context of the method
    // I18nExtension looks for @I18n on the method (=> not found) and then on the class (=> en)
  }

  @Test
  @I18n("de") // This will get add another @ExtendWith(I18nExtension)
  public void testDe() {
    // This test is extended with I18nExtension
    // The extension is only applied once (Junit removes duplicates)
    // I18nExtension#beforeEach is called with the context of the method
    // I18nExtension looks for @I18n on the method (=> de)
  }
}

[taylor.smock]

Modify Territories.Extension so that there are various initialization states, and doesn't reinitialize when not needed

[taylor.smock]

Replying to michael2402:

...

I was actually thinking more about items that really don't need to be initialized more than once (Territories is one such item), but may need to be overridden for whatever reason (maybe only one test needs the entirety of Territories to be initialized, but every other test in the class only needs the basic initialization done).

There are probably some other items that shouldn't be called more than once, but I don't know them off the top of my head.

[michael2402]

Replying to taylor.smock:

There are probably some other items that shouldn't be called more than once, but I don't know them off the top of my head.

Then add a uninitalize to them.

Before the annotated test => initialize. After the annotated test => uninitialize.

That way, we can be sure that every test works in the environment it defines. Otherwise, we would e.g. have hard to track problems when the order of tests changes.

[taylor.smock]

Modify Territories to have an uninitialize function (java-docs indicate that calling it outside of a test environment is a very bad idea (tm)), modify several tests to use the extensions (largely focused on longer tests, most taking >10s on my machine), add a utils class to find appropriate annotations and reset static classes, add replacements for assertionsInEDT, http, https, a new specific annotation for DeleteCommandCallback, setting up and cleaning up the layer environment, timezone usage, annotations for integration and slow tests

[michael2402]

@taylor.smock

TimeZoneAnnotation:

You should either mark this as only @Target({ElementType.TYPE}) to prevent use at the method level or you should use afterEach instead of afterAll.

When Extension Annotations are added at a method level, only the beforeEach and afterEach callbacks are executed, but the afterAll callback is never executed then (it is just silently ignored)

[Don-vip]

In 17478/josm:

see #16567 - see #20433 - restore parallel execution of imagery integration test

[Don-vip]

In 17531/josm:

see #16567 - consistency between extended source entry integration tests

[Don-vip]

In 17532/josm:

see #16567 - ignore IAE that can happen when running concurrent tests

[Don-vip]

In 17536/josm:

see #16567 - revert parallel execution of TaggingPresetPreferenceTestIT - huge instability

[Don-vip]

In 17541/josm:

see #16567 - better synchronization of Preferences.settingsMap

[simon04]

In 17677/josm:

see #16567 - JUnit 5: use org.junit.jupiter.api.Assumptions

[Don-vip]

In 17966/josm:

see #16567 - hide stdout/stderr of unit tests from Ant output

[Don-vip]

Replying to Don-vip:

In 17541/josm:

see #16567 - better synchronization of Preferences.settingsMap

Ah, my bad, this is the cause of the deadlock in #21462

[gaben]

Hmm, just found this ticket. I started fixing the remaining assertions/imports in tests, about 90 test files modified so far. I can provide a patch in a few days if needed.

[taylor.smock]

Replying to gaben:

Hmm, just found this ticket. I started fixing the remaining assertions/imports in tests, about 90 test files modified so far. I can provide a patch in a few days if needed.

I think I've already done that. I just need to get all the tests working, so I should probably dust off that patch series. See #21139 (git diffs available at ​https://gitlab.com/smocktaylor/josm/-/merge_requests/8 ).

[gaben]

Thanks, didn't know that. I changed org.junit.Assert to the new org.junit.jupiter.api.Assertions import path and fixed some minor code issues alongside. I see you changed some, but not all.

[gaben]

Replace Junit4 imports and assertions with the new Junit5 style code according to ​https://junit.org/junit5/docs/current/user-guide/#migrating-from-junit4-tips, plus minor simplifications

[taylor.smock]

Just a heads up, it looks like JMockit isn't properly cleaning up mocks in JUnit4 tests when run with the vintage runner.

See https://josm.openstreetmap.de/jenkins/job/JOSM/jdk=JDK8/lastCompletedBuild/testReport/org.openstreetmap.josm.tools.bugreport/BugReportTest/testSuppressedExceptions_String__Consumer__2_/ .

I'm trying to figure out what the smallest patch is to fix the issue, and then I think I'll rebase the patch I have. I had been planning on waiting for a ​fix in ant before I picked this up again, but I think it might be better to move off of JUnit4 sooner rather than later.

@gaben: I'll go ahead and pull your patch into my branch, just so I can fix conflicts at the same time.

[gaben]

@taylor, it seems your repo isn't available (at least publicly). Have you made progress since the last comment?

[taylor.smock]

Funny story that...

​GitLab decided to make it so OSS projects had to jump through hoops to have more than 5 GB of storage, and I had to clean up my personal space by a bit. So I cleaned out most of the "large" repos I had.

I thought I had a copy on my hard drive, but I wasn't able to find it. I should have forked it onto GitHub and then gone from there.

Anyway, one of the blockers was ant -- I fixed a bug in it (probably 6 months to a year ago) which was released a month or so ago.

I'll go ahead and merge your changes as soon as I track down why tests are suddenly failing (I suspect state pollution, and I fixed one bug I found while debugging in my IDE, which fixed the tests in my IDE but not via ant test).

If I forget, don't feel bad about pinging this ticket again.

[taylor.smock]

In 18690/josm:

See #16567: Convert all assertion calls to JUnit 5 (patch by gaben, modified)

The modifications are as follows:

• Merge DomainValidatorTest.testIDN and DomainValidatorTest.testIDNJava6OrLater
• Update some tests to use @ParameterizedTest (DomainValidatorTest)
• Replace various exception blocks with assertThrows. These typically looked like

      try {
          // Something that should throw an exception here
          fail("An exception should have been thrown");
      } catch (Exception e) {
          // Verify the exception matches expectations here
      }
  

• Replace assertTrue(val instanceof Clazz) with assertInstanceOf
• Replace JUnit 4 @Suite with JUnit 5 @Suite

Both the original patch and the modified patch fix various lint issues.

[taylor.smock]

In 36063/osm:

See #16567: Convert all assertion calls to JUnit 5 (patch by gaben, modified)

The modifications are as follows:

• Merge DomainValidatorTest.testIDN and DomainValidatorTest.testIDNJava6OrLater
• Update some tests to use @ParameterizedTest (DomainValidatorTest)
• Replace various exception blocks with assertThrows. These typically looked like

      try {
          // Something that should throw an exception here
          fail("An exception should have been thrown");
      } catch (Exception e) {
          // Verify the exception matches expectations here
      }
  

• Replace assertTrue(val instanceof Clazz) with assertInstanceOf
• Replace JUnit 4 @Suite with JUnit 5 @Suite

Both the original patch and the modified patch fix various lint issues.

[gaben]

I see the ticket is still open, I'd take this opportunity to add another patch :)

test/functional/org/openstreetmap/josm/data/BoundariesTestIT.java

@@ -28 +28 @@
     private static final List<String> RETIRED_ISO3166_1_CODES = Arrays.asList(
             "AN", "BU", "CS", "NT", "TP", "YU", "ZR");
 
-    private static final List<String> EXCEPTIONNALY_RESERVED_ISO3166_1_CODES = Arrays.asList(
+    private static final List<String> EXCEPTIONALLY_RESERVED_ISO3166_1_CODES = Arrays.asList(
             "AC", "CP", "DG", "EA", "EU", "EZ", "FX", "IC", "SU", "TA", "UK", "UN", "XK");
 
     private static final List<String> ISO3166_2_CODES = Arrays.asList(
@@ -72 +72 @@
             // Check for unknown ISO-3166-1 alpha 2 codes
             for (OsmPrimitive p : tagged.stream().filter(SearchCompiler.compile("ISO3166-1\\:alpha2")).collect(Collectors.toList())) {
                 String code = p.get("ISO3166-1:alpha2");
-                assertTrue(iso31661a2.contains(code) || EXCEPTIONNALY_RESERVED_ISO3166_1_CODES.contains(code), code);
+                assertTrue(iso31661a2.contains(code) || EXCEPTIONALLY_RESERVED_ISO3166_1_CODES.contains(code), code);
             }
             // Check presence of all ISO-3166-2 codes for USA, Canada, Australia (for speed limits)
             for (String code : ISO3166_2_CODES) {

[taylor.smock]

Replying to gaben:

I see the ticket is still open, I'd take this opportunity to add another patch :)

The following files still use JUnit 4. Some are for backwards compatibility reasons, but others are due to needing WireMock server support (note: wiremock now supports JUnit 5, so I should spend some time upgrading the rest of the tests). At that point, the only things which have JUnit 4 code should be compatibility code in testutils.

• source:trunk/test/unit/org/openstreetmap/josm/plugins/PluginHandlerMultiVersionTest.java​
• source:trunk/test/unit/org/openstreetmap/josm/plugins/PluginHandlerJOSMTooOldTest.java​
• source:trunk/test/unit/org/openstreetmap/josm/io/CertificateAmendmentTestIT.java​
• source:trunk/test/unit/org/openstreetmap/josm/gui/preferences/plugin/PluginPreferenceHighLevelTest.java​
• source:trunk/test/unit/org/openstreetmap/josm/gui/layer/gpx/DownloadWmsAlongTrackActionTest.java​
• source:trunk/test/unit/org/openstreetmap/josm/gui/dialogs/layer/CycleLayerActionTest.java​
• source:trunk/test/unit/org/openstreetmap/josm/gui/dialogs/MinimapDialogTest.java​
• source:trunk/test/unit/org/openstreetmap/josm/testutils/ParallelScheduler.java​
• source:trunk/test/unit/org/openstreetmap/josm/testutils/ExpectedRootException.java​
• source:trunk/test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java​
• source:trunk/test/unit/org/openstreetmap/josm/testutils/ParallelParameterized.java​
• source:trunk/test/unit/org/openstreetmap/josm/testutils/PluginServer.java​
• source:trunk/test/unit/org/openstreetmap/josm/testutils/TileSourceRule.java​

I'll do a pass for JUnit 4 tests in the plugins that we can maintain. External plugins are on their own.

[taylor.smock]

In 18694/josm:

Fix #22381: Try to automatically install newly required plugins on plugin update

This additionally updates the plugin tests to JUnit 5 (see #16567). Some tests
required fixing (for example, MinimapDialogTest fails if the system of
measurement is metric).

[taylor.smock]

In 36064/osm:

See #16567: Convert most plugin tests to JUnit 5

[taylor.smock]

In 18798/josm:

See #16567: Add @Territories annotation for JUnit 5

This reduces memory allocations during test runs by ~20% since we are no longer
running Territories.initialize() on every test where JOSMTestRules.territories()
was called. This reduced overall test time by ~10%, mostly due to less time spent
in JVM internals.

[taylor.smock]

In 18853/josm:

See #16567: Update to JUnit 5

This removes new JOSMTestRules() with no additional setup and most
JOSMFixture calls.

Removing the bare JOSMTestRules speeds up the test suite since there are two
fewer System.gc() calls per test.

[taylor.smock]

More @Annotations, convert most tests to use them

[taylor.smock]

In 18867/josm:

See #16567: Update to JUnit 5

This adds the @TaggingPresets annotation used by r18866 (see #23196). The
annotation tries to only initialize the presets when the current preset list does
not have the same hashCode as the default preset list. In order for this to work,
TaggingPresets#getTaggingPresets had to return a List (the method contract
has not changed), which calculates the hashCode in a deterministic manner.

[taylor.smock]

In 18870/josm:

See #16567: Update to JUnit 5

This converts most tests to use @Annotations. There are also some performance
improvements as it relates to tests.

[taylor.smock]

In 18893/josm:

Fix #16567: Upgrade to JUnit 5

JOSMTestRules and JOSMTestFixture can reset the default JOSM profile, which can
be unexpected for new contributors. This updates all tests to use JUnit 5 and
the new JUnit 5 annotations.

This also renames MapCSSStyleSourceFilterTest to MapCSSStyleSourceFilterPerformanceTest
to match the naming convention for performance tests and fixes some lint issues.

This was tested by running all tests individually and together.

[taylor.smock]

In 36193/osm:

See #16567: Upgrade to JUnit 5

This updates plugins to use the JUnit 5 annotations