fix(flaky): Workgroups theory tests hitting the 5s default timeout - #1785
peterdrier wants to merge 3 commits into
Conversation
MeetingUrl_AcceptsTheStorageLimitAndRejectsOverflow and Invalid_reasons_render_queue_with_original_text_and_error each build a full WorkgroupService over a cold in-memory EF context, paying the model-build cost inside HumansTheoryAttribute's 5s default budget. Same fix already applied to WorkgroupServiceRegistrationTests' theory in this project: bump to Timeout = 10000. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dm1CnRwEoo8YE2RNXvgZN3
|
The preview deployment for humans-qa is ready. 🟢 Open Preview | Open Build Logs | Open Application Logs Last updated at: 2026-09-21 16:30:58 CET |
|
Reviewed commit 58b4362 — no issues found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58b43620e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Builds a full WorkgroupService over the in-memory context, so on a cold runner it pays | ||
| // the EF model build inside a theory's default 5 s budget — same allowance as | ||
| // WorkgroupServiceRegistrationTests' theory in this project. | ||
| [HumansTheory(Timeout = 10000)] |
There was a problem hiding this comment.
On a cold, contended runner, the model-build cost falls on whichever WorkgroupsDbContext-backed theory executes first, not specifically these two methods; unchanged theories such as LogBody_RejectsOverflowWithoutChangingStoredEntries and the lifecycle tests still use the same harness with the 5-second limit. This can therefore move the intermittent CI failure rather than eliminate it. The existing src/Sections/Humans.Workgroups/Docs/debt.yml WG-1 entry confirms this diagnosis and prescribes an assembly-level model warmup, which fixes the startup cost at its source.
AGENTS.md reference: AGENTS.md:L41-L44
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed — replaced the per-test timeout bumps with the WorkgroupsModelWarmupFixture assembly fixture WG-1 prescribed, and closed WG-1. Thanks for catching that the timeout bump would've just relocated the failure.
Generated by Claude Code
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR Surface ReportCompared Summary: 3 changed file(s) | EF migrations: 0 added file(s), max 0/1 per context Reforge Surface Score
Section DeltasNo section score changes. Section Size & Complexity DeltasNo section size/complexity changes. Rule DeltasNo rule score changes. Published Write Surface14 of 48 sections publish write capability, 23 interfaces (0). Interface SurfaceNo new interfaces or interface methods. Diff Size
New Files
|
Codex correctly flagged that bumping two tests' [HumansTheory(Timeout)] just moves which test pays the cold WorkgroupsDbContext model-build cost next time — src/Sections/Humans.Workgroups/Docs/debt.yml already diagnosed this (WG-1) and prescribed the real fix: an assembly fixture that builds the model once before any test runs, so no [HumansFact]/[HumansTheory]'s 5s budget absorbs it. Reverts the per-test timeout bump, adds WorkgroupsModelWarmupFixture, and closes WG-1. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dm1CnRwEoo8YE2RNXvgZN3
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f9a58ddce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # WG-1 closed 2026-09-21 (peterdrier/Humans#1785): added a WorkgroupsModelWarmupFixture | ||
| # assembly fixture (tests/Humans.Workgroups.Tests/Infrastructure/) that builds the EF model | ||
| # once before any test runs, so it no longer falls inside whichever test's timeout budget. |
There was a problem hiding this comment.
The closure rule in memory/process/debt-ledger-additions.md requires deleting a closed row and citing its ID in the PR, explicitly leaving nothing marked done in place. These lines instead retain WG-1 as permanent completion history, turning the actionable debt ledger into a review transcript; remove them and keep the closure citation in the PR.
AGENTS.md reference: AGENTS.md:L46-L50
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
memory/process/debt-ledger-additions.md: closing an item deletes its row and cites the id in the PR — nothing marked done in place. The WG-1 closure note left the ledger as a review transcript; the PR already carries the citation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dm1CnRwEoo8YE2RNXvgZN3
2 participants
Summary
Weekly flaky-test scan of the last 7 days of CI (97 "Build and Test" + 97 "E2E Tests (QA)" runs on
main) found the #1 flakiest failure: two[HumansTheory]tests inHumans.Workgroups.Testsintermittently hitting xUnit's fixed 5000ms default timeout, with no other logic failures.Root cause (confirmed by Codex's review +
src/Sections/Humans.Workgroups/Docs/debt.ymlWG-1, filed a day before this scan):Humans.Workgroups.Testshas no per-assembly warmup, so whichever test happens to run first pays EF's one-timeWorkgroupsDbContextmodel-build cost inside its own 5s[HumansFact]/[HumansTheory]budget. On a cold, contended CI runner (49 test projects in the same job) that occasionally crosses 5s, and it isn't tied to any specific test — it's just whoever draws the short straw. My first attempt (bumping the two observed tests' timeouts) would only have moved the failure to a different test next time, as Codex pointed out; per-test overrides elsewhere in this project were for tests that are legitimately slower, not for this shared startup cost.Fix:
WorkgroupsModelWarmupFixture(assembly fixture,tests/Humans.Workgroups.Tests/Infrastructure/) builds the EF model once, before any test runs, using the same[assembly: AssemblyFixture(typeof(...))]mechanismHumans.Integration.Testsalready uses for its one-time Postgres setup. This is the fix WG-1 prescribed; the ledger entry is now closed.CI evidence
xUnit.netTest execution timed out after 5000 milliseconds, empty stack trace — no assertion failures, no build errors.MeetingUrl_AcceptsTheStorageLimitAndRejectsOverflow(editing: True): 4 failures.Invalid_reasons_render_queue_with_original_text_and_error(action: "Close"): 3 failures (one run hit both).Verification
No
dotnetSDK is available in this remote execution environment, so I could not rundotnet test tests/Humans.Workgroups.Testslocally across repeated runs. Relying on this PR's own CI as verification.🤖 Generated with Claude Code
https://claude.ai/code/session_01Dm1CnRwEoo8YE2RNXvgZN3