Skip to content

Commit 60eb094

Browse files
edburnsCopilotCopilot
committed
Fix Java tool-processor test generation and stabilize session-id test (#1799)
* Fix Java tool-processor test generation and stabilize session-id test Address the Java test failures observed in the Java 17 surefire/failsafe run by fixing how annotation-processing output is discovered in CopilotToolProcessor tests and by hardening one timing-sensitive session test. Changes included: - CopilotToolProcessor: resolve @copilotTool elements via TypeElement lookup and reuse that element list through validation and generation passes, making annotation discovery robust across compiler/module contexts. - CopilotToolProcessorTest: force annotation processing in the in-memory compile harness (-proc:full, explicit processor), close the file manager with try-with-resources, and add a collecting forwarding file manager that captures generated source content from getJavaFileForOutput to avoid missing generated CopilotToolMeta classes in tests. - CopilotSessionTest#testShouldGetLastSessionId: add bounded retry for session creation (including timeout and execution-timeout-cause handling) to absorb transient startup delays while preserving failure behavior on persistent errors. Result: - CopilotToolProcessorTest now consistently observes generated CopilotToolMeta output and passes. - The full requested Maven workflow (jacoco prepare/report + surefire + failsafe under Java 17, with prior Java 25 compile) completes successfully. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Spotless * Avoid leaking session. The retry on session creation uses `future.get(timeout)` but does not cancel the in-flight `createSession` future when a timeout occurs. If attempt 1 eventually completes after attempt 2 starts, it can leave an orphaned session registered in the client (and potentially race `getLastSessionId` persistence), reintroducing flakiness and leaking resources. Capture the future and cancel it on timeout before retrying. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Add abort-session snapshot variant for interrupted tool calls Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent c1c5b34 commit 60eb094

4 files changed

Lines changed: 128 additions & 12 deletions

File tree

‎java/src/main/java/com/github/copilot/tool/CopilotToolProcessor.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ public class CopilotToolProcessor extends AbstractProcessor {
4949

5050
@Override
5151
public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
52-
for (Element element : roundEnv.getElementsAnnotatedWith(CopilotTool.class)) {
52+
List<Element> annotatedElements = getCopilotToolAnnotatedElements(roundEnv);
53+
for (Element element : annotatedElements) {
5354
if (element.getKind() != ElementKind.METHOD) {
5455
continue;
5556
}
@@ -75,7 +76,7 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
7576

7677
// Group methods by enclosing type
7778
Map<TypeElement, List<ExecutableElement>> methodsByClass = new LinkedHashMap<>();
78-
for (Element element : roundEnv.getElementsAnnotatedWith(CopilotTool.class)) {
79+
for (Element element : annotatedElements) {
7980
if (element.getKind() != ElementKind.METHOD) {
8081
continue;
8182
}
@@ -95,6 +96,15 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
9596
return false;
9697
}
9798

99+
private List<Element> getCopilotToolAnnotatedElements(RoundEnvironment roundEnv) {
100+
TypeElement copilotToolType = processingEnv.getElementUtils()
101+
.getTypeElement("com.github.copilot.tool.CopilotTool");
102+
if (copilotToolType != null) {
103+
return new ArrayList<>(roundEnv.getElementsAnnotatedWith(copilotToolType));
104+
}
105+
return new ArrayList<>(roundEnv.getElementsAnnotatedWith(CopilotTool.class));
106+
}
107+
98108
private void generateMetaClass(TypeElement classElement, List<ExecutableElement> methods) {
99109
String packageName = processingEnv.getElementUtils().getPackageOf(classElement).getQualifiedName().toString();
100110
String simpleClassName = classElement.getSimpleName().toString();

‎java/src/test/java/com/github/copilot/CopilotSessionTest.java‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -756,8 +756,27 @@ void testShouldGetLastSessionId() throws Exception {
756756
ctx.configureForTest("session", "should_get_last_session_id");
757757

758758
try (CopilotClient client = ctx.createClient()) {
759-
CopilotSession session = client
760-
.createSession(new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get();
759+
CopilotSession session = null;
760+
for (int attempt = 1; attempt <= 2; attempt++) {
761+
CompletableFuture<CopilotSession> createFuture = client
762+
.createSession(new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL));
763+
try {
764+
session = createFuture.get(45, TimeUnit.SECONDS);
765+
break;
766+
} catch (java.util.concurrent.TimeoutException e) {
767+
createFuture.cancel(true);
768+
if (attempt == 2) {
769+
throw e;
770+
}
771+
} catch (java.util.concurrent.ExecutionException e) {
772+
if (e.getCause() instanceof java.util.concurrent.TimeoutException && attempt < 2) {
773+
createFuture.cancel(true);
774+
continue;
775+
}
776+
throw e;
777+
}
778+
}
779+
assertNotNull(session, "Session should be created");
761780

762781
session.sendAndWait(new MessageOptions().setPrompt("Say hello")).get(60, TimeUnit.SECONDS);
763782
String sessionId = session.getSessionId();

‎java/src/test/java/com/github/copilot/tool/CopilotToolProcessorTest.java‎

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,24 @@
1010
import static org.junit.jupiter.api.Assertions.assertTrue;
1111

1212
import java.io.File;
13+
import java.io.FilterWriter;
14+
import java.io.IOException;
15+
import java.io.Writer;
1316
import java.net.URI;
1417
import java.nio.file.Path;
1518
import java.security.CodeSource;
1619
import java.util.ArrayList;
20+
import java.util.LinkedHashMap;
1721
import java.util.LinkedHashSet;
1822
import java.util.List;
23+
import java.util.Map;
1924
import java.util.Set;
2025

2126
import javax.tools.Diagnostic;
2227
import javax.tools.DiagnosticCollector;
28+
import javax.tools.FileObject;
29+
import javax.tools.ForwardingJavaFileManager;
30+
import javax.tools.ForwardingJavaFileObject;
2331
import javax.tools.JavaCompiler;
2432
import javax.tools.JavaFileObject;
2533
import javax.tools.SimpleJavaFileObject;
@@ -540,25 +548,28 @@ private CompilationResult compileWithProcessor(List<JavaFileObject> sources) {
540548

541549
String classpath = resolveClasspath();
542550
List<String> options = new ArrayList<>();
551+
options.add("-proc:full");
552+
options.addAll(List.of("-processor", "com.github.copilot.tool.CopilotToolProcessor"));
543553
options.addAll(List.of("-classpath", classpath));
544554
options.addAll(List.of("-d", tempDir.toString()));
545555
options.addAll(List.of("-s", tempDir.toString()));
546556
// Allow experimental APIs during test compilation
547557
options.add("-Acopilot.experimental.allowed=true");
548558

549-
try {
550-
StandardJavaFileManager fileManager = compiler.getStandardFileManager(diagnostics, null, null);
559+
try (StandardJavaFileManager fileManager = compiler.getStandardFileManager(diagnostics, null, null)) {
551560
fileManager.setLocation(StandardLocation.SOURCE_OUTPUT, List.of(tempDir.toFile()));
552561
fileManager.setLocation(StandardLocation.CLASS_OUTPUT, List.of(tempDir.toFile()));
562+
CollectingFileManager collectingFileManager = new CollectingFileManager(fileManager);
553563

554-
JavaCompiler.CompilationTask task = compiler.getTask(null, fileManager, diagnostics, options, null,
555-
sources);
556-
task.setProcessors(List.of(new CopilotToolProcessor()));
564+
JavaCompiler.CompilationTask task = compiler.getTask(null, collectingFileManager, diagnostics, options,
565+
null, sources);
557566
task.call();
558567

559-
// Collect generated sources
560-
List<String> generatedSources = new ArrayList<>();
561-
collectGeneratedFiles(tempDir, generatedSources);
568+
List<String> generatedSources = collectingFileManager.getGeneratedSources();
569+
if (generatedSources.isEmpty()) {
570+
// Fallback for file-manager implementations that only materialize on disk.
571+
collectGeneratedFiles(tempDir, generatedSources);
572+
}
562573

563574
return new CompilationResult(diagnostics.getDiagnostics(), generatedSources, tempDir);
564575
} catch (Exception e) {
@@ -666,4 +677,52 @@ String getGeneratedSource(String qualifiedName) {
666677
return null;
667678
}
668679
}
680+
681+
private static class CollectingFileManager extends ForwardingJavaFileManager<StandardJavaFileManager> {
682+
private final Map<String, StringBuilder> generatedByClass = new LinkedHashMap<>();
683+
684+
CollectingFileManager(StandardJavaFileManager fileManager) {
685+
super(fileManager);
686+
}
687+
688+
@Override
689+
public JavaFileObject getJavaFileForOutput(Location location, String className, JavaFileObject.Kind kind,
690+
FileObject sibling) throws IOException {
691+
JavaFileObject delegate = super.getJavaFileForOutput(location, className, kind, sibling);
692+
if (kind != JavaFileObject.Kind.SOURCE) {
693+
return delegate;
694+
}
695+
StringBuilder captured = new StringBuilder();
696+
generatedByClass.put(className, captured);
697+
return new ForwardingJavaFileObject<>(delegate) {
698+
@Override
699+
public Writer openWriter() throws IOException {
700+
Writer target = delegate.openWriter();
701+
return new FilterWriter(target) {
702+
@Override
703+
public void write(char[] cbuf, int off, int len) throws IOException {
704+
captured.append(cbuf, off, len);
705+
super.write(cbuf, off, len);
706+
}
707+
708+
@Override
709+
public void write(int c) throws IOException {
710+
captured.append((char) c);
711+
super.write(c);
712+
}
713+
714+
@Override
715+
public void write(String str, int off, int len) throws IOException {
716+
captured.append(str, off, off + len);
717+
super.write(str, off, len);
718+
}
719+
};
720+
}
721+
};
722+
}
723+
724+
List<String> getGeneratedSources() {
725+
return generatedByClass.values().stream().map(StringBuilder::toString).toList();
726+
}
727+
}
669728
}

‎test/snapshots/session/should_abort_a_session.yaml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,31 @@ conversations:
5050
content: What is 2+2?
5151
- role: assistant
5252
content: 2 + 2 = 4
53+
- messages:
54+
- role: system
55+
content: ${system}
56+
- role: user
57+
content: run the shell command 'sleep 100' (note this works on both bash and PowerShell)
58+
- role: assistant
59+
content: I'll run the sleep command for 100 seconds.
60+
tool_calls:
61+
- id: toolcall_0
62+
type: function
63+
function:
64+
name: report_intent
65+
arguments: '{"intent":"Running sleep command"}'
66+
- id: toolcall_1
67+
type: function
68+
function:
69+
name: ${shell}
70+
arguments: '{"command":"sleep 100","description":"Run sleep 100 command","mode":"sync","initial_wait":105}'
71+
- role: tool
72+
tool_call_id: toolcall_0
73+
content: The execution of this tool, or a previous tool was interrupted.
74+
- role: tool
75+
tool_call_id: toolcall_1
76+
content: The execution of this tool, or a previous tool was interrupted.
77+
- role: user
78+
content: What is 2+2?
79+
- role: assistant
80+
content: 2 + 2 = 4

0 commit comments

Comments
 (0)