Skip to content

Commit

Permalink
fix: go back to hooking org.junit.jupiter.api.Test
Browse files Browse the repository at this point in the history
JUnit 5 test execution listeners get notified before @beforeeach
methods, and after @AfterEach methods. This causes some problems (e.g.
a temporary recording file can be deleted before it gets moved to its
final destination), but is also not what we want. By design, none of the
other agents capture test setup and teardown, so we shouldn't either.

These changes go back to just hooking JUnit 5's Test class. The change
to have instrumentation catch Throwable instead of exception was a
prerequisite for these changes, because AssertionError (raised when an
assertion fails) is an Error, not an Exception.

Also, change .metadata.framework to .metadata.frameworks so it matches
the spec.
  • Loading branch information
apotterri committed Oct 23, 2023
1 parent 8b2064d commit f15dc6d
Show file tree
Hide file tree
Showing 25 changed files with 238 additions and 338 deletions.
2 changes: 2 additions & 0 deletions agent/bin/test_run
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ set -x
./test/petclinic/test
if [[ $JAVA_VERSION == 17.* ]]; then
./test/petclinic-fw/test
else
./test/junit4/test
fi
./test/httpcore/test
./test/http_client/test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class RecordMethod {
@ExcludeReceiver
@HookCondition(RecordCondition.class)
public static void record(Event event, Object[] args) {
RecordingSupport.startRecording(event, "record_process", "process");
RecordingSupport.startRecording(event, new Recorder.Metadata("record_process", "process"));
}

@ArgumentArray
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,13 @@ public TestDetails(Event event) {
}
}

public static void startRecording(Event event, String recorderName, String recorderType) {
startRecording(new TestDetails(event), recorderName, recorderType);
public static void startRecording(Event event, Recorder.Metadata metadata) {
startRecording(new TestDetails(event), metadata);
}

public static void startRecording(TestDetails details, String recorderName, String recorderType) {
public static void startRecording(TestDetails details, Recorder.Metadata metadata) {
logger.debug("Recording started for {}", canonicalName(details.definedClass, details.isStatic, details.methodId));
try {
Recorder.Metadata metadata = new Recorder.Metadata(recorderName, recorderType);
final String feature = identifierToSentence(details.methodId);
final String featureGroup = identifierToSentence(details.definedClass);
metadata.scenarioName = String.format(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package com.appland.appmap.process.hooks.test;

import com.appland.appmap.output.v1.Event;
import com.appland.appmap.process.hooks.RecordingSupport;
import com.appland.appmap.record.Recorder;
import com.appland.appmap.transform.annotations.ArgumentArray;
import com.appland.appmap.transform.annotations.ExcludeReceiver;
import com.appland.appmap.transform.annotations.HookAnnotated;
import com.appland.appmap.transform.annotations.MethodEvent;

public class JUnit {
private static final Recorder recorder = Recorder.getInstance();
static final String JUNIT_NAME = "junit";

@ArgumentArray
@ExcludeReceiver
@HookAnnotated("org.junit.Test")
public static void junit(Event event, Object[] args) {
Recorder.Metadata metadata = new Recorder.Metadata(JUNIT_NAME, TestSupport.TEST_RECORDER_TYPE);
metadata.frameworks.add(new Recorder.Framework("JUnit", "4"));
RecordingSupport.startRecording(event, metadata);
}

@ArgumentArray
@ExcludeReceiver
@HookAnnotated(value = "org.junit.Test", methodEvent = MethodEvent.METHOD_RETURN)
public static void junit(Event event, Object returnValue, Object[] args) {
RecordingSupport.stopRecording(event, true);
}

@ArgumentArray
@HookAnnotated(value = "org.junit.Test", methodEvent = MethodEvent.METHOD_EXCEPTION)
public static void junit(Event event, Object self, Throwable exception, Object[] args) {
event.setException(exception);
recorder.add(event);
StackTraceElement ste = findErrorFrame(self, exception);
RecordingSupport.stopRecording(new RecordingSupport.TestDetails(event), false, exception.getMessage(),
ste.getLineNumber());
}

@ArgumentArray
@ExcludeReceiver
@HookAnnotated("org.junit.jupiter.api.Test")
public static void junit5Test(Event event, Object[] args) {
Recorder.Metadata metadata = new Recorder.Metadata(JUNIT_NAME, TestSupport.TEST_RECORDER_TYPE);
metadata.frameworks.add(new Recorder.Framework("JUnit", "5"));
RecordingSupport.startRecording(event, metadata);
}

@ArgumentArray
@ExcludeReceiver
@HookAnnotated(value = "org.junit.jupiter.api.Test", methodEvent = MethodEvent.METHOD_RETURN)
public static void junit5Test(Event event, Object returnValue, Object[] args) {
RecordingSupport.stopRecording(event, true);
}

@ArgumentArray
@HookAnnotated(value = "org.junit.jupiter.api.Test", methodEvent = MethodEvent.METHOD_EXCEPTION)
public static void junit5Test(Event event, Object self, Throwable exception, Object[] args) {
event.setException(exception);
recorder.add(event);
StackTraceElement errorFrame = findErrorFrame(self, exception);
RecordingSupport.stopRecording(new RecordingSupport.TestDetails(event), false, exception.getMessage(),
errorFrame.getLineNumber());
}

private static StackTraceElement findErrorFrame(Object self, Throwable exception) throws InternalError {
String selfClass = self.getClass().getName();
StackTraceElement errorFrame = null;
for (StackTraceElement frame : exception.getStackTrace()) {
if (frame.getClassName().equals(selfClass)) {
errorFrame = frame;
break;
}
}
if (errorFrame == null) {
throw new InternalError("no stack frame matched test class");
}
return errorFrame;
}
}

This file was deleted.

208 changes: 0 additions & 208 deletions agent/src/main/java/com/appland/appmap/process/hooks/test/JUnit5.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.appland.appmap.output.v1.Event;
import com.appland.appmap.process.hooks.RecordingSupport;
import com.appland.appmap.record.Recorder;
import com.appland.appmap.transform.annotations.ArgumentArray;
import com.appland.appmap.transform.annotations.ExcludeReceiver;
import com.appland.appmap.transform.annotations.HookAnnotated;
Expand All @@ -14,7 +15,7 @@ public class TestNG {
@ExcludeReceiver
@HookAnnotated("org.testng.annotations.Test")
public static void testng(Event event, Object[] args) {
RecordingSupport.startRecording(event, TESTNG_NAME, TestSupport.TEST_RECORDER_TYPE);
RecordingSupport.startRecording(event, new Recorder.Metadata(TESTNG_NAME, TestSupport.TEST_RECORDER_TYPE));
}

@ArgumentArray
Expand Down
Loading

0 comments on commit f15dc6d

Please sign in to comment.