From 2697fbc195ef428210925e4b05223e0ab64405a4 Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 5 Sep 2023 12:44:49 +0200 Subject: Rework stack trace trimming, using declared test class names --- .../com/yahoo/vespa/testrunner/JunitRunner.java | 5 +++ .../com/yahoo/vespa/testrunner/TestReport.java | 50 +++++++++++----------- .../testrunner/TestReportGeneratingListener.java | 5 ++- .../vespa/testrunner/AggregateTestRunnerTest.java | 23 +++++----- .../yahoo/vespa/testrunner/JunitRunnerTest.java | 20 ++++++--- .../vespa/testrunner/TestRunnerHandlerTest.java | 11 ++++- 6 files changed, 67 insertions(+), 47 deletions(-) (limited to 'vespa-osgi-testrunner') diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java index 21890bab569..6fd3400d41d 100644 --- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java @@ -22,6 +22,7 @@ import java.time.Clock; import java.util.Collection; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.SortedMap; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentSkipListMap; @@ -32,6 +33,8 @@ import java.util.logging.Level; import java.util.logging.LogRecord; import java.util.logging.Logger; +import static java.util.stream.Collectors.toSet; + /** * @author mortent @@ -115,9 +118,11 @@ public class JunitRunner extends AbstractComponent implements TestRunner { List> testClasses = classLoader.apply(suite); if (testClasses == null) return null; + Set testClassNames = testClasses.stream().map(Class::getName).collect(toSet()); testRuntimeProvider.initialize(testConfig); TestReportGeneratingListener testReportListener = new TestReportGeneratingListener(suite, + testClassNames, record -> logRecords.put(record.getSequenceNumber(), record), stdoutTee, stderrTee, diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java index a2ac86309d9..e967ddd8abe 100644 --- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java @@ -32,28 +32,30 @@ public class TestReport { private final Object monitor = new Object(); private final Set complete = new HashSet<>(); + private final Set testClassNames; private final Clock clock; private final ContainerNode root; private final Suite suite; private NamedNode current; private TestPlan plan; - private TestReport(Clock clock, Suite suite, ContainerNode root) { + private TestReport(Clock clock, Suite suite, Set testClassNames, ContainerNode root) { this.clock = clock; this.root = root; + this.testClassNames = Set.copyOf(testClassNames); this.current = root; this.suite = suite; } - TestReport(Clock clock, Suite suite) { - this(clock, suite, new ContainerNode(null, null, toString(suite), clock.instant())); + TestReport(Clock clock, Suite suite, Set testClassNames) { + this(clock, suite, testClassNames, new ContainerNode(null, null, toString(suite), clock.instant())); } static TestReport createFailed(Clock clock, Suite suite, Throwable thrown) { if (thrown instanceof OutOfMemoryError) throw (Error) thrown; - TestReport failed = new TestReport(clock, suite); + TestReport failed = new TestReport(clock, suite, Set.of()); failed.complete(); - failed.root().children.add(new FailureNode(failed.root(), clock.instant(), thrown, suite)); + failed.root().children.add(new FailureNode(failed.root(), clock.instant(), thrown, suite, Set.of())); return failed; } @@ -127,7 +129,7 @@ public class TestReport { synchronized (monitor) { Status status = Status.successful; if (thrown != null) { - FailureNode failure = new FailureNode(current, clock.instant(), thrown, suite); + FailureNode failure = new FailureNode(current, clock.instant(), thrown, suite, testClassNames); current.children.add(failure); status = failure.status(); } @@ -138,7 +140,7 @@ public class TestReport { void log(LogRecord record) { synchronized (monitor) { - if (record.getThrown() != null) trimStackTraces(record.getThrown(), JunitRunner.class.getName()); + if (record.getThrown() != null) trimStackTraces(record.getThrown(), testClassNames); if ( ! (current.children.peekLast() instanceof OutputNode)) current.children.add(new OutputNode(current)); @@ -158,7 +160,9 @@ public class TestReport { ContainerNode newRoot = new ContainerNode(null, null, root.name(), root.start()); newRoot.children.addAll(root.children); newRoot.children.addAll(other.root.children); - TestReport merged = new TestReport(clock, suite, newRoot); + Set testClassNames = new HashSet<>(this.testClassNames); + testClassNames.addAll(other.testClassNames); + TestReport merged = new TestReport(clock, suite, testClassNames, newRoot); merged.complete(); return merged; } @@ -277,9 +281,9 @@ public class TestReport { private final Throwable thrown; private final Suite suite; - public FailureNode(NamedNode parent, Instant now, Throwable thrown, Suite suite) { + public FailureNode(NamedNode parent, Instant now, Throwable thrown, Suite suite, Set testClassNames) { super(parent, null, thrown.toString(), now); - trimStackTraces(thrown, JunitRunner.class.getName()); + trimStackTraces(thrown, testClassNames); this.thrown = thrown; this.suite = suite; @@ -327,26 +331,20 @@ public class TestReport { /** * Recursively trims stack traces for the given throwable and its causes/suppressed. - * This is based on the assumption that the relevant stack is anything above the first native - * reflection invocation, above any frame in the given root class. + * This is based on the assumption that the relevant stack is anything from the first + * test bundle class frame, and upwards; the exception is for dynamic tests, where a + * specific dynamic test factory method is right below the first user frame. */ - static void trimStackTraces(Throwable thrown, String testFrameworkRootClass) { + static void trimStackTraces(Throwable thrown, Set testClassNames) { if (thrown == null) return; StackTraceElement[] stack = thrown.getStackTrace(); - int i = 0; - int firstReflectFrame = -1; - int cutoff = 0; - boolean rootedInTestFramework = false; + int i = -1; + int cutoff = stack.length; while (++i < stack.length) { - rootedInTestFramework |= testFrameworkRootClass.equals(stack[i].getClassName()); - if (firstReflectFrame == -1 && stack[i].getClassName().startsWith("jdk.internal.reflect.")) - firstReflectFrame = i; // jdk.internal.reflect class invokes the first user test frame, on both jdk 17 and 21. - if (rootedInTestFramework && firstReflectFrame > 0) { - cutoff = firstReflectFrame; - break; - } + for (String name : testClassNames) if (stack[i].getClassName().startsWith(name)) + cutoff = i + 1; boolean isDynamicTestInvocation = "org.junit.jupiter.engine.descriptor.DynamicTestTestDescriptor".equals(stack[i].getClassName()); if (isDynamicTestInvocation) { cutoff = i; @@ -356,9 +354,9 @@ public class TestReport { thrown.setStackTrace(copyOf(stack, cutoff)); for (Throwable suppressed : thrown.getSuppressed()) - trimStackTraces(suppressed, testFrameworkRootClass); + trimStackTraces(suppressed, testClassNames); - trimStackTraces(thrown.getCause(), testFrameworkRootClass); + trimStackTraces(thrown.getCause(), testClassNames); } private static String toString(Suite suite) { diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReportGeneratingListener.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReportGeneratingListener.java index 5bc9fda6835..487cc8c501a 100644 --- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReportGeneratingListener.java +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReportGeneratingListener.java @@ -18,6 +18,7 @@ import java.time.Clock; import java.time.ZoneOffset; import java.util.HashMap; import java.util.Map; +import java.util.Set; import java.util.function.Consumer; import java.util.logging.Handler; import java.util.logging.Level; @@ -41,8 +42,8 @@ class TestReportGeneratingListener implements TestExecutionListener { private final Handler handler; // Captures logging from test code. private final Clock clock; - TestReportGeneratingListener(Suite suite, Consumer logger, TeeStream stdoutTee, TeeStream stderrTee, Clock clock) { - this.report = new TestReport(clock, suite); + TestReportGeneratingListener(Suite suite, Set testClassNames, Consumer logger, TeeStream stdoutTee, TeeStream stderrTee, Clock clock) { + this.report = new TestReport(clock, suite, testClassNames); this.logger = logger; this.stdoutTee = stdoutTee; this.stderrTee = stderrTee; diff --git a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java index 2690c89e1c1..0c55645eb33 100644 --- a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java +++ b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java @@ -12,6 +12,7 @@ import java.time.Clock; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.logging.Level; import java.util.logging.LogRecord; @@ -147,11 +148,6 @@ class AggregateTestRunnerTest { merged.root().tally().getOrDefault(status, 0L)); } - @Test - void testReportStatus() { - assertEquals(FAILURE, JunitRunner.testRunnerStatus(TestReport.createFailed(Clock.systemUTC(), Suite.SYSTEM_TEST, new RuntimeException("hello")))); - } - @Test void testStackTrimming() { try { @@ -163,17 +159,22 @@ class AggregateTestRunnerTest { } } catch (Exception e) { - TestReport.trimStackTraces(e, "org.junit.platform.commons.util.ReflectionUtils"); + TestReport.trimStackTraces(e, Set.of(AggregateTestRunnerTest.class.getName())); assertEquals(""" - java.lang.RuntimeException: java.lang.RuntimeException: inner - \tat com.yahoo.vespa.testrunner.AggregateTestRunnerTest.testStackTrimming(AggregateTestRunnerTest.java:162) - Caused by: java.lang.RuntimeException: inner - \tat com.yahoo.vespa.testrunner.AggregateTestRunnerTest.testStackTrimming(AggregateTestRunnerTest.java:159) - """, + java.lang.RuntimeException: java.lang.RuntimeException: inner + at com.yahoo.vespa.testrunner.AggregateTestRunnerTest.testStackTrimming(AggregateTestRunnerTest.java:158) + Caused by: java.lang.RuntimeException: inner + at com.yahoo.vespa.testrunner.AggregateTestRunnerTest.testStackTrimming(AggregateTestRunnerTest.java:155) + """, ExceptionUtils.getStackTraceAsString(e)); } } + @Test + void testReportStatus() { + assertEquals(FAILURE, JunitRunner.testRunnerStatus(TestReport.createFailed(Clock.systemUTC(), Suite.SYSTEM_TEST, new RuntimeException("hello")))); + } + static class MockTestRunner implements TestRunner { final List log = new ArrayList<>(); diff --git a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/JunitRunnerTest.java b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/JunitRunnerTest.java index 12700040f60..6878579b950 100644 --- a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/JunitRunnerTest.java +++ b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/JunitRunnerTest.java @@ -28,13 +28,18 @@ import java.io.BufferedReader; import java.io.InputStream; import java.io.InputStreamReader; import java.lang.annotation.Annotation; +import java.net.URL; +import java.net.URLClassLoader; import java.time.Clock; import java.time.Instant; import java.time.ZoneId; +import java.util.ArrayList; +import java.util.Arrays; import java.util.EnumMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.UnaryOperator; import java.util.stream.Stream; import static ai.vespa.hosted.cd.internal.TestRuntimeProvider.testRuntime; @@ -97,17 +102,20 @@ class JunitRunnerTest { } static TestRunner test(Suite suite, byte[] testConfig, Class... testClasses) { - JunitRunner runner = new JunitRunner(clock, - config -> { assertSame(testConfig, config); testRuntime.set(new MockTestRuntime()); }, - __ -> List.of(testClasses), - JunitRunnerTest::execute); try { + JunitRunner runner = new JunitRunner(clock, + config -> { + assertSame(testConfig, config); + testRuntime.set(new MockTestRuntime()); + }, + __ -> List.of(testClasses), + JunitRunnerTest::execute); runner.test(suite, testConfig).get(); + return runner; } catch (Exception e) { - fail(e); + throw new AssertionError(e); } - return runner; } diff --git a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/TestRunnerHandlerTest.java b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/TestRunnerHandlerTest.java index 6d6fbbf2cf1..9edd9a23c68 100644 --- a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/TestRunnerHandlerTest.java +++ b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/TestRunnerHandlerTest.java @@ -19,10 +19,15 @@ import org.junit.jupiter.api.Test; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.UncheckedIOException; +import java.net.MalformedURLException; +import java.net.URISyntaxException; +import java.net.URL; +import java.net.URLClassLoader; import java.time.Clock; import java.time.Instant; import java.time.ZoneId; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -47,16 +52,18 @@ class TestRunnerHandlerTest { private TestRunner aggregateRunner; @BeforeEach - void setup() { + void setup() throws Exception { TestReport moreTestsReport = JunitRunnerTest.test(Suite.PRODUCTION_TEST, new byte[0], FailingTestAndBothAftersTest.class, WrongBeforeAllTest.class, FailingExtensionTest.class) .getReport(); + Exception cnfe = new ClassNotFoundException("School's out all summer!"); + cnfe.setStackTrace(Arrays.copyOf(cnfe.getStackTrace(), 1)); TestReport failedReport = TestReport.createFailed(Clock.fixed(testInstant, ZoneId.of("UTC")), Suite.PRODUCTION_TEST, - new ClassNotFoundException("School's out all summer!")); + cnfe); aggregateRunner = AggregateTestRunner.of(List.of(new MockRunner(TestRunner.Status.SUCCESS, AggregateTestRunnerTest.report.mergedWith(moreTestsReport) .mergedWith(failedReport)))); -- cgit v1.2.3