Skip to content

test(kind): fix flaky CRD timeout coverage on Windows - #2298

Open
afscrome wants to merge 1 commit into
CommunityToolkit:mainfrom
afscrome:afscrome-kind-windows-flakiness
Open

afscrome wants to merge 1 commit into
CommunityToolkit:mainfrom
afscrome:afscrome-kind-windows-flakiness

Conversation

@afscrome

Copy link
Copy Markdown
Contributor

Linked issue: N/A — reported through the failing CI runs below.

Remove ManifestPostApplyCheck_RejectsEstablishedResponseAfterDeadline, which races a one-second cancellation timer against a fake Kubernetes response delayed by 1.25 seconds. On Windows, the response can complete before the delayed cancellation callback runs, causing an intermittent failure.

Extend the existing hung-request timeout theory to cover clients that both honour and ignore cancellation, under strict and best-effort policies. The fake read stays pending throughout the assertion and is released in finally for cleanup. This retains timeout coverage without racing two timers. No production behaviour or public APIs change.

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • New integration
    • Docs are written
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

Other information

Observed Windows failures (the corresponding Linux jobs passed):

Validation on Windows:

  • dotnet run --project tests\CommunityToolkit.Aspire.Hosting.Kind.Tests\CommunityToolkit.Aspire.Hosting.Kind.Tests.csproj --configuration Release -- --filter-class '*KindManifestTests' — passed, 106 tests. Build emitted transient executable-copy retry warnings, then completed successfully.
  • git diff --check — passed.

A full-suite investigation before this change was stopped after a TypeScript AppHost test failed because Helm was missing locally; it did not complete. Unchecked new-integration/API-documentation items are not applicable. Latest-main freshness has not been verified.

Replace the delayed-response deadline test with hung-read coverage for clients that honour or ignore cancellation, under strict and best-effort policies.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 24, 2026 21:43
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://github.com/raw/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.sh | bash -s -- 2298

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.ps1) } 2298"

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.

🟢 Approval recommended

The focused test changes preserve timeout-policy coverage while removing the response-timer race, with no blocking issues identified.

No open issues 🎉

What changed in this PR

This test-only change stabilizes Kind CRD timeout coverage on Windows by replacing a timing-sensitive response test with a controlled pending request. Production behavior and public APIs remain unchanged.

Changes:

  • Removes the flaky test that races a delayed response against cancellation.
  • Covers clients that honor or ignore cancellation under strict and best-effort policies.
  • Releases the pending fake request in finally for cleanup.
File Description
tests/​CommunityToolkit.Aspire.Hosting.Kind.Tests/​KindManifestTests.cs Replaces timing-sensitive coverage with four hung-request timeout cases and explicit cleanup.

Review effort: Balanced

@afscrome
afscrome enabled auto-merge (squash) October 24, 2026 21:48

This branch has not been deployed

No deployments
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