Skip to content

Unit test cleanup for WebSocket connect and mock multi-match expectations#982

Open
jhugard wants to merge 5 commits into
mainfrom
user/jhugard/unit-test-cleanup
Open

Unit test cleanup for WebSocket connect and mock multi-match expectations#982
jhugard wants to merge 5 commits into
mainfrom
user/jhugard/unit-test-cleanup

Conversation

@jhugard
Copy link
Copy Markdown
Collaborator

@jhugard jhugard commented May 20, 2026

Summary

This branch cleans up two unit-test regressions on top of main and restores the Windows Debug x64 TAEF suite to green.

The first fix is a direct follow-on to PR #961, which updated the WebSocket connect completion path to always consume the async connect result via HCGetWebSocketConnectResult(). However, the WebSocket unit-test connect stubs were still using an incomplete fake-provider contract.

The second fix is a direct follow-on to PR #979 and cleans up a stale mock multi-match expectation. That PR intentionally changed mock multi-match selection from stack-style behavior to round-robin behavior, but MockTests::ExampleMultiSpecificUrlBodyMock still asserted the old behavior.

Included Fixes

1. WebSocket connect test contract cleanup

File changed:

  • Tests/UnitTests/Tests/WebsocketTests.cpp

What changed:

  • Test_Internal_HCWebSocketConnectAsync
    • now passes the websocket handle as XAsync context
    • now begins with reinterpret_cast<void*>(HCWebSocketConnectAsync)
    • now completes with sizeof(WebSocketCompletionResult)
    • now implements XAsyncOp::GetResult
  • Test_Internal_HCWebSocketConnectAsyncAndClose
    • now begins with reinterpret_cast<void*>(HCWebSocketConnectAsync)
  • TestWebSocketConnectAndCloseProvider::ConnectAsync
    • now begins with reinterpret_cast<void*>(HCWebSocketConnectAsync)

Why:

After PR #961, the fake connect providers in the test harness must satisfy the same XAsync identity and result-payload contract as real providers, otherwise HCGetWebSocketConnectResult() fails and WebsocketTests::TestConnect no longer represents a valid provider implementation.

Commit on this branch:

  • 0b118889c44db3afe56d23263f73c67562dd7704 - Fix websocket connect test async result contract

2. Mock multi-match expectation cleanup

File changed:

  • Tests/UnitTests/Tests/MockTests.cpp

What changed:

  • ExampleMultiSpecificUrlBodyMock now expects the post-PR improve multi mock behavior #979 round-robin mock multi-match behavior instead of the older newest-match-wins behavior.
  • A clarifying comment was added to make the new expectation explicit.

Why:

The first failing commit for the next failing test was:

  • ab6a11bd31a28ccc9c98b6858fe170a665040a1a - improve multi mock behavior (#979)

That commit intentionally changed mock multi-match selection to round-robin and explicitly says so in the commit message body. The test was never updated, so MockTests::ExampleMultiSpecificUrlBodyMock became stale even though the new behavior was intentional.

Commit on this branch:

  • e7b2e7661aeab65e258599e4e345dfa72e1214f6 - Update mock multi-match test for round-robin behavior

Validation

Validated on clean branch user/jhugard/unit-test-cleanup from main:

  • isolated xbox::httpclienttest::WebsocketTests::TestConnect passes
  • isolated xbox::httpclienttest::MockTests::ExampleMultiSpecificUrlBodyMock passes
  • full Windows Debug x64 TAEF suite passes
    • Summary: Total=87, Passed=87, Failed=0, Blocked=0, Not Run=0, Skipped=0

Additional regression confirmation during the earlier bisect work:

  • applying the WebSocket test-contract fix to 0fa5f24 changed isolated TestConnect from hang/failure to pass
  • with that fix applied, the patched 0fa5f24 TAEF DLL completed with:
    • Summary: Total=84, Passed=84, Failed=0, Blocked=0, Not Run=0, Skipped=0

result->errorCode = S_OK;
result->websocket = websocket;
return S_OK;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now that the provider returns a proper WebSocketCompletionResult, it might be worth adding an end-to-end assertion in TestConnect — call HCGetWebSocketConnectResult after the async completes and verify connectResult.errorCode == S_OK and connectResult.websocket == websocket. Would fully exercise the new GetResult path.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Now that the provider returns a proper WebSocketCompletionResult, it might be worth adding an end-to-end assertion in TestConnect — call HCGetWebSocketConnectResult after the async completes and verify connectResult.errorCode == S_OK and connectResult.websocket == websocket. Would fully exercise the new GetResult path.

Done.

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.

2 participants