Index: test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java =================================================================== --- test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java (revision 17428) +++ test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java (working copy) @@ -1,25 +1,31 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.testutils; +import static org.junit.jupiter.api.Assertions.fail; + import java.awt.Color; import java.awt.Window; import java.awt.event.WindowEvent; import java.io.ByteArrayInputStream; -import java.io.File; import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; +import java.lang.annotation.Annotation; import java.lang.annotation.Documented; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.lang.reflect.AnnotatedElement; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.security.GeneralSecurityException; import java.text.MessageFormat; import java.util.Arrays; import java.util.Map; import java.util.TimeZone; +import java.util.UUID; import java.util.logging.Handler; import org.awaitility.Awaitility; @@ -27,7 +33,9 @@ import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.BeforeEachCallback; +import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.ExtensionContext.Store; import org.junit.rules.TemporaryFolder; import org.junit.rules.TestRule; import org.junit.runner.Description; @@ -85,6 +93,7 @@ */ public class JOSMTestRules implements TestRule, AfterEachCallback, BeforeEachCallback, AfterAllCallback, BeforeAllCallback { private int timeout = isDebugMode() ? -1 : 10 * 1000; + private final Path josmHomeTemp; private TemporaryFolder josmHome; private boolean usePreferences = false; private APIType useAPI = APIType.NONE; @@ -105,6 +114,7 @@ private boolean territories; private boolean metric; private boolean main; + /** * This boolean is only used to indicate if JUnit5 is used in a test. If it is, * we must not call after in {@link JOSMTestRules.CreateJosmEnvironment#evaluate}. @@ -112,7 +122,22 @@ */ private boolean junit5; + private FailOnTimeoutStatement failOnTimeout; + /** + * Create a new JOSMTestRules object + */ + public JOSMTestRules() { + Path tempHome = null; + try { + tempHome = Files.createTempDirectory("josm-test-rules-home"); + } catch (IOException e) { + fail(e); + } + this.josmHomeTemp = tempHome; + } + + /** * Disable the default timeout for this test. Use with care. * @return this instance, for easy chaining */ @@ -446,6 +471,8 @@ @Override public void beforeEach(ExtensionContext context) throws Exception { this.junit5 = true; + + // Temporary statement is necessary until we are able to remove JUnit4. Statement temporaryStatement = new Statement() { @Override public void evaluate() throws Throwable { @@ -452,10 +479,25 @@ // do nothing } }; + final Store store = context.getStore(ExtensionContext.Namespace.create(JOSMTestRules.class)); + // First process any Override* annotations for per-test overrides. + // The following only work because "option" methods modify JOSMTestRules in-place + final String tempRevision = store.getOrDefault(OverrideAssumeRevisionExtension.KEY, String.class, null); + if (tempRevision != null) { + this.assumeRevision(tempRevision); + } + + final Integer tempTimeout = store.getOrDefault(OverrideTimeoutExtension.KEY, Integer.class, null); + if (tempTimeout != null) { + this.timeout(tempTimeout); + } try { - this.apply(temporaryStatement, - Description.createTestDescription(this.getClass(), "JOSMTestRules JUnit5 Compatibility")) - .evaluate(); + beforeEachTest(); + + if (this.timeout > 0) { + this.failOnTimeout = new FailOnTimeoutStatement(temporaryStatement, this.timeout); + this.failOnTimeout.beforeEach(context); + } } catch (Throwable e) { throw new Exception(e); } @@ -463,17 +505,26 @@ @Override public void afterEach(ExtensionContext context) throws Exception { + if (this.failOnTimeout != null) { + this.failOnTimeout.afterEach(context); + } after(); } @Override public void beforeAll(ExtensionContext context) throws Exception { - beforeEach(context); + if (this.tileSourceRule != null) { + this.tileSourceRule.beforeAll(context); + } + this.beforeAllTests(); } @Override public void afterAll(ExtensionContext context) throws Exception { afterEach(context); + if (this.tileSourceRule != null) { + this.tileSourceRule.afterAll(context); + } } /** @@ -481,9 +532,7 @@ * @throws InitializationError If an error occurred while creating the required environment. * @throws ReflectiveOperationException if a reflective access error occurs */ - protected void before() throws InitializationError, ReflectiveOperationException { - cleanUpFromJosmFixture(); - + protected void beforeEachTest() throws InitializationError, ReflectiveOperationException { if (this.assumeRevisionString != null) { this.originalVersion = Version.getInstance(); final Version replacementVersion = new MockVersion(this.assumeRevisionString); @@ -490,11 +539,14 @@ TestUtils.setPrivateStaticField(Version.class, "instance", replacementVersion); } - // Add JOSM home - if (josmHome != null) { + // Add JOSM home. Take advantage of the fact that josmHome is only initialized with preferences. + // And is always initialized with them. + if (this.josmHome != null && this.josmHomeTemp != null) { try { - File home = josmHome.newFolder(); - System.setProperty("josm.home", home.getAbsolutePath()); + // TODO When JUnit4 support is dropped, use {@link ExtensionContext#getUniqueId} + // Currently, assume that the temporary directory is automatically cleared on boot. + final Path home = Files.createTempDirectory(josmHomeTemp, UUID.randomUUID().toString()); + System.setProperty("josm.home", home.toAbsolutePath().toString()); JosmBaseDirectories.getInstance().clearMemos(); } catch (IOException e) { throw new InitializationError(e); @@ -622,6 +674,10 @@ } } + protected void beforeAllTests() { + cleanUpFromJosmFixture(); + } + /** * Clean up what test not using these test rules may have broken. */ @@ -704,7 +760,8 @@ @Override public void evaluate() throws Throwable { - before(); + beforeAllTests(); + beforeEachTest(); try { base.evaluate(); } finally { @@ -723,10 +780,11 @@ * The junit timeout statement has problems when switching timezones. This one does not. * @author Michael Zangl */ - private static class FailOnTimeoutStatement extends Statement { + private static class FailOnTimeoutStatement extends Statement implements BeforeEachCallback, AfterEachCallback { private final int timeout; private final Statement original; + private TimeoutThread thread; FailOnTimeoutStatement(Statement original, int timeout) { this.original = original; @@ -735,15 +793,21 @@ @Override public void evaluate() throws Throwable { - TimeoutThread thread = new TimeoutThread(original); - thread.setDaemon(true); - thread.start(); + beforeEach(null); + afterEach(null); + } + + @Override + public void afterEach(ExtensionContext context) throws Exception { thread.join(timeout); thread.interrupt(); if (!thread.isDone) { Throwable exception = thread.getExecutionException(); if (exception != null) { - throw exception; + if (exception instanceof Exception) { + throw (Exception) exception; + } + throw new Exception(exception); } else { if (Logging.isLoggingEnabled(Logging.LEVEL_DEBUG)) { // i.e. skip expensive formatting of stack trace if it won't be shown @@ -754,7 +818,16 @@ throw new Exception(MessageFormat.format("Test timed out after {0}ms", timeout)); } } + + } + + @Override + public void beforeEach(ExtensionContext context) throws Exception { + this.thread = new TimeoutThread(original); + this.thread.setDaemon(true); + this.thread.start(); + } } private static final class TimeoutThread extends Thread { @@ -787,10 +860,37 @@ getInputArguments().toString().indexOf("-agentlib:jdwp") > 0; } + private static T handleAnnotation(ExtensionContext context, Class annotation) { + AnnotatedElement element = context.getElement().orElse(null); + ExtensionContext currentContext = context.getParent().orElse(null); + while (element == null && currentContext != null) { + element = currentContext.getElement().orElse(null); + currentContext = currentContext.getParent().orElse(null); + } + + if (element != null) { + return element.getAnnotation(annotation); + } + return null; + } + + class OverrideAssumeRevisionExtension implements BeforeEachCallback { + static final String KEY = "revision"; + @Override + public void beforeEach(ExtensionContext context) throws Exception { + OverrideAssumeRevision annotation = handleAnnotation(context, OverrideAssumeRevision.class); + if (annotation != null) { + context.getStore(ExtensionContext.Namespace.create(JOSMTestRules.class)).put(KEY, annotation.value()); + } + } + + } + /** * Override this test's assumed JOSM version (as reported by {@link Version}). * @see JOSMTestRules#assumeRevision(String) */ + @ExtendWith(OverrideAssumeRevisionExtension.class) @Documented @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) @@ -802,10 +902,22 @@ String value(); } + class OverrideTimeoutExtension implements BeforeEachCallback { + static final String KEY = "timeout"; + @Override + public void beforeEach(ExtensionContext context) throws Exception { + OverrideTimeout annotation = handleAnnotation(context, OverrideTimeout.class); + if (annotation != null) { + context.getStore(ExtensionContext.Namespace.create(JOSMTestRules.class)).put(KEY, annotation.value()); + } + } + } + /** * Override this test's timeout. * @see JOSMTestRules#timeout(int) */ + @ExtendWith(OverrideTimeoutExtension.class) @Documented @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) Index: test/unit/org/openstreetmap/josm/testutils/TileSourceRule.java =================================================================== --- test/unit/org/openstreetmap/josm/testutils/TileSourceRule.java (revision 17428) +++ test/unit/org/openstreetmap/josm/testutils/TileSourceRule.java (working copy) @@ -2,6 +2,7 @@ package org.openstreetmap.josm.testutils; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; +import static org.junit.jupiter.api.Assertions.fail; import static org.openstreetmap.josm.TestUtils.getPrivateStaticField; import java.awt.Color; @@ -10,17 +11,23 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Objects; +import java.util.stream.Collectors; import javax.imageio.ImageIO; +import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.BeforeAllCallback; +import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.runner.Description; import org.junit.runners.model.Statement; import org.openstreetmap.josm.data.imagery.ImageryInfo; import org.openstreetmap.josm.data.imagery.ImageryLayerInfo; +import org.openstreetmap.josm.gui.bbox.JosmMapViewer.TileSourceProvider; import org.openstreetmap.josm.gui.bbox.SlippyMapBBoxChooser; import org.openstreetmap.josm.tools.Logging; @@ -28,12 +35,13 @@ import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder; import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.junit.WireMockRule; +import com.github.tomakehurst.wiremock.verification.LoggedRequest; /** * A JUnit rule, based on {@link WireMockRule} to provide a test with a simple mock tile server serving multiple tile * sources. */ -public class TileSourceRule extends WireMockRule { +public class TileSourceRule extends WireMockRule implements BeforeAllCallback, AfterAllCallback { private static class ByteArrayWrapper { public final byte[] byteArray; @@ -165,6 +173,9 @@ protected final boolean clearLayerList; protected final boolean clearSlippyMapSources; protected final boolean registerInLayerList; + private List slippyMapProviders; + private TileSourceProvider slippyMapDefaultProvider; + private List originalImageryInfoList; /** * Construct a TileSourceRule for use with a JUnit test. @@ -238,19 +249,23 @@ return super.apply(new Statement() { @Override public void evaluate() throws Throwable { - try { - // a hack to circumvent a WireMock bug concerning delayed server startup. sending an early request - // to the mock server seems to prompt it to start earlier (though this request itself is not - // expected to succeed). see https://github.com/tomakehurst/wiremock/issues/97 - new java.net.URL(TileSourceRule.this.url("/_poke")).getContent(); - } catch (IOException e) { - Logging.trace(e); - } + applyRunServerEarlyStart(); base.evaluate(); } }, description); } + private void applyRunServerEarlyStart() { + try { + // a hack to circumvent a WireMock bug concerning delayed server startup. sending an early request + // to the mock server seems to prompt it to start earlier (though this request itself is not + // expected to succeed). see https://github.com/tomakehurst/wiremock/issues/97 + new java.net.URL(TileSourceRule.this.url("/_poke")).getContent(); + } catch (IOException e) { + Logging.trace(e); + } + } + /** * A junit-rule {@code apply} method exposed separately, containing initialization steps which can only be performed * once more of josm's environment has been set up. @@ -264,45 +279,12 @@ if (this.registerInLayerList || this.clearLayerList) { return new Statement() { @Override - @SuppressWarnings("unchecked") public void evaluate() throws Throwable { - List slippyMapProviders = null; - SlippyMapBBoxChooser.TileSourceProvider slippyMapDefaultProvider = null; - List originalImageryInfoList = null; - if (TileSourceRule.this.clearSlippyMapSources) { - try { - slippyMapProviders = (List) getPrivateStaticField( - SlippyMapBBoxChooser.class, - "providers" - ); - // pop this off the beginning of the list, keep for later - slippyMapDefaultProvider = slippyMapProviders.remove(0); - } catch (ReflectiveOperationException e) { - Logging.warn("Failed to remove default SlippyMapBBoxChooser TileSourceProvider"); - } - } - - if (TileSourceRule.this.clearLayerList) { - originalImageryInfoList = ImageryLayerInfo.instance.getLayers(); - ImageryLayerInfo.instance.clear(); - } - if (TileSourceRule.this.registerInLayerList) { - for (ConstSource source : TileSourceRule.this.sourcesList) { - ImageryLayerInfo.addLayer(source.getImageryInfo(TileSourceRule.this.port())); - } - } - + realBeforeRegisterLayers(); try { base.evaluate(); } finally { - // clean up to original state - if (slippyMapDefaultProvider != null && slippyMapProviders != null) { - slippyMapProviders.add(0, slippyMapDefaultProvider); - } - if (originalImageryInfoList != null) { - ImageryLayerInfo.instance.clear(); - ImageryLayerInfo.addLayers(originalImageryInfoList); - } + realAfterRegisterLayers(); } } }; @@ -311,6 +293,46 @@ } } + @SuppressWarnings("unchecked") + private void realBeforeRegisterLayers() { + if (this.registerInLayerList || this.clearLayerList) { + List slippyMapProviders = null; + if (TileSourceRule.this.clearSlippyMapSources) { + try { + slippyMapProviders = (List) getPrivateStaticField( + SlippyMapBBoxChooser.class, + "providers" + ); + // pop this off the beginning of the list, keep for later + this.slippyMapDefaultProvider = slippyMapProviders.remove(0); + } catch (ReflectiveOperationException e) { + Logging.warn("Failed to remove default SlippyMapBBoxChooser TileSourceProvider"); + } + } + + if (TileSourceRule.this.clearLayerList) { + originalImageryInfoList = ImageryLayerInfo.instance.getLayers(); + ImageryLayerInfo.instance.clear(); + } + if (TileSourceRule.this.registerInLayerList) { + for (ConstSource source : TileSourceRule.this.sourcesList) { + ImageryLayerInfo.addLayer(source.getImageryInfo(TileSourceRule.this.port())); + } + } + } + } + + private void realAfterRegisterLayers() { + // clean up to original state + if (slippyMapDefaultProvider != null && slippyMapProviders != null) { + slippyMapProviders.add(0, slippyMapDefaultProvider); + } + if (originalImageryInfoList != null) { + ImageryLayerInfo.instance.clear(); + ImageryLayerInfo.addLayers(originalImageryInfoList); + } + } + /** * A standard implementation of apply which simply calls both sub- {@code apply} methods, {@link #applyRunServer} * and {@link #applyRegisterLayers}. Called when used as a standard junit rule. @@ -319,4 +341,25 @@ public Statement apply(Statement base, Description description) { return applyRunServer(applyRegisterLayers(base, description), description); } + + @Override + public void afterAll(ExtensionContext context) throws Exception { + after(); + Collection missedRequests = super.findAllUnmatchedRequests(); + if (!missedRequests.isEmpty()) { + String message = missedRequests.stream().map(m -> m.getUrl()).collect(Collectors.joining("\n\n")); + fail(message); + } + stop(); + realAfterRegisterLayers(); + } + + @Override + public void beforeAll(ExtensionContext context) throws Exception { + super.start(); + WireMock.configureFor("localhost", port()); + before(); + applyRunServerEarlyStart(); + realBeforeRegisterLayers(); + } }