Modify

Opened 2 years ago

Last modified 40 hours ago

#16567 reopened enhancement

[PATCH] Upgrade to JUnit 5

Reported by: Don-vip Owned by: team
Priority: major Milestone: Longterm
Component: Unit tests Version:
Keywords: junit Cc:

Description

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

Attachments (20)

16567.patch (2.4 KB) - added by taylor.smock 13 months ago.
Allow JUnit 5 tests to use JOSMTestRules
16567.1.patch (10.6 KB) - added by taylor.smock 13 months ago.
Modify build.xml to use junitlauncher instead of junit. (jacoco output not working, looks like it is a problem with the destfile syntax).
junit_jars.sh (1.2 KB) - added by taylor.smock 13 months ago.
Add script to get jar files (for easy downloading and testing)
16567.4.patch (10.6 KB) - added by taylor.smock 12 months ago.
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.
16567.5.patch (10.7 KB) - added by taylor.smock 12 months ago.
Fix an issue where non-unit tests (i.e., functional tests) didn't have JOSMTestRules in their classpath.
junit_difference.py (1.3 KB) - added by taylor.smock 12 months ago.
Show tests that have different numbers of passed/failed tests
16567.6.patch (10.7 KB) - added by taylor.smock 11 months ago.
Update to ensure patch applies cleanly (see r15703)
16567.9.patch (13.6 KB) - added by taylor.smock 7 months ago.
Update for Ivy dependencies
16567.11.patch (12.9 KB) - added by taylor.smock 6 months ago.
Fix ant test-perf (failing since the jacoco agent was added, even when coverage was disabled)
assertThat-warning.txt (35.3 KB) - added by simon04 6 months ago.
This patch causes plenty of deprecation warnings…
16567.12.patch (12.7 KB) - added by taylor.smock 6 months ago.
Remove unnecessary modification from JOSMTestRules
16567.deprecated.1.patch (52.1 KB) - added by taylor.smock 6 months ago.
Fix (most) deprecation warnings. There is still one in ExpectedRootException and OptionParserTest.
16567.deprecated.2.patch (58.6 KB) - added by taylor.smock 6 months ago.
ExpectedRootException has been deprecated, and LayerManagerTest has been modified to account for that
16567.deprecated.3.patch (59.0 KB) - added by taylor.smock 6 months ago.
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.
16567.deprecated.4.patch (56.9 KB) - added by taylor.smock 6 months ago.
Update for testMultipleShort changes
16567.fixup1.patch (1.5 KB) - added by taylor.smock 6 months ago.
Ensure that after is not run until after the tests complete
16567.fixup1.1.patch (2.3 KB) - added by taylor.smock 6 months ago.
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)
16567.imageryit.patch (8.3 KB) - added by taylor.smock 6 months ago.
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
16567.domainvalidatortestit.patch (3.2 KB) - added by taylor.smock 5 months ago.
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.
16567.domainvalidatortestit.2.patch (6.5 KB) - added by taylor.smock 5 months ago.

Download all attachments as: .zip

Change History (134)

comment:1 Changed 2 years ago by Don-vip

Milestone: 18.08

comment:2 Changed 2 years ago by Don-vip

Changed 13 months ago by taylor.smock

Attachment: 16567.patch added

Allow JUnit 5 tests to use JOSMTestRules

comment:3 Changed 13 months ago by 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"
Last edited 13 months ago by taylor.smock (previous) (diff)

comment:4 in reply to:  2 Changed 13 months ago by 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 )

Changed 13 months ago by taylor.smock

Attachment: 16567.1.patch added

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

comment:5 Changed 13 months ago by 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

comment:6 Changed 13 months ago by taylor.smock

Summary: Upgrade to JUnit 5[WIP PATCH] Upgrade to JUnit 5

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).

comment:7 Changed 13 months ago by Don-vip

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

comment:8 Changed 13 months ago by 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/)

Changed 13 months ago by taylor.smock

Attachment: junit_jars.sh added

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

comment:9 in reply to:  8 Changed 13 months ago by 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.

comment:10 Changed 13 months ago by 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.

comment:11 Changed 13 months ago by 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?

comment:12 in reply to:  11 ; Changed 13 months ago by 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.

comment:13 in reply to:  12 Changed 13 months ago by 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.

Changed 12 months ago by taylor.smock

Attachment: 16567.4.patch added

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.

comment:14 Changed 12 months ago by 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.

comment:15 Changed 12 months ago by 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.

Last edited 12 months ago by taylor.smock (previous) (diff)

Changed 12 months ago by taylor.smock

Attachment: 16567.5.patch added

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

Changed 12 months ago by taylor.smock

Attachment: junit_difference.py added

Show tests that have different numbers of passed/failed tests

comment:16 Changed 12 months ago by taylor.smock

Summary: [WIP PATCH] Upgrade to JUnit 5[PATCH] Upgrade to JUnit 5

comment:17 Changed 12 months ago by Don-vip

Milestone: 19.12
Owner: changed from team to Don-vip
Priority: normalmajor
Status: newassigned

comment:18 Changed 11 months ago by 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.

comment:19 Changed 11 months ago by Don-vip

Milestone: 19.1220.01

comment:20 Changed 11 months ago by Don-vip

In 15703/josm:

see #16567 - generate Jacoco CSV and XML reports

Changed 11 months ago by taylor.smock

Attachment: 16567.6.patch added

Update to ensure patch applies cleanly (see r15703)

comment:21 Changed 10 months ago by Don-vip

Milestone: 20.0120.02

comment:22 Changed 10 months ago by taylor.smock

Is there anything I need to do?

comment:23 in reply to:  22 Changed 10 months ago by 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.

comment:24 Changed 10 months ago by 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).

comment:25 Changed 9 months ago by Don-vip

Milestone: 20.0220.03

comment:26 Changed 9 months ago by Don-vip

Milestone: 20.0320.04

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

Changed 7 months ago by taylor.smock

Attachment: 16567.9.patch added

Update for Ivy dependencies

comment:27 Changed 7 months ago by Klumbumbus

Milestone: 20.0420.05

Milestone renamed

comment:28 Changed 7 months ago by simon04

Milestone: 20.05Longterm

comment:29 Changed 6 months ago by 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).

Last edited 6 months ago by taylor.smock (previous) (diff)

Changed 6 months ago by taylor.smock

Attachment: 16567.11.patch added

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

Changed 6 months ago by simon04

Attachment: assertThat-warning.txt added

This patch causes plenty of deprecation warnings…

comment:30 Changed 6 months ago by 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.

Changed 6 months ago by taylor.smock

Attachment: 16567.12.patch added

Remove unnecessary modification from JOSMTestRules

Changed 6 months ago by taylor.smock

Attachment: 16567.deprecated.1.patch added

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

comment:31 Changed 6 months ago by 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).

Last edited 6 months ago by taylor.smock (previous) (diff)

Changed 6 months ago by taylor.smock

Attachment: 16567.deprecated.2.patch added

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

Changed 6 months ago by taylor.smock

Attachment: 16567.deprecated.3.patch added

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.

comment:32 Changed 6 months ago by 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.

comment:33 Changed 6 months ago by Don-vip

Owner: changed from Don-vip to team
Status: assignednew

comment:34 Changed 6 months ago by simon04

In 16608/josm:

see #16567 - Fix OptionParserTest.testMultipleShort

Changed 6 months ago by taylor.smock

Attachment: 16567.deprecated.4.patch added

Update for testMultipleShort changes

comment:35 Changed 6 months ago by simon04

Resolution: fixed
Status: newclosed

In 16617/josm:

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

I applied attachment:16567.12.patch

Last edited 6 months ago by simon04 (previous) (diff)

comment:36 Changed 6 months ago by simon04

In 16618/josm:

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

I applied attachment:16567.deprecated.4.patch

Last edited 6 months ago by simon04 (previous) (diff)

comment:37 Changed 6 months ago by simon04

Milestone: Longterm20.06

comment:38 Changed 6 months ago by 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)

comment:39 Changed 6 months ago by simon04

Resolution: fixed
Status: closedreopened

comment:40 Changed 6 months ago by simon04

In 16633/josm:

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

Changed 6 months ago by taylor.smock

Attachment: 16567.fixup1.patch added

Ensure that after is not run until after the tests complete

comment:41 Changed 6 months ago by 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.

Changed 6 months ago by taylor.smock

Attachment: 16567.fixup1.1.patch added

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)

comment:42 Changed 6 months ago by simon04

In 16658/josm:

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

comment:43 Changed 6 months ago by simon04

In 16659/josm:

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

comment:44 in reply to:  43 Changed 6 months ago by 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.

comment:46 Changed 6 months ago by 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

    diff --git a/test/unit/org/openstreetmap/josm/gui/preferences/imagery/ImageryPreferenceTestIT.java b/test/unit/org/openstreetmap/josm/gui/preferences/imagery/ImageryPreferenceTestIT.java
    index 24fc65551..b1950c09b 100644
    a b public static void afterClass() { 
    128128        ImageryLayerInfo.instance.load(false);
    129129        return ImageryLayerInfo.instance.getDefaultLayers()
    130130                .stream()
     131                .limit(3)
    131132                .map(x -> new Object[] {x.getId(), x})
    132133                .collect(Collectors.toList());
    133134    }

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 :-(

Changed 6 months ago by taylor.smock

Attachment: 16567.imageryit.patch added

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

comment:48 Changed 6 months ago by 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).

comment:49 in reply to:  48 Changed 6 months ago by 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 ...
Last edited 6 months ago by taylor.smock (previous) (diff)

comment:50 Changed 5 months ago by 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/

comment:51 Changed 5 months ago by 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...)

comment:52 Changed 5 months ago by simon04

Resolution: fixed
Status: reopenedclosed

In 16727/josm:

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

comment:53 Changed 5 months ago by 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)

comment:54 Changed 5 months ago by 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/

comment:55 in reply to:  54 Changed 5 months ago by 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.

Changed 5 months ago by 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.

Changed 5 months ago by taylor.smock

comment:56 Changed 5 months ago by Klumbumbus

Milestone: 20.0620.07
Resolution: fixed
Status: closedreopened

comment:58 in reply to:  52 ; Changed 5 months ago by 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".

comment:59 in reply to:  58 Changed 5 months ago by 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).

comment:60 Changed 5 months ago by simon04

In 16765/josm:

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

comment:61 Changed 4 months ago by simon04

Milestone: 20.07Longterm

comment:62 in reply to:  53 Changed 3 months ago by 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…

comment:63 Changed 8 weeks ago by simon04

In 17135/josm:

see #16567 - Update to JUnit 5.7.0

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

comment:64 Changed 6 weeks ago by 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?

comment:65 in reply to:  64 ; Changed 6 weeks ago by 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?

comment:66 in reply to:  65 ; Changed 6 weeks ago by 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?

comment:67 in reply to:  66 Changed 5 weeks ago by 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.

comment:68 in reply to:  66 ; Changed 5 weeks ago by 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.

comment:69 in reply to:  68 ; Changed 5 weeks ago by 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?

comment:70 in reply to:  69 Changed 5 weeks ago by 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.

comment:71 Changed 5 weeks ago by 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

comment:72 Changed 5 weeks ago by Don-vip

In 17276/josm:

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

comment:73 Changed 5 weeks ago by Don-vip

In 17277/josm:

see #16567 - fix more obvious test errors

comment:74 Changed 5 weeks ago by 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?

comment:75 Changed 5 weeks ago by Don-vip

In 17279/josm:

see #16567 - revert r17275 for CycleLayerActionTest / MinimapDialogTest

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

comment:76 Changed 5 weeks ago by Don-vip

In 17280/josm:

see #16567 - fix assertEquals parameters order

comment:77 Changed 5 weeks ago by GerdP

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

comment:78 in reply to:  74 ; Changed 5 weeks ago by 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)

comment:79 in reply to:  77 Changed 5 weeks ago by 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. :)

comment:80 Changed 5 weeks ago by GerdP

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

comment:81 in reply to:  80 Changed 5 weeks ago by 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)

comment:82 Changed 5 weeks ago by GerdP

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

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

comment:83 in reply to:  82 Changed 5 weeks ago by 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 ).

comment:84 Changed 5 weeks ago by GerdP

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

comment:85 Changed 5 weeks ago by 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.

comment:86 in reply to:  78 ; Changed 5 weeks ago by 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 :(

comment:87 in reply to:  85 ; Changed 5 weeks ago by 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) ...

comment:88 in reply to:  86 ; Changed 5 weeks ago by 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?

comment:89 in reply to:  88 ; Changed 5 weeks ago by 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)

comment:90 in reply to:  87 Changed 5 weeks ago by 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.

comment:91 in reply to:  89 Changed 5 weeks ago by 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.

comment:92 Changed 5 weeks ago by Don-vip

In 17288/josm:

see #16567 - fix integration tests

comment:93 Changed 4 weeks ago by 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/

comment:94 Changed 3 weeks ago by 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

comment:95 Changed 3 weeks ago by 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	

comment:96 Changed 3 weeks ago by Don-vip

In 17310/josm:

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

comment:97 Changed 3 weeks ago by Don-vip

Interestingly, RenderingCLIAreaTest fails without using JMockit...

comment:98 Changed 3 weeks ago by Don-vip

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

comment:99 Changed 3 weeks ago by 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.

comment:100 in reply to:  99 Changed 3 weeks ago by Don-vip

Replying to Stereo:

16-ea builds seem to be failing right now.

See ticket:19724#comment:3

comment:101 Changed 2 weeks ago by Don-vip

In 35642/osm:

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

comment:103 Changed 8 days ago by simon04

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

comment:104 in reply to:  103 Changed 8 days ago by 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 :)

comment:105 Changed 5 days ago by 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.

comment:106 Changed 4 days ago by 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.

comment:107 Changed 4 days ago by Klumbumbus

Great.

comment:108 Changed 45 hours ago by 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.

comment:109 Changed 42 hours ago by Don-vip

In 17383/josm:

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

comment:110 Changed 41 hours ago by 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?

comment:111 in reply to:  110 Changed 41 hours ago by 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. :(

comment:112 Changed 41 hours ago by GerdP

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

comment:113 in reply to:  112 Changed 40 hours ago by 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.

comment:114 Changed 40 hours ago by Don-vip

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as reopened The owner will remain team.
as The resolution will be set.
to The owner will be changed from team to the specified user.
The owner will change to Don-vip
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.