Skip to content

Fix E2E tests after runtime update #919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SteveSandersonMS merged 2 commits into from
Mar 24, 2026
+19 −1
Merged

Fix E2E tests after runtime update #919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e6e165
Fix session start test
SteveSandersonMS Mar 24, 2026
afe77e5
Update E2E test proxy to account for newer CLI behaviors
SteveSandersonMS Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion nodejs/test/e2e/session.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ describe("Sessions", async () => {
});
expect(session.sessionId).toMatch(/^[a-f0-9-]+$/);

expect(await session.getMessages()).toMatchObject([
const allEvents = await session.getMessages();
const sessionStartEvents = allEvents.filter((e) => e.type === "session.start");
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Good fix! This change makes the test robust to event ordering. The same pattern should be applied to Python, Go, and .NET tests - they currently assume messages[0] is always session.start, which will fail with the runtime update.

expect(sessionStartEvents).toMatchObject([
{
type: "session.start",
data: { sessionId: session.sessionId, selectedModel: "fake-test-model" },
Expand Down
16 changes: 16 additions & 0 deletions test/harness/replayingCapiProxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,22 @@ export class ReplayingCapiProxy extends CapturingHttpProxy {
}
}

// Beyond this point, we're only going to be able to supply responses in CI if we have a snapshot,
// and we only store snapshots for chat completion. For anything else (e.g., custom-agents fetches),
// return 404 so the CLI treats them as unavailable instead of erroring.
if (options.requestOptions.path !== chatCompletionEndpoint) {
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change improves test harness behavior by returning 404 for non-chat-completion requests (e.g., custom-agents fetches) instead of erroring. This is test infrastructure and doesn't impact SDK API consistency. ✅

const headers = {
"content-type": "application/json",
"x-github-request-id": "proxy-not-found",
};
options.onResponseStart(404, headers);
options.onData(
Buffer.from(JSON.stringify({ error: "Not found by test proxy" })),
);
options.onResponseEnd();
return;
}

// Fallback to normal proxying if no cached response found
// This implicitly captures the new exchange too
const isCI = process.env.GITHUB_ACTIONS === "true";
Expand Down
Loading