Repository navigation
[release/17.0] Let REPL test readiness wait cover a cold container image pull - #22021
Closed
Mitch Denny (mitchdenny) wants to merge 1 commit into
Closed
Mitch Denny (mitchdenny) wants to merge 1 commit into
Mitch Denny (mitchdenny) wants to merge 1 commit into
Conversation
## 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. <!-- /failure-stats --> 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 9fcb379)
14 tasks
This comment has been minimized.
This comment has been minimized.
Mitch Denny (mitchdenny)
marked this pull request as ready for review
November 10, 2026 04:55
Mitch Denny (mitchdenny)
requested review from
Eric Erhardt (eerhardt) and
Ankit Jain (radical)
as code owners
November 10, 2026 04:55
Member
Author
|
Approved for inclusion in release/17.0. Do not merge yet. |
Mitch Denny (mitchdenny)
force-pushed
the
mitchdenny-backport-17-0-24-pr21916
branch
from
November 10, 2026 05:46
3759ae8 to
c751c72
Compare
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 22021Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 22021" |
This comment has been minimized.
This comment has been minimized.
Mitch Denny (mitchdenny)
force-pushed
the
mitchdenny-backport-17-0-24-pr21916
branch
from
November 10, 2026 06:49
c751c72 to
3759ae8
Compare
Mitch Denny (mitchdenny)
force-pushed
the
mitchdenny-backport-17-0-24-pr21916
branch
from
November 10, 2026 07:30
3759ae8 to
c751c72
Compare
Mitch Denny (mitchdenny)
force-pushed
the
mitchdenny-backport-17-0-24-pr21916
branch
from
November 10, 2026 09:00
c751c72 to
3759ae8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Backport of #21916 to
release/17.0(cherry-picked from 9fcb379 with no modifications). Layer 24 of 55 in the release/17.0 backport stack; stacked on #22020.Backport recommendation: No — Test flake.
Backport of #21916
Checklist
<remarks />and<code />elements on your triple slash comments?