[quality-improver] Acceptance tests assert hardcoded upper bounds on wall-clock stopwatch elapsed time (CI timing flakiness risk)

[github-actions]

🎯 Repository Quality Improvement Report — CI Timing-Assertion Robustness

Analysis Date: 2026-08-31
Focus Area: CI Timing-Assertion Robustness (custom)
Strategy Type: Custom

Executive Summary

Several acceptance tests measure the wall-clock duration of an end-to-end test-host process launch with a Stopwatch and then assert an upper bound on the elapsed seconds (e.g. Assert.IsLessThan(8, stopwatch.Elapsed.TotalSeconds, ...)). Unlike the project's own AcceptanceAssert.DurationPattern guidance — which correctly avoids hardcoding a rendered test-duration string because that format grows leading parts on slow machines — these assertions hardcode a process-launch timing budget that is not tied to any configured timeout and has no adaptive/environment-aware justification. On a loaded CI runner (macOS/Windows agents are called out in the repo's own testing guidelines as slower), simply starting a .NET test host, letting Environment.ProcessorCount * 5 data consumers register, or tearing down a TestHostControllerFinalization step can legitimately exceed a few seconds without any product regression, causing an intermittent, hard-to-reproduce CI failure that has nothing to do with the behavior under test.

This is distinct from the already-documented DurationPattern guidance (which is about not hardcoding the rendered (NNNms) string) — these are separate, additional hardcoded numeric ceilings on real host process wall-clock time, currently present in at least three acceptance test files. None of the four affected assertions currently carry a comment explaining the margin chosen or a fallback/inconclusive path if the timing budget is exceeded on a slow agent.

Full Analysis Report

Focus Area: CI Timing-Assertion Robustness

Current State Assessment

Metrics Collected:

Metric	Value	Status
Acceptance test files asserting Assert.IsLessThan(N, stopwatch.Elapsed.TotalSeconds, ...)	3 files, 4 assertion sites	⚠️
Assertions with an explanatory comment about the chosen margin	0 / 4	❌
Existing repo guidance covering rendered-duration hardcoding (DurationPattern)	Present and followed	✅
Existing repo guidance covering stopwatch upper-bound hardcoding	Absent	❌

Findings

Strengths

• The repo already has strong tooling (AcceptanceAssert.DurationPattern) and explicit contributor guidance against hardcoding rendered per-test duration strings like \(\d+ms\).
• Most timing-sensitive acceptance tests use rendezvous/barrier-based assertions (e.g. TestDependencyExecutionTests) instead of elapsed-time comparisons, which is the more robust pattern already used elsewhere in the same file set.

Areas for Improvement

• ⚠️ test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/TestHostProcessLifetimeHandlerTests.cs — Timeout_BoundsBlockingFinalizationWithoutDisposingRunningHandler (line 69) and Timeout_BoundsBlockingDisposalWithoutRetryingIt (line 95) each assert Assert.IsLessThan(8, stopwatch.Elapsed.TotalSeconds, ...) around a full process launch + a 500ms-configured timeout + a 0.5s finalization budget, leaving under an 6.5x margin that is easily consumed by CI process-start overhead alone.
• ⚠️ test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DataConsumerThroughputTests.cs — MultipleDataConsumers_ShouldCompleteInReasonableTime (line 24) asserts Assert.IsLessThan(7, stopwatch.Elapsed.TotalSeconds, ...) around registering Environment.ProcessorCount * 5 data consumers and running a full host process; the margin scales with core count in an untested way and has no comment justifying "7".
• ⚠️ test/IntegrationTests/MSTest.Acceptance.IntegrationTests/TimeoutWhenExpiresTests.cs — RunAndAssertAttributeTakesPrecedenceAsync (line 175) asserts Assert.IsLessThan(25, stopwatch.Elapsed.TotalSeconds) against a runsettings-configured 25000ms timeout value that is baked into the same helper, i.e. the assertion and the fixture-under-test share the exact same magic number with no tolerance margin at all — a process-launch overhead of even a few hundred milliseconds beyond the configured timeout will fail the assertion.
• ❌ None of the four sites include a comment recording why the specific number was chosen or what margin over the "expected" duration it represents, making it hard for a future maintainer to know whether a CI failure here is a real regression or normal environment noise.

---

🤖 Suggested Improvement Tasks

Task 1: Add contributor guidance for elapsed-time upper-bound assertions

Priority: Medium
Estimated Effort: Small

Extend the existing "Testing Guidelines" documentation (the same section that already covers AcceptanceAssert.DurationPattern) with a short rule: when an acceptance test asserts an upper bound on Stopwatch.Elapsed around a real process launch, the assertion must include a comment explaining the margin relative to any configured timeout/budget in the same test, and the margin must generously exceed known slow-CI overhead (process start + JIT + host teardown), not just the nominal timeout value.

Task 2: Give TimeoutWhenExpiresTests.RunAndAssertAttributeTakesPrecedenceAsync a real margin over its own configured timeout

Priority: High
Estimated Effort: Small

In test/IntegrationTests/MSTest.Acceptance.IntegrationTests/TimeoutWhenExpiresTests.cs, the runsettings value injected into <{runSettingsEntry}>25000</{runSettingsEntry}> and the assertion Assert.IsLessThan(25, stopwatch.Elapsed.TotalSeconds); (line 175) use the same 25-second figure, leaving zero slack for process launch/build overhead. Increase the assertion's threshold to a value that comfortably exceeds the configured timeout (e.g. 25s timeout + several seconds of host-launch margin) and add a comment stating the relationship, so the test measures "the timeout attribute value took precedence, and the host still terminated promptly after it fired" rather than a tight race against the same number.

Task 3: Re-examine the TestHostProcessLifetimeHandlerTests 8-second ceilings

Priority: Medium
Estimated Effort: Small

In test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/TestHostProcessLifetimeHandlerTests.cs, Timeout_BoundsBlockingFinalizationWithoutDisposingRunningHandler and Timeout_BoundsBlockingDisposalWithoutRetryingIt both assert Assert.IsLessThan(8, stopwatch.Elapsed.TotalSeconds, ...) against a --timeout 500ms configuration and TESTINGPLATFORM_TESTHOSTCONTROLLER_FINALIZATION_TIMEOUT_SECONDS=0.5. Document (in a comment) how the 8-second figure was derived (e.g., process launch overhead budget + configured 0.5s finalization timeout + safety factor) so a future flaky-test triage can tell at a glance whether an observed 8.5s run is a real regression or expected CI noise, and consider whether a larger constant is warranted given other CI slow-agent evidence already documented in the repo's testing guidelines (macOS/Windows durations growing under load).

Task 4: Document the DataConsumerThroughputTests 7-second budget's dependency on core count

Priority: Low
Estimated Effort: Small

In test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DataConsumerThroughputTests.cs, MultipleDataConsumers_ShouldCompleteInReasonableTime registers Environment.ProcessorCount * 5 data consumers and then asserts Assert.IsLessThan(7, stopwatch.Elapsed.TotalSeconds, ...). Add a comment noting that the workload scales with Environment.ProcessorCount, so the 7-second budget should be revisited if CI agents change core counts, and consider whether the assertion should scale the threshold with Environment.ProcessorCount rather than using a single flat constant.

Task 5: Prefer rendezvous/barrier-based assertions over elapsed-time ceilings where feasible

Priority: Low
Estimated Effort: Medium

Where practical, follow the pattern already used in test/IntegrationTests/MSTest.Acceptance.IntegrationTests/TestDependencyExecutionTests.cs (DependsOn_RunsPrerequisitesFirst_AndLetsIndependentBranchesOverlap), which asserts overlap via an in-process rendezvous/barrier signal recorded by the generated asset rather than comparing elapsed wall-clock time. This removes CI-load sensitivity entirely for cases that are really checking "did X happen concurrently/promptly" rather than "did X complete within N seconds." Not every timing assertion can be converted (e.g., the true measurement of "the process actually respects --timeout" inherently needs a wall clock), but each of the four sites above should be evaluated for whether a deterministic signal-based check could replace or tighten the loose stopwatch-based one.

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type
2026-08-24	embedded-project-packability-intent-clarity	Custom
2026-08-25	package-readme-content-parity-gap	Custom
2026-08-26	videorecorder-core-logic-test-coverage-gap	Custom
2026-08-28	githubactions-feature-activation-logic-coverage-gap	Custom
2026-08-28	trx-report-metadata-parity-gap	Custom

---

🎯 Recommendations

Immediate Actions (This Week)

1. Fix the zero-margin TimeoutWhenExpiresTests assertion (Task 2) — Priority: High

Short-term Actions (This Month)

1. Add contributor guidance and document the derivation of the other three hardcoded thresholds (Tasks 1, 3, 4) — Priority: Medium
2. Evaluate converting elapsed-time ceilings to deterministic rendezvous-based checks where feasible (Task 5) — Priority: Low

Next analysis: 2026-09-01 — Focus area selected based on diversity algorithm

🤖 Automated content by GitHub Copilot. Generated by the Repository Quality Improver workflow. · auto · 135.3 AIC · ⌖ 3.44 AIC · ⊞ 16.8K · [◷]( · ◷)

_{Add this agentic workflow to your repo}

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repository-quality-improver.md@main

• expires on Sep 2, 2026, 10:30 PM UTC