Ticket #16567: 16567.separate_all_each.1.patch

test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java

@@ -1 +1 @@
 // 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 +33 @@
 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 +93 @@
  */
 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 +114 @@
     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 +122 @@
      */
     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 +471 @@
     @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 +479 @@
                 // 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 +505 @@
 
     @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 +532 @@
      * @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 +539 @@
             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 +674 @@
         }
     }
 
+    protected void beforeAllTests() {
+        cleanUpFromJosmFixture();
+    }
+
     /**
      * Clean up what test not using these test rules may have broken.
      */
@@ -704 +760 @@
 
         @Override
         public void evaluate() throws Throwable {
-            before();
+            beforeAllTests();
+            beforeEachTest();
             try {
                 base.evaluate();
             } finally {
@@ -723 +780 @@
      * 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 +793 @@
 
         @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 +818 @@
                     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 +860 @@
                 getInputArguments().toString().indexOf("-agentlib:jdwp") > 0;
     }
 
+    private static <T extends Annotation> T handleAnnotation(ExtensionContext context, Class<T> 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 +902 @@
         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)

test/unit/org/openstreetmap/josm/testutils/TileSourceRule.java

@@ -2 +2 @@
 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 +11 @@
 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 +35 @@
 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 +173 @@
     protected final boolean clearLayerList;
     protected final boolean clearSlippyMapSources;
     protected final boolean registerInLayerList;
+    private List<TileSourceProvider> slippyMapProviders;
+    private TileSourceProvider slippyMapDefaultProvider;
+    private List<ImageryInfo> originalImageryInfoList;
 
     /**
      * Construct a TileSourceRule for use with a JUnit test.
@@ -238 +249 @@
         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 +279 @@
         if (this.registerInLayerList || this.clearLayerList) {
             return new Statement() {
                 @Override
-                @SuppressWarnings("unchecked")
                 public void evaluate() throws Throwable {
-                    List<SlippyMapBBoxChooser.TileSourceProvider> slippyMapProviders = null;
-                    SlippyMapBBoxChooser.TileSourceProvider slippyMapDefaultProvider = null;
-                    List<ImageryInfo> originalImageryInfoList = null;
-                    if (TileSourceRule.this.clearSlippyMapSources) {
-                        try {
-                            slippyMapProviders = (List<SlippyMapBBoxChooser.TileSourceProvider>) 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 +293 @@
         }
     }
 
+    @SuppressWarnings("unchecked")
+    private void realBeforeRegisterLayers() {
+        if (this.registerInLayerList || this.clearLayerList) {
+            List<SlippyMapBBoxChooser.TileSourceProvider> slippyMapProviders = null;
+            if (TileSourceRule.this.clearSlippyMapSources) {
+                try {
+                    slippyMapProviders = (List<SlippyMapBBoxChooser.TileSourceProvider>) 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 +341 @@
     public Statement apply(Statement base, Description description) {
         return applyRunServer(applyRegisterLayers(base, description), description);
     }
+
+    @Override
+    public void afterAll(ExtensionContext context) throws Exception {
+        after();
+        Collection<LoggedRequest> 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();
+    }
 }