Skip to content

fix(test): stop the OAuth callback tests losing a failure to the zone - #89

Open
winetree94 wants to merge 1 commit into
mainfrom
fix-oauth-callback-test-race
Open

fix(test): stop the OAuth callback tests losing a failure to the zone#89
winetree94 wants to merge 1 commit into
mainfrom
fix-oauth-callback-test-race

Conversation

@winetree94

Copy link
Copy Markdown
Contributor

What

main is red on Dart tests, and PR #85 cannot merge behind it. The failure is openai_oauth_gateway_test.dart: the callback page escapes provider-supplied text, reported as a bare exception rather than a failed expectation:

ChatGPT sign in failed: <script>alert(1)</script>
package:coder_daemon/src/openai_oauth_gateway.dart 218:11  OpenAIOAuthGateway._completeBrowser

That is an unhandled async error, not an assertion. _completeBrowser fails the session while it is still answering the callback request, but all three denial tests attached their expectLater only after awaiting the HTTP round trip. For the length of a real socket read the error on session.completion had no handler, so the zone reported it and the runner failed the test. Linux wins the race consistently; macOS and Windows do not.

The three affected tests now start watching session.completion before triggering the callback. No assertion is weakened or removed — the same matchers run, just with the handler attached in time.

Verification

  • dart test test/openai_oauth_gateway_test.dart ×3 with randomized ordering, all green
  • dart analyze --fatal-infos packages/coder_daemon, dart format clean

The gateway fails a browser session while it is still answering the
callback request, so the three denial tests attached their expectation
after awaiting a real socket round trip. For the length of that read the
error on session.completion had no handler, and the runner reported it as
an unhandled async error rather than the failure the test was asserting.
macOS and Windows lost the race often enough to redden main.

The expectations now start watching before the round trip.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant