Skip to content

Stabilize Postgres reconnect test against reconnect timing race - #9946

Merged
glen-84 merged 1 commit into
mainfrom
gai/stabilize-postgres-reconnect-test
Jun 18, 2026
Merged

Stabilize Postgres reconnect test against reconnect timing race#9946
glen-84 merged 1 commit into
mainfrom
gai/stabilize-postgres-reconnect-test

Conversation

@glen-84

@glen-84 glen-84 commented Jun 18, 2026

Copy link
Copy Markdown
Member

Summary

  • Reconnect_Should_ReconnectWhenConnectionIsClosed waited a fixed 1 second (SpinWait.SpinUntil) for the background reconnect to swap in a new connection, then asserted unconditionally. Under CI load the reconnect can exceed that window, producing an intermittent Assert.NotEqual failure.
  • Replaced the fixed-timeout wait with a deterministic await on a TaskCompletionSource signalled by the reconnect's onConnect callback (10s ceiling), mirroring the sibling Reconnect_Should_CallOnDisconnect_When_ConnectionIsClosed test. Connection is assigned before onConnect is awaited, so the new connection is guaranteed to be in place when the assertion runs.

Test plan

  • Built HotChocolate.Subscriptions.Postgres.Tests and ran the affected test 10× locally on net10.0: 10/10 passed.
  • The wait now keys off the actual reconnect event rather than a fixed timeout, removing the timing race on every target framework.

Copilot AI review requested due to automatic review settings June 18, 2026 09:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR stabilizes the ResilientNpgsqlConnection Postgres reconnect test by removing a fixed-duration wait that can flake under CI load, and replacing it with a deterministic await tied to the actual reconnect event.

Changes:

  • Replaced SpinWait.SpinUntil(..., 1s) with a TaskCompletionSource that completes when the second onConnect fires (reconnect), with a 10s ceiling.
  • Ensured the assertion runs only after the reconnect callback indicates the connection swap has occurred, matching the pattern used by the sibling reconnect test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-code-quality

Copy link
Copy Markdown

Code Coverage Overview

Languages: C#

C# / code-coverage/dotnet

The overall coverage in the gai/stabilize-postgr... branch remains at 49%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main 9e5f4b0 gai/stabilize-postgr... 7b9947c +/-
/home/runner/wo...gerMessage.g.cs 19% 15% -4%
/home/runner/wo...gerMessage.g.cs 28% 25% -3%
/home/runner/wo...eiveEndpoint.cs 80% 77% -3%
/home/runner/wo...geDispatcher.cs 77% 75% -2%
/home/runner/wo...xecutionNode.cs 87% 85% -2%
/home/runner/wo...tocolHandler.cs 85% 85% 0%
/home/runner/wo.../ThrowHelper.cs 26% 26% 0%
/home/runner/wo...text.Pooling.cs 99% 100% +1%
/home/runner/wo...cutorManager.cs 94% 95% +1%
/home/runner/wo...qlConnection.cs 89% 92% +3%

Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@glen-84
glen-84 merged commit 5af3916 into main Jun 18, 2026
145 checks passed
@glen-84
glen-84 deleted the gai/stabilize-postgres-reconnect-test branch June 18, 2026 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants