fix: Prevent crash when test fails on a different thread

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
 

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) {

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]

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"
 }

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;
+  }
 }