From 4dca856d02dccd0b332a75a672abbdb68ce99d58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Wed, 29 Oct 2025 17:58:14 +0100 Subject: [PATCH 1/2] build: Download petclinic archive instead of full clone To speed up CI, download a tarball of a specific commit hash instead of performing a full git clone. This is particularly useful for pinning spring-petclinic to a version compatible with Java 17. After extracting the tarball, a new git repository is initialized to simulate a checkout (which the tests expect). Refactor the script to handle different Java versions and use the appropriate petclinic version for each. This commit also updates the appmap-labels.yml file to correctly identify the logging classes used in newer versions of the project. --- agent/bin/test_projects | 51 +++++++++++++++++++------- agent/test/petclinic/appmap-labels.yml | 5 +++ 2 files changed, 42 insertions(+), 14 deletions(-) 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/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] From d0d19c8b058c985d51e770a68af1d60bce48d960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Wed, 29 Oct 2025 15:52:10 +0100 Subject: [PATCH 2/2] fix: Prevent crash when test fails on a different thread When a test fails due to an exception thrown on a different thread, the test class itself may not appear in the exceptions stack trace. The agent was previously unable to handle this scenario, crashing with a `java.lang.InternalError` because it could not find a stack frame matching the test class. This commit introduces a more robust heuristic to find the most likely source of the failure. If an exact match for the test class is not found, the agent now falls back to the stack frame with the longest common package prefix. This prevents the crash and correctly identifies the failure location in a framework-agnostic way. A regression test has been added to simulate this cross-thread exception scenario and verify the fix. --- .../process/hooks/test/TestSupport.java | 56 ++++++++++++++++--- agent/test/test-frameworks/frameworks.bats | 10 +++- .../samples/petclinic/JunitTests.java | 30 ++++++++++ 3 files changed, 88 insertions(+), 8 deletions(-) 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/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; + } }