diff --git a/agent/bin/test_projects b/agent/bin/test_projects index 00e56800..137147f1 100755 --- a/agent/bin/test_projects +++ b/agent/bin/test_projects @@ -6,14 +6,9 @@ mkdir -p build/fixtures source "test/helper.bash" export ANNOTATION_JAR="$(find_annotation_jar)" -function is_old_java { - local version="$1" - [[ "$version" == 1.8* ]] || [[ "$version" == 11.* ]] -} - function install_petclinic ( local repo="$1"; shift - local branch=${1:-main} + local ref=${1:-main} local pkg="$(basename $repo)" if [[ -d "build/fixtures/${pkg}" ]]; then @@ -24,8 +19,28 @@ function install_petclinic ( cd build/fixtures rm -rf "${pkg}" - git clone https://github.com/"${repo}".git --depth 1 --branch "${branch}" - cd "${pkg}" + + if [[ "${#ref}" == 40 ]]; then + # It's a commit hash, download archive + echo "Downloading archive for ${repo} at commit ${ref}" + curl -L "https://github.com/${repo}/archive/${ref}.tar.gz" | tar xz + mv "${pkg}-${ref}" "${pkg}" + cd "${pkg}" + # The archive doesn't include git history, but the tests rely on it for + # metadata. We create a fresh git repo to satisfy the tests. + git init + git config user.email "test@example.com" + git config user.name "Test User" + git remote add origin "https://github.com/${repo}.git" + git add . + git commit -m "Initial commit" + else + # It's a branch, use git clone + echo "Cloning ${repo} at branch ${ref}" + git clone https://github.com/"${repo}".git --depth 1 --branch "${ref}" + cd "${pkg}" + fi + cd ../../.. ) @@ -54,12 +69,20 @@ function install_scala_test_app { cd ../../.. } -if is_old_java "$JAVA_VERSION"; then - install_petclinic "land-of-apps/spring-petclinic" old-java-support -else - install_petclinic "spring-projects/spring-petclinic" - install_petclinic "spring-petclinic/spring-framework-petclinic" -fi +case "${JAVA_VERSION}" in + 1.8*|11.*) + install_petclinic "land-of-apps/spring-petclinic" old-java-support + ;; + 17.*) + # The spring-petclinic main branch now requires Java 25. This is the last commit that supports Java 17. + install_petclinic "spring-projects/spring-petclinic" "3aa79e3944ab1b626288f5d0629e61643ab8fb4a" + install_petclinic "spring-petclinic/spring-framework-petclinic" + ;; + *) # For Java 25+ + install_petclinic "spring-projects/spring-petclinic" "main" + install_petclinic "spring-petclinic/spring-framework-petclinic" + ;; +esac patch -N -p1 -d build/fixtures/spring-petclinic < test/petclinic/pom.patch diff --git a/agent/src/main/java/com/appland/appmap/process/hooks/test/TestSupport.java b/agent/src/main/java/com/appland/appmap/process/hooks/test/TestSupport.java index 5fe1ab39..bf83c156 100644 --- a/agent/src/main/java/com/appland/appmap/process/hooks/test/TestSupport.java +++ b/agent/src/main/java/com/appland/appmap/process/hooks/test/TestSupport.java @@ -63,19 +63,61 @@ private static void startRecording(TestDetails details, Recorder.Metadata metada RecordingSupport.startRecording(details, metadata); } + /** + * Finds the most relevant stack frame for a test failure. + * + * The primary goal is to find the stack frame that corresponds to the test + * class itself. However, in some scenarios (e.g., when an assertion fails on + * a different thread), the test class may not be in the stack trace at all. + * + * To handle this, we use a fallback heuristic: we find the stack frame that + * has the longest common package prefix with the test class. This is usually + * the entry point into the user's code and the most likely source of the + * failure. + * + * @param self The test class instance + * @param exception The exception that caused the failure + * @return The most relevant stack frame + * @throws InternalError if no suitable stack frame can be found + */ static StackTraceElement findErrorFrame(Object self, Throwable exception) throws InternalError { String selfClass = self.getClass().getName(); - StackTraceElement errorFrame = null; + StackTraceElement bestMatch = null; + int bestMatchLength = 0; + for (StackTraceElement frame : exception.getStackTrace()) { - if (frame.getClassName().equals(selfClass)) { - errorFrame = frame; - break; + final String frameClassName = frame.getClassName(); + if (frameClassName.equals(selfClass)) { + // This is the ideal case: we found the test class in the stack trace. + return frame; } + + int commonPrefix = commonPrefixLength(selfClass, frameClassName); + if (commonPrefix >= bestMatchLength) { + // We use >= to get the last best match, which is the most likely to be + // the entry point into the user's code. + bestMatch = frame; + bestMatchLength = commonPrefix; + } + } + + if (bestMatch != null) { + // We didn't find the test class, but we have a good fallback. + return bestMatch; } - if (errorFrame == null) { - throw new InternalError("no stack frame matched test class"); + + // This can happen if the exception has an empty stack trace, which is rare + // but possible. + throw new InternalError("no stack frame matched test class"); + } + + private static int commonPrefixLength(String s1, String s2) { + int len = Math.min(s1.length(), s2.length()); + int i = 0; + while (i < len && s1.charAt(i) == s2.charAt(i)) { + i++; } - return errorFrame; + return i; } private static boolean hasTestAnnotation(ClassLoader cl, Method stackMethod) { diff --git a/agent/test/petclinic/appmap-labels.yml b/agent/test/petclinic/appmap-labels.yml index 7dae1036..6b1a2911 100644 --- a/agent/test/petclinic/appmap-labels.yml +++ b/agent/test/petclinic/appmap-labels.yml @@ -8,3 +8,8 @@ packages: name: (info|debug) labels: [log] +- path: org.apache.commons.logging.impl + methods: + - class: Slf4jLogFactory\$Slf4jLocationAwareLog + name: (info|debug) + labels: [log] diff --git a/agent/test/test-frameworks/frameworks.bats b/agent/test/test-frameworks/frameworks.bats index a217ab4c..20d6d674 100644 --- a/agent/test/test-frameworks/frameworks.bats +++ b/agent/test/test-frameworks/frameworks.bats @@ -52,7 +52,7 @@ run_framework_test() { assert_json_eq '.metadata.test_status' "failed" assert_json_contains '.metadata.test_failure.message' 'false is not true' - assert_json_eq '.metadata.test_failure.location' "src/test/java/org/springframework/samples/petclinic/JunitTests.java:20" + assert_json_eq '.metadata.test_failure.location' "src/test/java/org/springframework/samples/petclinic/JunitTests.java:23" } @test "test status set for failed test in testng" { @@ -104,4 +104,12 @@ run_framework_test() { output="$(< tmp/appmap/testng/org_springframework_samples_petclinic_TestngTests_testItThrows.appmap.json)" assert_json_eq '.metadata.test_status' "succeeded" +} + +@test "No InternalError on different thread exception" { + run_framework_test "junit" "JunitTests.offThreadExceptionTest" + assert_failure + # The test should fail with a RuntimeException, but not an InternalError + assert_output --partial "java.lang.RuntimeException" + refute_output --partial "java.lang.InternalError" } \ No newline at end of file diff --git a/agent/test/test-frameworks/src/test/java/org/springframework/samples/petclinic/JunitTests.java b/agent/test/test-frameworks/src/test/java/org/springframework/samples/petclinic/JunitTests.java index 4ffb3c8e..9cc50fc5 100644 --- a/agent/test/test-frameworks/src/test/java/org/springframework/samples/petclinic/JunitTests.java +++ b/agent/test/test-frameworks/src/test/java/org/springframework/samples/petclinic/JunitTests.java @@ -1,7 +1,10 @@ package org.springframework.samples.petclinic; +import static java.util.concurrent.Executors.newSingleThreadExecutor; import static org.junit.Assert.assertTrue; +import java.util.concurrent.Future; +import java.util.concurrent.ExecutorService; import com.appland.appmap.annotation.NoAppMap; import org.junit.Test; @@ -38,4 +41,31 @@ public void testAnnotatedClassNotRecorded() { } } + private static class ExecutorRunner { + public Throwable run() { + ExecutorService executor = newSingleThreadExecutor(); + Future future = executor.submit(() -> { + throw new RuntimeException("Off-thread exception for testing"); + }); + try { + future.get(); + } catch (java.util.concurrent.ExecutionException e) { + return e.getCause(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } finally { + executor.shutdown(); + } + return null; + } + } + + @Test + public void offThreadExceptionTest() throws Throwable { + Throwable throwable = new ExecutorRunner().run(); + if (throwable == null) { + throw new AssertionError("Expected exception from off-thread execution"); + } + throw throwable; + } }