Skip to content

Commit 186c8c9

Browse files
authored
Merge pull request #216 from github/copilot/fix-review-comment-3268776043
fix: bound polling loop timeouts by remaining deadline time
2 parents a4af255 + ee7f834 commit 186c8c9

1 file changed

Lines changed: 45 additions & 7 deletions

File tree

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

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -760,12 +760,30 @@ void testShouldGetLastSessionId() throws Exception {
760760
.createSession(new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get();
761761

762762
session.sendAndWait(new MessageOptions().setPrompt("Say hello")).get(60, TimeUnit.SECONDS);
763+
String sessionId = session.getSessionId();
764+
session.close();
763765

764-
String lastId = client.getLastSessionId().get(30, TimeUnit.SECONDS);
766+
// Poll until getLastSessionId returns the expected value.
767+
// Session state is persisted asynchronously; polling keeps fast
768+
// machines fast and slow CI safe (mirrors Node.js/.NET patterns).
769+
String lastId = null;
770+
long deadline = System.currentTimeMillis() + 10_000;
771+
while (System.currentTimeMillis() < deadline) {
772+
long remaining = Math.max(1, deadline - System.currentTimeMillis());
773+
long iterationTimeout = Math.min(remaining, 500);
774+
try {
775+
lastId = client.getLastSessionId().get(iterationTimeout, TimeUnit.MILLISECONDS);
776+
} catch (java.util.concurrent.TimeoutException ignored) {
777+
// RPC call took longer than the per-iteration cap; retry
778+
continue;
779+
}
780+
if (sessionId.equals(lastId)) {
781+
break;
782+
}
783+
Thread.sleep(50);
784+
}
765785
assertNotNull(lastId, "Last session ID should not be null");
766-
assertEquals(session.getSessionId(), lastId, "Last session ID should match the current session ID");
767-
768-
session.close();
786+
assertEquals(sessionId, lastId, "Last session ID should match the current session ID");
769787
}
770788
}
771789

@@ -840,11 +858,31 @@ void testShouldGetSessionMetadataById() throws Exception {
840858
var session = client
841859
.createSession(new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get();
842860

861+
// Send a message to persist the session to disk
843862
session.sendAndWait(new MessageOptions().setPrompt("Say hello")).get(60, TimeUnit.SECONDS);
844863

845-
var metadata = client.getSessionMetadata(session.getSessionId()).get(30, TimeUnit.SECONDS);
846-
assertNotNull(metadata, "Metadata should not be null for known session");
847-
assertEquals(session.getSessionId(), metadata.getSessionId(), "Metadata session ID should match");
864+
// Poll until metadata becomes available; the CLI persists session
865+
// state asynchronously so it may not be queryable immediately
866+
// (mirrors .NET WaitForConditionAsync pattern).
867+
var sessionId = session.getSessionId();
868+
com.github.copilot.sdk.json.SessionMetadata metadata = null;
869+
long deadline = System.currentTimeMillis() + 10_000;
870+
while (System.currentTimeMillis() < deadline) {
871+
long remaining = Math.max(1, deadline - System.currentTimeMillis());
872+
long iterationTimeout = Math.min(remaining, 500);
873+
try {
874+
metadata = client.getSessionMetadata(sessionId).get(iterationTimeout, TimeUnit.MILLISECONDS);
875+
} catch (java.util.concurrent.TimeoutException ignored) {
876+
// RPC call took longer than the per-iteration cap; retry
877+
continue;
878+
}
879+
if (metadata != null) {
880+
break;
881+
}
882+
Thread.sleep(50);
883+
}
884+
assertNotNull(metadata, "Timed out waiting for getSessionMetadata() to return the persisted session");
885+
assertEquals(sessionId, metadata.getSessionId(), "Metadata session ID should match");
848886

849887
// A non-existent session should return null
850888
var notFound = client.getSessionMetadata("non-existent-session-id").get(30, TimeUnit.SECONDS);

0 commit comments

Comments
 (0)