Skip to content

test(prodstyle): make the readiness test async so its required Timeout is enforceable - #349

Merged
IanFrelinger merged 1 commit into
masterfrom
fix/prodstyle-timeout-async-convention
Aug 17, 2026
Merged

test(prodstyle): make the readiness test async so its required Timeout is enforceable#349
IanFrelinger merged 1 commit into
masterfrom
fix/prodstyle-timeout-async-convention

Conversation

@IanFrelinger

Copy link
Copy Markdown
Owner

Why

The kernel-coverage gate has been red on master for every commit since the .NET 10 merge, for two different reasons in the same class:

commitfailure
d3d90fabTimeoutConventionTests FAIL — the readiness test lacked Timeout
3a4ae93e (post-#348)the readiness test itself FAIL — Tests marked with Timeout are only supported for async tests

#348 added [Fact(Timeout = TestTimeouts.HostTouching)] to satisfy the convention, but Ready_is_503_while_starting_200_once_started_and_503_again_once_stopping is synchronous, and xunit enforces Timeout by racing the returned Task against a delay — it cannot do that for a void test and fails it outright. One red was traded for another.

What

  • The readiness test returns Task. Its body is still synchronous (it pins the lifetime transitions on NexoEndpoints.EvaluateReadiness directly, as the comment explains); the signature is what xunit needs to honour the timeout.
  • TimeoutConventionTests now requires both halves — an explicit Timeoutand a Task return — so an unenforceable timeout is named at convention time rather than turning a passing test into a failing one. Without this the two rules contradict each other, and the next void ProdStyle test repeats the outage.
  • Collapses the duplicate Fact/Theory branches: TheoryAttribute derives from FactAttribute, so the first lookup already matched theories and a Theory without a timeout was reported twice.

Verification

Locally on net10.0, TimeoutConventionTests + HealthAndReadinessProdStyleTests: 4 passed, 0 failed.

Negative check — reverting the test to void makes the convention fail with the message that names the fix:

HealthAndReadinessProdStyleTests.Ready_is_503_...: ProdStyle test has [Fact(Timeout = N)]
but returns Void; make it async Task (xunit only honours Timeout on Task-returning tests
and fails the test at run time otherwise)

The other two reds on master (Distribution Matrix Gate, Docs Link Check) are unrelated infra — Failed to download action … 429/503 from codeload.

🤖 Generated with Claude Code

…t is enforceable
#348 added [Fact(Timeout = TestTimeouts.HostTouching)] to satisfy
TimeoutConventionTests, but the test is synchronous and xunit only honours
Timeout on Task-returning tests, so the run failed with "Tests marked with
Timeout are only supported for async tests" -- kernel-coverage stayed red,
one failure traded for another.
The convention now requires both halves: a ProdStyle/E2E test must carry an
explicit Timeout AND return Task, so an unenforceable timeout is named at
convention time instead of at run time. Verified by reverting the test to
void: the convention fails with "has [Fact(Timeout = N)] but returns Void;
make it async Task".
Also collapses the duplicate Fact/Theory branches -- TheoryAttribute derives
from FactAttribute, so the first lookup already covered both and a Theory
without a timeout was reported twice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cursor

cursorBot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@IanFrelinger
IanFrelinger merged commit 6195da0 into masterAug 17, 2026
14 checks passed
@IanFrelinger
IanFrelinger deleted the fix/prodstyle-timeout-async-convention branch August 17, 2026 17:56
Sign up for freeto 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

@IanFrelinger@PlzTouchGrass