From 3759ae897a0c9d19bfba90e6c0a3d0d9818a5748 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sun, 8 Nov 2026 02:44:32 -0800 Subject: [PATCH] Let REPL test readiness wait cover a cold container image pull (#21916) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description `PostgresReplCommandTests.ReplExecutesAuthenticatedQuery` failed on main in [run 42152057806](https://github.com/microsoft/aspire/actions/runs/42152057806) ([Hosting.PostgreSQL job](https://github.com/microsoft/aspire/actions/runs/42152057806/job/127678069176)). The shared REPL test base (`tests/Shared/ContainerReplCommandTestBase.cs`) gave the resource 1 minute to become healthy. `StartAsync` returns before DCP pulls the image, so that minute also had to cover a cold pull. The postgres pull alone took about 55s in that run, which left too little time for the container to start. ## Failure stats (14d, attempt 1) `CI` workflow on microsoft/aspire, runs created 2026-10-25 06:28 → 2026-11-08 06:28 UTC, **attempt 1 only**. Denominator: attempt-1 jobs that ran (success or failure). Hits: the failed job's log matches the signature. Jobs that lost their runner, which have no log, are excluded. Main runs have no reruns, so their flake check is the next main run of the job. | | | |---|---| | Signature | `PostgresReplCommandTests.ReplExecutesAuthenticatedQuery` failed with `Resource 'postgres' failed to become healthy before the operation was cancelled` | | Job | Hosting.PostgreSQL (ubuntu + windows) | | main pushes: hits / runs | 1 / 316 (0.32%) | | main pushes, 30d: hits / runs | 1 / 546 (0.18%). The 16 days before the 14d window add 0 hits. | | PR runs: hits / runs | 0 / 921 (0.00%); 1 PR-caused excluded | | Combined rate | 1 / 1237 = **0.08%** | | Flake check | main: next main run of the job passed 1/1 | | First / last hit | 2026-11-06 / 2026-11-06 | | Still hitting main? | Last main hit: 2026-11-06. 44 main runs since the last hit (2026-11-06), with 0 hits | | Fix evidence | Local: FakeTimeProvider budget test 5/5 fail → 5/5 pass. Real Docker REPL suites pass (Postgres 24/24). This PR's CI: Hosting.PostgreSQL ubuntu + windows passed 1/1. | Failing attempt-1 jobs, most recent first (1 of 1 log-verified hits): | Date (UTC) | Branch | Job | Signature excerpt | |---|---|---|---| | [2026-11-06 02:42](https://github.com/microsoft/aspire/actions/runs/42152057806/job/127678069176) | **main** | Hosting.PostgreSQL (ubuntu-latest) | `Resource 'postgres' failed to become healthy before the operation was cancelled` | **Verdict: WEAK.** Only **1 non-PR-caused hit** (main, 2026-11-06). The 1 PR hit came from the in-flight WSLC-on-Windows PR and is excluded. The other 7 failures of this test had different errors (`Terminal … has already`, `CliPath` options validation, Polly timeout, `Stopped waiting`). Across all shared-base REPL tests (Postgres/Redis/Valkey/MongoDB/SqlServer), the same healthy-timeout hit 2/1580 main jobs, and 11 more PR hits were all on the WSLC PR. A timeout increase is low risk, but the data does not show a recurring flake. The fix moves the readiness wait into a `WaitForReplResourceHealthyAsync` helper that uses `TestConstants.ExtraLongTimeoutTimeSpan` (180s locally, 360s on CI). That constant already exists and is documented as sized for image pulls. This raises a timeout to match a measured cold pull. It does not add retries or sleeps, and the assertions are unchanged. The helper takes a `TimeProvider` so the budget can be tested. The Redis, MongoDB and SqlServer test projects now link `AsyncTestHelpers.cs`, because they share the base class. Other recent failures of this test came from in-flight PRs (for example the WSLC Windows work in #21564) and are not addressed here. Refs #21600 ## Confidence - New regression test `ReplReadinessWaitCoversColdImagePull` (FakeTimeProvider, no Docker) moves the clock forward 2 minutes, then reports the resource as failed to start. It expects the wait to fail for that reason rather than be cancelled. - Before the fix (1-minute budget): 5/5 runs failed with `OperationCanceledException`. - After the fix: 5/5 passed. - With real Docker: `PostgresReplCommandTests` 24/24, `RedisReplCommandTests` 20/20 and `ValkeyReplCommandTests` 20/20 passed. - All five affected test projects build. - **Not run locally:** the MongoDB and SqlServer REPL container tests. They failed on my machine with `No space left on device` during image extraction. CI covers them. **How we stop this recurring:** container readiness waits in shared test bases should use `TestConstants.ExtraLongTimeoutTimeSpan`, which accounts for image pulls, instead of hand-picked minute values. ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - [ ] No. Follow-up changes expected. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - [ ] No - Did you add public API? - [ ] Yes - [x] No - Does the change make any security assumptions or guarantees? - [ ] Yes - [x] No **🤖 Posted by AspireAgent (AI Agent)** Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 9fcb3794aa23a6846bbadd13a078d3b686324380) --- .../Aspire.Hosting.MongoDB.Tests.csproj | 1 + .../Aspire.Hosting.PostgreSQL.Tests.csproj | 1 + .../PostgresReplCommandTests.cs | 24 +++++++++++++++++++ .../Aspire.Hosting.Redis.Tests.csproj | 1 + .../Aspire.Hosting.SqlServer.Tests.csproj | 1 + tests/Shared/ContainerReplCommandTestBase.cs | 15 ++++++++---- 6 files changed, 39 insertions(+), 4 deletions(-) diff --git a/tests/Aspire.Hosting.MongoDB.Tests/Aspire.Hosting.MongoDB.Tests.csproj b/tests/Aspire.Hosting.MongoDB.Tests/Aspire.Hosting.MongoDB.Tests.csproj index faaeea98eb9..d6469b9e1c9 100644 --- a/tests/Aspire.Hosting.MongoDB.Tests/Aspire.Hosting.MongoDB.Tests.csproj +++ b/tests/Aspire.Hosting.MongoDB.Tests/Aspire.Hosting.MongoDB.Tests.csproj @@ -5,6 +5,7 @@ + diff --git a/tests/Aspire.Hosting.PostgreSQL.Tests/Aspire.Hosting.PostgreSQL.Tests.csproj b/tests/Aspire.Hosting.PostgreSQL.Tests/Aspire.Hosting.PostgreSQL.Tests.csproj index 7043e4c8011..1e67cf1ac68 100644 --- a/tests/Aspire.Hosting.PostgreSQL.Tests/Aspire.Hosting.PostgreSQL.Tests.csproj +++ b/tests/Aspire.Hosting.PostgreSQL.Tests/Aspire.Hosting.PostgreSQL.Tests.csproj @@ -16,6 +16,7 @@ + diff --git a/tests/Aspire.Hosting.PostgreSQL.Tests/PostgresReplCommandTests.cs b/tests/Aspire.Hosting.PostgreSQL.Tests/PostgresReplCommandTests.cs index fc03b618cbe..377f0fe5127 100644 --- a/tests/Aspire.Hosting.PostgreSQL.Tests/PostgresReplCommandTests.cs +++ b/tests/Aspire.Hosting.PostgreSQL.Tests/PostgresReplCommandTests.cs @@ -6,6 +6,8 @@ using Aspire.Hosting.Tests; using Aspire.Hosting.Utils; using Aspire.TestUtilities; +using Microsoft.AspNetCore.InternalTesting; +using Microsoft.Extensions.Time.Testing; #pragma warning disable ASPIRETERMINAL001 @@ -72,6 +74,28 @@ public async Task ReplRejectsMissingTargetPort() Assert.Equal("The PostgreSQL REPL port is not available.", exception.Message); } + [Fact] + public async Task ReplReadinessWaitCoversColdImagePull() + { + // A cold postgres image pull took ~55s in CI before the container could even start, so the + // readiness wait must still be alive well past that point. + var coldImagePullAndStartup = TimeSpan.FromMinutes(2); + + using var builder = TestDistributedApplicationBuilder.Create(); + var postgres = builder.AddPostgres("postgres"); + await using var app = builder.Build(); + var timeProvider = new FakeTimeProvider(); + + var waitTask = WaitForReplResourceHealthyAsync(app, postgres.Resource.Name, timeProvider); + timeProvider.Advance(coldImagePullAndStartup); + Assert.False(waitTask.IsCompleted); + + // The wait only observes the resource once it reaches a terminal state, so the wait failing for + // that reason (rather than being cancelled) shows the budget had not run out. + await app.ResourceNotifications.PublishUpdateAsync(postgres.Resource, s => s with { State = KnownResourceStates.FailedToStart }); + await Assert.ThrowsAsync(() => waitTask).DefaultTimeout(); + } + [Theory] [InlineData(null)] [InlineData(5433)] diff --git a/tests/Aspire.Hosting.Redis.Tests/Aspire.Hosting.Redis.Tests.csproj b/tests/Aspire.Hosting.Redis.Tests/Aspire.Hosting.Redis.Tests.csproj index c7308f132cc..c0ff10d0f6e 100644 --- a/tests/Aspire.Hosting.Redis.Tests/Aspire.Hosting.Redis.Tests.csproj +++ b/tests/Aspire.Hosting.Redis.Tests/Aspire.Hosting.Redis.Tests.csproj @@ -2,6 +2,7 @@ + diff --git a/tests/Aspire.Hosting.SqlServer.Tests/Aspire.Hosting.SqlServer.Tests.csproj b/tests/Aspire.Hosting.SqlServer.Tests/Aspire.Hosting.SqlServer.Tests.csproj index 2a187782b4c..4a4e7c2e6b1 100644 --- a/tests/Aspire.Hosting.SqlServer.Tests/Aspire.Hosting.SqlServer.Tests.csproj +++ b/tests/Aspire.Hosting.SqlServer.Tests/Aspire.Hosting.SqlServer.Tests.csproj @@ -7,6 +7,7 @@ + diff --git a/tests/Shared/ContainerReplCommandTestBase.cs b/tests/Shared/ContainerReplCommandTestBase.cs index 001755a1b35..73942f65f72 100644 --- a/tests/Shared/ContainerReplCommandTestBase.cs +++ b/tests/Shared/ContainerReplCommandTestBase.cs @@ -5,6 +5,7 @@ using Aspire.Hosting.ApplicationModel; using Aspire.Hosting.Tests.Utils; using Aspire.Hosting.Utils; +using Microsoft.AspNetCore.InternalTesting; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; @@ -160,10 +161,7 @@ protected static async Task VerifyReplAsync( await app.StartAsync(startup.Token); } - using (var readiness = new CancellationTokenSource(TimeSpan.FromMinutes(1))) - { - await app.ResourceNotifications.WaitForResourceHealthyAsync(resource.Name, readiness.Token); - } + await WaitForReplResourceHealthyAsync(app, resource.Name, TimeProvider.System); using var cancellation = new CancellationTokenSource(TimeSpan.FromMinutes(1)); await using var terminal = await TerminalCommandTestHelpers.ExecuteTerminalCommandAsync(app, resource, "repl", cancellation.Token); @@ -187,4 +185,13 @@ protected static async Task VerifyReplAsync( throw new TimeoutException($"REPL did not produce the expected output. Terminal screen:{Environment.NewLine}{terminal.GetScreenText()}", exception); } } + + protected static async Task WaitForReplResourceHealthyAsync(DistributedApplication app, string resourceName, TimeProvider timeProvider) + { + // StartAsync returns before DCP pulls the container image, so this wait also has to cover a + // cold image pull. A cold postgres pull alone took ~55s in CI, which exhausted the previous + // 1-minute budget before the container could become healthy. + using var readiness = new CancellationTokenSource(TestConstants.ExtraLongTimeoutTimeSpan, timeProvider); + await app.ResourceNotifications.WaitForResourceHealthyAsync(resourceName, readiness.Token); + } }