Changeset 18549 in josm for trunk/test/unit/org/openstreetmap

Timestamp:

2022-09-06T18:40:21+02:00 (2 years ago)

Author:

taylor.smock

Message:

See #22183: NoClassDefFoundError: Could not initialize class org.openstreetmap.josm.actions.SessionSaveAction

The root cause is a suppressed NPE whose stack trace is not given
in the bug report.

This adds a method to BugReport to add suppressed exceptions to a report.
In the future, if we keep the code long-term, methods in Logging which take
exceptions may assume the exception is suppressed, and add it to the
suppressed exceptions.

File:

• trunk/test/unit/org/openstreetmap/josm/tools/bugreport/BugReportTest.java (modified) (4 diffs)

trunk/test/unit/org/openstreetmap/josm/tools/bugreport/BugReportTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.tools.bugreport;
 
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -9 +12 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
-
+import java.lang.reflect.Field;
+import java.util.function.Consumer;
+import java.util.logging.Handler;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.platform.commons.util.ReflectionUtils;
 import org.openstreetmap.josm.actions.ShowStatusReportAction;
+import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
+import org.openstreetmap.josm.tools.Logging;
 
 /**
@@ -21 +39 @@
 @BasicPreferences
 class BugReportTest {
+    private static Handler[] handlers;
+
+    @AfterAll
+    static void cleanup() {
+        // Clear queue
+        new BugReport(BugReport.intercept(new NullPointerException())).getReportText("");
+        Logging.clearLastErrorAndWarnings();
+        for (Handler handler : handlers) {
+            Logging.getLogger().addHandler(handler);
+        }
+    }
+
+    @BeforeAll
+    static void setup() {
+        handlers = Logging.getLogger().getHandlers();
+        // Avoid console spam
+        for (Handler handler : handlers) {
+            Logging.getLogger().removeHandler(handler);
+        }
+    }
+
     /**
      * Test {@link BugReport#getReportText}
@@ -70 +109 @@
         return BugReport.getCallingMethod(2);
     }
+
+    @Test
+    void testSuppressedExceptionsOrder() {
+        final String methodName = "testSuppressedExceptionsOrder";
+        BugReport.addSuppressedException(new NullPointerException(methodName));
+        BugReport.addSuppressedException(new IllegalStateException(methodName));
+        BugReport bugReport = new BugReport(BugReport.intercept(new IOException(methodName)));
+        final String report = assertDoesNotThrow(() -> bugReport.getReportText(methodName));
+        assertAll(() -> assertTrue(report.contains("NullPointerException")),
+                () -> assertTrue(report.contains("IOException")),
+                () -> assertTrue(report.contains("IllegalStateException")));
+        int ioe = report.indexOf("IOException");
+        int npe = report.indexOf("NullPointerException");
+        int ise = report.indexOf("IllegalStateException");
+        assertAll("Ordering of exceptions is wrong",
+                () -> assertTrue(ioe < npe, "IOException should be reported before NullPointerException"),
+                () -> assertTrue(npe < ise, "NullPointerException should be reported before IllegalStateException"));
+    }
+
+    static Stream<Arguments> testSuppressedExceptions() {
+        return Stream.of(
+                Arguments.of("GuiHelper::runInEDTAndWaitAndReturn",
+                        (Consumer<Runnable>) r -> GuiHelper.runInEDTAndWaitAndReturn(() -> {
+                            r.run();
+                            return null;
+                        })),
+                Arguments.of("GuiHelper::runInEDTAndWait", (Consumer<Runnable>) GuiHelper::runInEDTAndWait),
+                Arguments.of("MainApplication.worker", (Consumer<Runnable>) MainApplication.worker::execute)
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource
+    void testSuppressedExceptions(String workerName, Consumer<Runnable> worker) {
+        // Throw a npe in the worker. Workers might give us the exception, wrapped or otherwise.
+        try {
+            worker.accept(() -> {
+                throw new NullPointerException();
+            });
+        } catch (Exception e) {
+            // pass. MainApplication.worker can continue throwing the NPE;
+            Logging.trace(e);
+        }
+        // Ensure that the threads are synced
+        assertDoesNotThrow(() -> worker.accept(() -> { /* sync */ }));
+        // Now throw an exception
+        BugReport bugReport = new BugReport(BugReport.intercept(new IOException("testSuppressedExceptions")));
+        String report = bugReport.getReportText(workerName);
+        assertTrue(report.contains("IOException"));
+        assertTrue(report.contains("NullPointerException"));
+    }
+
+    @Test
+    void testSuppressedExceptionsReportedOnce() {
+        // Add the exception
+        BugReport.addSuppressedException(new NullPointerException("testSuppressedExceptionsReportedOnce"));
+        BugReport bugReport = new BugReport(BugReport.intercept(new IOException("testSuppressedExceptionsReportedOnce")));
+        // Get the report which clears the suppressed exceptions
+        String report = bugReport.getReportText("");
+        assertTrue(report.contains("IOException"));
+        assertTrue(report.contains("NullPointerException"));
+
+        BugReport bugReport2 = new BugReport(BugReport.intercept(new IOException("testSuppressedExceptionsReportedOnce")));
+        String report2 = bugReport2.getReportText("");
+        assertTrue(report2.contains("IOException"));
+        assertFalse(report2.contains("NullPointerException"));
+    }
+
+    @Test
+    void testManyExceptions() throws ReflectiveOperationException {
+        Field suppressedExceptions = BugReport.class.getDeclaredField("MAXIMUM_SUPPRESSED_EXCEPTIONS");
+        ReflectionUtils.makeAccessible(suppressedExceptions);
+        final byte expected = suppressedExceptions.getByte(null);
+        final int end = 2 * expected;
+        // Add many suppressed exceptions
+        for (int i = 0; i < end; i++) {
+            BugReport.addSuppressedException(new NullPointerException("NPE: " + i));
+        }
+        BugReport bugReport = new BugReport(BugReport.intercept(new IOException("testManyExceptions")));
+        String report = bugReport.getReportText("");
+        Matcher matcher = Pattern.compile("NPE: (\\d+)").matcher(report);
+        for (int i = end - expected; i < end; ++i) {
+            assertTrue(matcher.find());
+            assertEquals(Integer.toString(i), matcher.group(1));
+        }
+        assertFalse(matcher.find());
+    }
+
+    @Test
+    void testNullException() {
+        // This should add a NPE to the suppressed exceptions
+        assertDoesNotThrow(() -> BugReport.addSuppressedException(null));
+        BugReport bugReport = new BugReport(BugReport.intercept(new IOException("testNullException")));
+        // Getting the report text should not throw an exception.
+        String report = assertDoesNotThrow(() -> bugReport.getReportText(""));
+        assertTrue(report.contains("IOException"));
+        assertTrue(report.contains("NullPointerException"));
+    }
 }