Don't let the deadline truncate a run that already finished - #10625
Closed
Jakub Jareš (nohwnd) wants to merge 19 commits into
Closed
Don't let the deadline truncate a run that already finished#10625Jakub Jareš (nohwnd) wants to merge 19 commits into
Jakub Jareš (nohwnd) wants to merge 19 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>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds deadline-aware graceful stopping and hang-dump collection without misclassifying already-completed test runs.
Changes:
- Introduces deadline parsing, stop scheduling, and exit code 15.
- Adds deadline-triggered HangDump with one bounded test query per dump.
- Adds tests, reporting, localization, documentation, and API baselines.
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 callbacks. |
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Microsoft.Testing.Platform.UnitTests.csproj |
Includes the deadline helper. |
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Helpers/DeadlineHelperTests.cs |
Tests deadline parsing and margins. |
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Extensions/AbortAtDeadlineExtensionTests.cs |
Tests deadline-stop races and outcomes. |
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/HangDumpTests.cs |
Tests one query per dump tree. |
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/GitHubActionsExitCodeTests.cs |
Tests deadline exit-code reporting. |
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HangDumpTests.cs |
Covers deadline-triggered dumps. |
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/AbortAtDeadlineTests.cs |
Covers deadline-stop behavior end-to-end. |
src/Platform/Microsoft.Testing.Platform/Services/TestApplicationResult.cs |
Applies deadline exit code 15. |
src/Platform/Microsoft.Testing.Platform/Services/StopPoliciesService.cs |
Implements deadline policy state and callbacks. |
src/Platform/Microsoft.Testing.Platform/Services/IStopPoliciesService.cs |
Defines deadline policy operations. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hant.xlf |
Adds Traditional Chinese localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hans.xlf |
Adds Simplified Chinese localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.tr.xlf |
Adds Turkish localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ru.xlf |
Adds Russian localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pt-BR.xlf |
Adds Portuguese localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pl.xlf |
Adds Polish localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ko.xlf |
Adds Korean localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ja.xlf |
Adds Japanese localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.it.xlf |
Adds Italian localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.fr.xlf |
Adds French localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.es.xlf |
Adds Spanish localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.de.xlf |
Adds German localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.cs.xlf |
Adds Czech localization entries. |
src/Platform/Microsoft.Testing.Platform/Resources/PlatformResources.resx |
Adds deadline-stop messages. |
src/Platform/Microsoft.Testing.Platform/InternalAPI/net/InternalAPI.Unshipped.txt |
Records the 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 |
Disarms deadlines after execution. |
src/Platform/Microsoft.Testing.Platform/Helpers/ExitCodes.cs |
Adds exit code 15. |
src/Platform/Microsoft.Testing.Platform/Helpers/EnvironmentVariableConstants.cs |
Adds 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-driven 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 Traditional Chinese resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.zh-Hans.xlf |
Adds Simplified Chinese resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.tr.xlf |
Adds Turkish resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ru.xlf |
Adds Russian resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.pt-BR.xlf |
Adds Portuguese resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.pl.xlf |
Adds Polish resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ko.xlf |
Adds Korean resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ja.xlf |
Adds Japanese resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.it.xlf |
Adds Italian resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.fr.xlf |
Adds French resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.es.xlf |
Adds Spanish resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.de.xlf |
Adds German resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.cs.xlf |
Adds Czech resource entry. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/ExtensionResources.resx |
Adds deadline dump message. |
src/Platform/Microsoft.Testing.Extensions.HangDump/Microsoft.Testing.Extensions.HangDump.csproj |
Includes the deadline helper. |
src/Platform/Microsoft.Testing.Extensions.HangDump/InternalAPI/InternalAPI.Unshipped.txt |
Records HangDump deadline APIs. |
src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpProcessLifetimeHandler.cs |
Adds deadline dumps and bounded querying. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.zh-Hant.xlf |
Adds Traditional Chinese resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.zh-Hans.xlf |
Adds Simplified Chinese resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.tr.xlf |
Adds Turkish resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.ru.xlf |
Adds Russian resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.pt-BR.xlf |
Adds Portuguese resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.pl.xlf |
Adds Polish resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.ko.xlf |
Adds Korean resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.ja.xlf |
Adds Japanese resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.it.xlf |
Adds Italian resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.fr.xlf |
Adds French resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.es.xlf |
Adds Spanish resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.de.xlf |
Adds German resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/xlf/GitHubActionsResources.cs.xlf |
Adds Czech resource entry. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/Resources/GitHubActionsResources.resx |
Adds deadline exit-code explanation. |
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/PACKAGE.md |
Documents deadline failure 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 exit code 15 to its reason. |
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. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 75 out of 75 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpProcessLifetimeHandler.cs:350
- Disposing the timer waits only for its callback, but the callback returns immediately after publishing
_activityIndicatorTask; it does not wait for the dump. If the host exits independently while a deadline dump is running, this method can enumerate_dumpFilesconcurrently and publish before the dump adds its artifacts, so the completed dump is never surfaced. Claim the dump gate and await the published task before enumerating_dumpFiles.
if (_deadlineTimer is not null)
{
#if NETCOREAPP
await _deadlineTimer.DisposeAsync().ConfigureAwait(false);
#else
src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpProcessLifetimeHandler.cs:587
logFailureAsynccan itself throw (logger providers propagate failures), which makes this helper fault instead of returning the promised empty list.QueryOnceAndDumpTreeAsyncthen never reaches the dump loop, and the deadline path can kill the tree without producing a dump. Treat this diagnostic as best-effort by swallowing logging failures before returning the empty result.
catch (Exception ex)
{
await logFailureAsync(ex).ConfigureAwait(false);
return [];
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>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 75 out of 75 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpProcessLifetimeHandler.cs:595
logFailureAsyncis unprotected inside the fallback path. The composite logger propagates provider exceptions (Logging/Logger.cs:25-33), so a logger failure escapes instead of returning an empty list;QueryOnceAndDumpTreeAsyncthen skips every dump and thefinallypath only kills the process tree. Keep this diagnostic best-effort so a logging provider cannot suppress the dump.
await logFailureAsync(ex).ConfigureAwait(false);
Comment on lines
+357
to
+361
| await TryReportAsync( | ||
| () => _outputDevice.DisplayAsync( | ||
| this, | ||
| new FormattedTextOutputDeviceData(PlatformResources.AbortAtDeadlineMessage), | ||
| _cancellationTokenSource.CancellationToken), |
Member
Author
|
Closing: this was opened against The chain was #10018 -> #10614 -> #10618 -> #10623 -> #10625 -> #10627, each one answering the review on the previous duplicate. The work itself is real and is kept on the branch; #10627 stays open until it is reconciled onto #10018. Cause is fixed: delivery now pushes onto the branch of the pull request the work is about, and refuses rather than opening a new one. 🤖 |
Jakub Jareš (nohwnd)
deleted the
pilot/testfx-10623-answer-the-review-20260818074531
branch
August 19, 2026 10:41
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An absolute CI deadline could stop a run that had already completed its tests, turning a finished run into a truncated one. Disarm the deadline once test execution completes, and report the deadline stop through its own exit code (15,
TestExecutionStoppedAtDeadline) so it is not confused with an ordinary abort.HangDump also honors the deadline now: it schedules the dump a margin ahead of the deadline so the dump has a chance to finish before the CI runner hard-kills the process, and queries the in-progress tests once per dump operation rather than once per process, so a wedged host costs one bounded wait instead of one per process in the tree.
Verified:
build.cmd -projects test/UnitTests/Microsoft.Testing.Extensions.UnitTestspasses with the new unit tests.🤖
🤖