Don't let the deadline truncate a run that already finished - #10627
Don't let the deadline truncate a run that already finished#10627Jakub Jareš (nohwnd) wants to merge 20 commits into
Conversation
CI systems (AzDO, GitHub Actions) hard-cancel a job at a fixed wall-clock time. When that happens the runner kills the test process, so we lose the TRX/HTML/AzDO reports and get no dump for a hanging test. This teaches MTP about that deadline through an environment variable and lets it react a bit early, while it still controls its own shutdown: - TESTINGPLATFORM_DEADLINE is an absolute instant (ISO 8601, parsed to UTC). - At deadline minus stop margin (default 60s) an in-process extension asks the framework to gracefully stop scheduling new tests, so the session ends normally and every reporter finalizes. - At deadline minus dump margin (default 30s) the out-of-process HangDump controller takes a dump of the process tree and kills the host, for the case where the host is wedged and never reaches the graceful stop. The deadline comes from the environment, so there is no hardcoded timeout in MTP. The margins are MTP side policy and are env overridable. Wiring the real deadline from the CI timeout is a small bit of YAML, left for a follow-up. It is opt-in: with no deadline set both timers stay unarmed and there is no behavior change. The graceful stop also degrades to a no-op when the framework does not expose IGracefulStopTestExecutionCapability. Verified: build.cmd -pack passes 0/0, and the new acceptance tests pass (AbortAtDeadlineTests 5/5, HangDumpTests 30/30 including the deadline dump). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…-cancellation # Conflicts: # src/Platform/Microsoft.Testing.Extensions.TrxReport/InternalAPI/InternalAPI.Unshipped.txt
Fixes from the review of microsoft#10018: - HangDump: the deadline timer and the inactivity timer both wrote _activityIndicatorTask, so the losing one could overwrite the winner's real dump task with a completed no-op, and Dispose would stop waiting for the dump mid-flight. Move the one-shot guard into a TriggerDumpOnce trampoline so only the winning timer assigns _activityIndicatorTask. - HangDump: the deadline path logged "Hang dump timeout expired", which is misleading because no inactivity timeout expired. Give the deadline case its own reason and its own output message (new HangDumpDeadlineReached resource + regenerated xlf). - AbortAtDeadlineExtension: compute a local non-null stopAt instead of dereferencing _stopAt.Value, so there is no nullable deref. - Add the UTF-8 BOM to the three new source files to satisfy the charset=utf-8-bom editorconfig rule. Verified: build 0/0, AbortAtDeadline 5/5, full HangDump suite 30/30. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…-cancellation # Conflicts: # src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt
More fixes from the review of microsoft#10018: - HangDump: the absolute deadline timer was armed only after the pipe handshake in OnTestHostProcessStartedAsync. A test host that wedges during startup never connects back over the pipe, so those waits blocked past the deadline and the deadline dump/kill was never armed, which is exactly the case the deadline is for. Arm the deadline timer right after we have the test host process info (before the handshake). The dump path only needs the PID; the in-progress-test list needs the consumer pipe, so I make it best-effort and skip it when the pipe never connected. - HangDump: the winning timer published _activityIndicatorTask without any ordering against disposal. Disposal could read the field as null, release, and tear the pipes down while a dump the timer just started was still running. Guard the "take the dump once" gate and the task publish under one lock, and have Dispose/DisposeAsync take that lock, claim the gate so no new dump can start, and capture the in-flight task to wait on outside the lock. The lock is a System.Threading.Lock on net9.0 and an object below it, so it still compiles on netstandard2.0. - AbortAtDeadline and HangDump: deadline - margin on DateTimeOffset throws for a very old (but valid) deadline or a large margin. Add a shared DeadlineHelper.SubtractSaturating that clamps at DateTimeOffset.MinValue, so underflow means "already in the past" -> act immediately. Verified: build 0/0, AbortAtDeadline 5/5, full HangDump suite 30/30. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The automated re-review flagged seven spots after the last push. All are in the two deadline timers and the hang dump resource text. AbortAtDeadlineExtension: - DataTypesConsumed is now empty. The extension only implements IDataConsumer so the message bus keeps a live reference to it (which keeps its timer alive). Returning [TestNodeUpdateMessage] made the bus route every test result to a no-op ConsumeAsync, which is O(test-count) for nothing. - The timer callback calls HandleDeadlineAsync directly instead of Task.Run. On single-threaded runtimes (browser/WASI) Task.Run can queue work that never runs; the method is already async and yields at the first await. - Arming the timer clamps a far-future due time to the Timer maximum (~49.7 days) instead of throwing. The run is disposed long before that, so the timer never fires early in practice. - The graceful stop now runs in its own try/catch, separate from the best-effort diagnostics. A logging or output-device failure can no longer skip the stop, and the diagnostics failure log is itself swallowed so it cannot re-throw and skip the stop either. HangDumpProcessLifetimeHandler: - Same far-future clamp on the deadline dump timer. - The in-progress-test query before dumping is wrapped in try/catch. A non-null pipe client is not necessarily connected (it is created when the host sends its pipe name but connected later), so a deadline dump firing in that window could hit an unconnected pipe. Any failure is logged and swallowed so it cannot block taking the dump and killing the tree. - Renamed the resource HangDumpDeadlineReached to HangDumpDeadlineApproaching and reworded the text and log reason to "approaching". The dump fires at deadline minus the dump margin, so the deadline has not been reached yet. Regenerated the xlf files. Local Debug pack is 0/0. AbortAtDeadline acceptance tests 5/5 and the full HangDump acceptance suite 30/30. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…-cancellation # Conflicts: # src/Platform/Microsoft.Testing.Extensions.HangDump/InternalAPI/InternalAPI.Unshipped.txt # src/Platform/Microsoft.Testing.Extensions.MSBuild/InternalAPI.Unshipped.txt # src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt
…ocalization, disposal) This is the review round on the deadline-aware cancellation prototype. The big behavioural change is the exit code: a run that the deadline cuts short no longer reports success. - A deadline-truncated run now returns ExitCode.TestExecutionStoppedAtDeadline (15) instead of 0. The stop policy service gained IsDeadlineTriggered and a deadline callback, mirroring the max-failed-tests path, and the extension sets the flag before it asks for the graceful stop so the exit code cannot be raced by session finalization. Without this a suite that never finished would go green on CI, which is the opposite of what the feature is for. - The in-proc extension is only registered for a console run. Server mode builds the framework per request, so it would re-arm the timer against the same absolute instant on every request and fire immediately once the deadline passed. Discovery requests are skipped too. - The operator-facing description and console message moved into PlatformResources and are regenerated into the 13 locales, matching the HangDump side. - The graceful-stop logging is now wrapped the same way as the rest of the handler so a throwing logger cannot escape the timer callback and FailFast the process, and the handler task is drained with a bounded wait on disposal so it is not still touching the logger and output device after teardown. - DeadlineHelper now logs the resolved deadline and margins, warns when the deadline is set but malformed, warns when the framework has no graceful-stop capability, and warns when the dump margin is not smaller than the stop margin. The offset-less footgun is at least visible in the log now. I removed the dead non-negative margin guard. - HangDump disposes both timers on every teardown path (not only the clean exit), takes the dump outside the dump lock (claim the gate under the lock, yield before the work), and lets the deadline dump be published through the normal exited path by returning from the failed handshake when a dump is already in progress. - Added DeadlineHelperTests covering parsing, timezone handling, margin fallback, and the saturating subtraction, which were only exercised indirectly before. Capped the acceptance asset's wait so a broken stop path fails the assertions fast instead of hanging until the harness times out. Local Debug pack is 0/0. DeadlineHelper unit tests 28/28 and AbortAtDeadline acceptance tests 5/5. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0147c6f-0cfb-4dd6-b420-582b3c7988de
…-cancellation # Conflicts: # src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/InternalAPI/InternalAPI.Unshipped.txt # src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/PACKAGE.md
The timer callback could pass the _disposed check, then Dispose could run to completion and read _handleDeadlineTask while it was still null (draining nothing), and only afterwards would the callback publish the task and run the handler against torn-down services. Publish the task and set/read _disposed under a shared lock so the two cannot interleave: either the callback publishes before Dispose captures it (Dispose then drains it), or Dispose sets _disposed first and the callback observes it and never starts the handler. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0147c6f-0cfb-4dd6-b420-582b3c7988de
The in-progress-test list gathered before a hang dump was only best-effort against failures, not against a connected-but-wedged host. RequestReplyAsync waited on the application token, which is not cancelled while the run is still "in progress" -- exactly when the deadline dump fires -- so a host that connected its pipe but then stopped replying would block the dump and kill indefinitely and consume the whole (default 30s) dump margin, producing no dump. Wrap the query in a linked CTS bounded to 5s so the metadata collection can never eat the margin; the timeout surfaces as OperationCanceledException and is swallowed like any other query failure, letting the dump and tree-kill proceed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0147c6f-0cfb-4dd6-b420-582b3c7988de
The deadline timer was armed at construction and only disarmed on Dispose, so if it fired after the run had already finished -- while the reporters were still finalizing TRX/HTML/AzDO output -- a fully-completed run wrongly exited with code 15 and reported that tests stopped early. AbortAtDeadlineExtension now also implements ITestSessionLifetimeHandler. When OnTestSessionFinishingAsync fires (right after the framework invoker returns, before the reporters render), it sets an _executionCompleted flag under the same lock the timer callback and Dispose use. OnDeadlineReached checks that flag on its fast-path and again inside the lock, so a late timer fire during reporting is a no-op. It never un-marks a real mid-execution truncation: if the timer already fired during execution, IsDeadlineTriggered is already set and setting the flag afterwards is harmless. Wire the extension into testSessionLifetimeHandlers in TestHostBuilder and move the TestSessionLifetimeHandlersContainer registration below that so the container includes it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0147c6f-0cfb-4dd6-b420-582b3c7988de
HandleDeadlineAsync is started synchronously inside _lock and its returned task is published to _handleDeadlineTask. An async method does not necessarily yield at its first await: the logger, output device, the empty deadline-callback queue and the framework graceful-stop capability can all return already-completed tasks, which would run the whole handler synchronously while _lock is held. If the graceful stop then synchronously waited on session finishing or disposal (both take _lock), that would deadlock. Yield immediately (await Task.Yield()) at the top of HandleDeadlineAsync, matching HangDumpProcessLifetimeHandler.TakeDumpOfTreeAsync, so control returns to the caller, the task is observed, and _lock is released before any handler work runs. Task.Yield posts the continuation back to the current context, so it is cooperative and safe on a single-threaded runtime (browser/WASI), unlike Task.Run. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0147c6f-0cfb-4dd6-b420-582b3c7988de
…-cancellation Resolved conflicts: - InternalAPI.Unshipped.txt for HangDump, MSBuild and Retry: both sides appended entries, kept both. - GitHubActionsReport/PACKAGE.md: kept main's rewritten job-summary bullet and cross-module aggregation note, re-applied the deadline early stop and TestExecutionStoppedAtDeadline (15) mentions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Four fixes to the deadline-aware cancellation prototype: Deadline callback registration could lose a callback. The registering thread could read IsDeadlineTriggered as false, the trigger could then commit and snapshot a still-empty queue, and only afterwards would the callback be enqueued -- where a one-shot deadline never reaches it. Guard the transition and the callback list with one lock, so each callback runs exactly once. The deadline verdict is committed before the graceful stop is requested (it has to be, or the framework can finalize the session before the flag lands). If StopTestExecutionAsync then threw, the verdict stayed set and a run that executed every test reported TestExecutionStoppedAtDeadline. Revert it when the stop request fails. The extension disarmed the deadline from ITestSessionLifetimeHandler, which it claimed ran before the reporters. It does not: the extension is an IDataConsumer, and NotifyTestSessionEndAsync drains the bus, runs the non-consumer handlers, and only then runs the consumer handlers -- this one appended after the reporters. A deadline reached during that window still marked a fully-executed run as truncated. Signal completion from the host instead, right after the test framework invoker returns. Document ExitCode.TestExecutionStoppedAtDeadline, which was the only member without a summary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…dump The deadline handler checked "test execution completed" only before it started, then yielded and logged before committing the verdict, so the invoker could return in that window and a run in which every test had finished was still reported as deadline-truncated. Decide it under the same lock the completion transition uses, immediately before the commit. The hang dump asked the test host for the in-progress tests once per process in the tree, so a wedged pipe cost five seconds per process and a six-process tree ate the whole 30s dump margin. Ask once per dump. Also save DeadlineHelperTests.cs as UTF-8 with BOM, as .editorconfig requires for C# files. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
GetReason only had assertions for exit codes 9, 13 and 14, so replacing the new deadline arm with the unknown fallback would have gone unnoticed. Assert the deadline-specific text for 15 and pin it against the fallback. The "query the in-progress tests once per dump operation, not once per process" rule was only exercised by an acceptance test with a single process and a responsive pipe, which passes either way. Move the query and the per-process fan-out into QueryOnceAndDumpTreeAsync and test it with a six-process tree and a stalled query; TakeDumpOfTreeAsync itself dumps and then kills every process it walks, so it cannot be driven against a real tree. Moving the query back into the loop now fails the test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
RevertDeadlineTrigger cleared the flag that also recorded whether the deadline callbacks had run, so reverting the verdict re-armed them: a later trigger could run every callback a second time, and a registration arriving after the revert was queued into a list nothing drains any more. Track callback execution separately and clear only the verdict. Let a dump interrupt the pipe handshake. Killing the test host does not complete this process's own WaitConnectionAsync, so a host that wedged before connecting kept OnTestHostProcessStartedAsync blocked for the full five-minute hang timeout, well past the CI deadline, and OnTestHostProcessExitedAsync never ran to publish the dump that had been taken. Move the in-progress-test query bound into QueryInProgressTestsWithTimeoutAsync so a test can prove the product gives up on a reply that never arrives. The previous test supplied a fake that timed out on its own, so it passed whether or not the product bounded the wait. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Claiming the run closes the door on NotifyTestExecutionCompleted, so a completion arriving after the claim can no longer disarm the deadline. The user-facing message sat between the claim and the commit, so a run that finished while that message was being written was ignored and still reported as stopped at the deadline. Commit the verdict immediately after the claim and write the message afterwards. The new test fails against the previous order. Construct the HangDump consumer pipe client with exitProcessOnConnectionLoss: false. It only carries the best-effort in-progress-test query, and its peer is a test host we are usually about to dump and kill, so a disconnect during the query called IEnvironment.Exit on the controller that still had to take and publish the dump. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds deadline-aware graceful stopping and hang-dump handling while avoiding false truncation reports for completed runs.
Changes:
- Adds deadline parsing, scheduling, exit-code handling, and race-safe stop policies.
- Makes HangDump deadline-aware and bounds stalled host queries.
- Adds unit, acceptance, reporting, API-baseline, and localization coverage.
Reviewed changes
Copilot reviewed 75 out of 75 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/StopPoliciesServiceTests.cs |
Tests deadline policy state and concurrency. |
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Microsoft.Testing.Platform.UnitTests.csproj |
Links the deadline helper into tests. |
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Helpers/DeadlineHelperTests.cs |
Tests deadline and margin parsing. |
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Extensions/AbortAtDeadlineExtensionTests.cs |
Tests deadline-stop race handling. |
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/HangDumpTests.cs |
Tests bounded dump queries. |
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/GitHubActionsExitCodeTests.cs |
Tests deadline exit-code reporting. |
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HangDumpTests.cs |
Verifies deadline-triggered dumps. |
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/AbortAtDeadlineTests.cs |
Verifies deadline stopping end-to-end. |
src/Platform/Microsoft.Testing.Platform/Services/TestApplicationResult.cs |
Applies the deadline exit verdict. |
src/Platform/Microsoft.Testing.Platform/Services/StopPoliciesService.cs |
Adds synchronized one-shot deadline state. |
src/Platform/Microsoft.Testing.Platform/Services/IStopPoliciesService.cs |
Defines deadline policy operations. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hant.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hans.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.tr.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ru.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pt-BR.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pl.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ko.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ja.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.it.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.fr.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.es.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.de.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.cs.xlf |
Adds localized resource placeholders. |
src/Platform/Microsoft.Testing.Platform/Resources/PlatformResources.resx |
Adds deadline-stop messages. |
src/Platform/Microsoft.Testing.Platform/InternalAPI/net/InternalAPI.Unshipped.txt |
Records async disposal API. |
src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt |
Records new internal deadline APIs. |
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.Framework.cs |
Registers the deadline extension. |
src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.MessageBus.cs |
Signals execution completion promptly. |
src/Platform/Microsoft.Testing.Platform/Helpers/ExitCodes.cs |
Adds deadline exit code 15. |
src/Platform/Microsoft.Testing.Platform/Helpers/EnvironmentVariableConstants.cs |
Defines deadline environment variables. |
src/Platform/Microsoft.Testing.Platform/Helpers/DeadlineHelper.cs |
Parses deadlines and margins. |
src/Platform/Microsoft.Testing.Platform/Extensions/AbortAtDeadlineExtension.cs |
Implements deadline-aware graceful stopping. |
src/Platform/Microsoft.Testing.Extensions.TrxReport/InternalAPI/InternalAPI.Unshipped.txt |
Updates embedded API baseline. |
src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt |
Updates embedded API baseline. |
src/Platform/Microsoft.Testing.Extensions.MSBuild/InternalAPI.Unshipped.txt |
Updates embedded API baseline. |
src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt |
Updates embedded API baseline. |
src/Platform/Microsoft.Testing.Extensions.HtmlReport/InternalAPI/InternalAPI.Unshipped.txt |
Updates embedded API baseline. |
src/Platform/Microsoft.Testing.Extensions.HotReload/InternalAPI/InternalAPI.Unshipped.txt |
Updates embedded API baseline. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.zh-Hant.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.zh-Hans.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.tr.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ru.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.pt-BR.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.pl.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ko.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ja.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.it.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.fr.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.es.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.de.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.cs.xlf |
Adds deadline dump resource. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/ExtensionResources.resx |
Adds deadline dump message. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Microsoft.Testing.Extensions.HangDump.csproj |
Embeds the deadline helper. |
src/Platform/Microsoft.Testing.Extensions.HangDump/InternalAPI/InternalAPI.Unshipped.txt |
Records new HangDump internals. |
src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpProcessLifetimeHandler.cs |
Adds deadline dumps and bounded queries. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.zh-Hant.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.zh-Hans.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.tr.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.ru.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.pt-BR.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.pl.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.ko.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.ja.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.it.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.fr.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.es.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.de.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.cs.xlf |
Adds deadline exit-code text. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/GitHubActionsResources.resx |
Adds deadline failure reason. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/PACKAGE.md |
Documents deadline annotations. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/InternalAPI/InternalAPI.Unshipped.txt |
Updates the exit-code API baseline. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsExitCode.cs |
Maps deadline exit-code reasons. |
src/Platform/Microsoft.Testing.Extensions.CtrfReport/InternalAPI/InternalAPI.Unshipped.txt |
Updates embedded API baseline. |
src/Platform/Microsoft.Testing.Extensions.AzureDevOpsReport/InternalAPI/InternalAPI.Unshipped.txt |
Updates embedded API baseline. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Swallowing faults does not cover a logger or output device that wedges: it hands back a task that never completes. The user-facing deadline message is awaited after the verdict is committed and before the graceful stop is requested, so a wedged output device left the run recorded as stopped at the deadline while the stop was never asked for. Bound each best-effort diagnostic with TimeoutAfterAsync, which abandons the task and keeps observing it. In HangDump, the delegate that reports a failed in-progress-test query is a logger call and logger providers can fail. That throw escaped QueryInProgressTestsWithTimeoutAsync and took the dump with it, even though the query is explicitly best-effort. Guard it and always return the empty list. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 75 out of 75 changed files in this pull request and generated 3 comments.
Suppressed comments (1)
src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpProcessLifetimeHandler.cs:402
- The central startup-wedge recovery remains uncovered. The new acceptance test hangs only after the normal pipe handshake, and the added unit tests exercise the query helpers; removing this handshake cancellation would leave all of them passing while reintroducing the five-minute block described by the PR. Add a test where the host never connects, the deadline fires, and verify startup returns and the dump is published.
// Interrupt the pipe handshake, if one is still in flight. We are about to dump and kill the test
// host, and a host that wedged before connecting leaves that handshake waiting for a connection
// that will never arrive; killing it does not complete our own wait, so nothing else would end it
// before DefaultHangTimeSpanTimeout and the dump would never reach OnTestHostProcessExitedAsync
// to be published. Cancelled outside _dumpLock on purpose: the waiters' continuations can run
// inline here, and they continue into a handshake that takes _dumpLock again on its way out.
try
{
handshakeCancellation?.Cancel();
| { | ||
| // A dump is already in progress; the failed handshake is expected. Return normally so | ||
| // OnTestHostProcessExitedAsync runs and publishes the dump that is being taken. | ||
| await _logger.LogDebugAsync($"Test host handshake failed after the dump started; continuing so the dump can be published. {ex}").ConfigureAwait(false); |
| catch (Exception ex) | ||
| { | ||
| // Writing the list is a convenience; it must never block taking the dump and killing the tree. | ||
| await _logger.LogDebugAsync($"Could not write the in-progress tests next to the dump. Continuing with the dump. {ex}").ConfigureAwait(false); | ||
| } |
| // throw would escape the caller, which is explicitly best-effort, and skip the dump entirely. | ||
| try | ||
| { | ||
| await logFailureAsync(ex).ConfigureAwait(false); |
|
Closing: the six commits this was holding are now on #10018 (head This was the tip of a chain my automation created - #10018 -> #10614 -> #10618 -> #10623 -> #10625 -> #10627 - where each session answered the review on the previous duplicate and opened another pull request against 🤖 |
A run that reaches the CI deadline should be dumped and stopped, but a run that already finished should not be reported as truncated, and the machinery that gets us there should not block on, or be killed by, a test host that is already going away.
Claiming the run for the deadline is what closes the door on a late completion, so nothing may be awaited between that claim and the verdict. The user-facing message sat in exactly that gap, so a run that finished while the message was being written was ignored and still reported as stopped at the deadline. The verdict is now committed immediately after the claim and the message follows it.
Swallowing faults is not enough for those diagnostics on its own: a wedged logger or output device does not throw, it hands back a task that never completes. The message after the claim is awaited when the verdict is already committed and before the stop is requested, so a wedged provider left the run recorded as stopped at the deadline while
StopTestExecutionAsyncwas never called. Each best-effort report is now bounded, and the abandoned task is kept observed.RevertDeadlineTriggerundoes the verdict when the graceful stop it was meant to precede could not be requested. It cleared the same flag that recorded whether the one-shot deadline callbacks had run, so reverting also re-armed them: a later trigger could run every callback a second time, and a registration arriving after the revert was queued into a list nothing drains any more. The verdict and the callback gate are now separate.In HangDump, a dump interrupts the pipe handshake. Killing the test host does not complete this process's own
WaitConnectionAsync— that pipe is waiting for a client that will never connect — so a host that wedged during startup keptOnTestHostProcessStartedAsyncblocked for the full five-minute hang timeout, well past the CI deadline, andOnTestHostProcessExitedAsyncnever ran to publish the dump that had been taken.The consumer pipe client is now built with
exitProcessOnConnectionLoss: false. It carries only the best-effort in-progress-test query, and its peer is a host we are usually about to dump and kill, so a disconnect during the query used to callExiton the controller that still had to publish the dump. The bound on that query also moved intoQueryInProgressTestsWithTimeoutAsync, so it is the product that gives up on a reply that never arrives; the previous test supplied a fake that timed out on its own and passed either way. That query is best-effort down to its diagnostics too: the delegate that reports a failed query is a logger call, and a throw from it used to escape and skip the dump, so it is now guarded and the empty-list fallback always stands.Verified:
build.cmdand theMicrosoft.Testing.PlatformandMicrosoft.Testing.Extensionsunit tests pass locally.🤖