Surface out-of-process data collector messages on the MTP execution path - #16272
Merged
Azat Mukhametshin (azat-msft) merged 6 commits intoJul 15, 2026
Merged
Conversation
…path When an MTP application runs under vstest.console there is no testhost, so MtpProxyExecutionManager owns the out-of-process data-collection lifecycle directly. Data collector log messages are buffered on the run events handler and delivered during the AfterTestRunEnd exchange, but the MTP path only drained that buffer once, in BeforeTestRun (startup messages). The messages produced during the run and at session end (per-test-case notifications, warnings/errors, the Blame sequence-file path, disposal) were therefore silently dropped -- unlike the classic path, which relays them live via DataCollectionTestRunEventsHandler. Flush the buffer again after AfterTestRun completes so any out-of-process collector (Blame, Event Log, custom ones) has its full lifecycle surfaced. Add an acceptance test that runs a pure-MTP app with a generic out-of-process data collector and asserts the run completes with the normal summary and that the collector lifecycle (SessionStarted, TestHostLaunched, per-test-case events, SessionEnded) and per-test-case attachments are surfaced. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08a0cb32-b521-4cee-9cb4-485c25bb1ec0
Copilot started reviewing on behalf of
Azat Mukhametshin (azat-msft)
July 13, 2026 20:11
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a gap on the Microsoft.Testing.Platform (MTP) execution path where out-of-process data collector messages (and related lifecycle diagnostics) were buffered during/after the run but not forwarded to the console logger, causing them to be silently dropped.
Changes:
- Flush buffered out-of-process data collector log messages after
AfterTestRunEndcompletes inMtpProxyExecutionManager. - Refactor message forwarding into a
SurfaceDataCollectionMessageshelper used both pre-run (startup) and post-run (session end). - Add an acceptance test covering MTP + generic out-of-proc data collector (SampleDataCollector) message surfacing and per-test-case attachment production.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/MtpUnderVstestTests.cs | Adds an end-to-end acceptance test to verify collector lifecycle messages and attachments are surfaced for MTP runs. |
| src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpProxyExecutionManager.cs | Drains buffered datacollector log messages after the run completes to avoid silent drops on the MTP path. |
Azat Mukhametshin (azat-msft)
marked this pull request as draft
July 13, 2026 21:18
Copilot started reviewing on behalf of
Azat Mukhametshin (azat-msft)
July 13, 2026 21:46
View session
Copilot started reviewing on behalf of
Azat Mukhametshin (azat-msft)
July 13, 2026 21:57
View session
Azat Mukhametshin (azat-msft)
force-pushed
the
fix/mtp-datacollector-hang
branch
from
July 14, 2026 10:32
bbceb0a to
ceab6e6
Compare
Copilot started reviewing on behalf of
Azat Mukhametshin (azat-msft)
July 14, 2026 10:33
View session
- SurfaceDataCollectionMessages now also flushes and clears the buffered raw (telemetry) messages, not just log messages, mirroring ProxyExecutionManagerWithDataCollection on the classic path so MTP runs no longer drop datacollector raw messages or retain them for the manager lifetime. - Acceptance test: assert the full datacollector message prefix for the TestCaseStarted/TestCaseEnded checks so they cannot match unrelated output; point the collector source files at a dedicated subdirectory instead of the results directory; and assert the exact per-test-case attachment count while excluding that source directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fb7de7a6-b8a1-4ebb-8266-5c5a948f245c
Copilot started reviewing on behalf of
Azat Mukhametshin (azat-msft)
July 14, 2026 11:09
View session
Member
Author
|
Addressed the review feedback (commit 00ac892):
Verified: Note: the two review comments about |
Azat Mukhametshin (azat-msft)
marked this pull request as ready for review
July 14, 2026 13:44
Copilot started reviewing on behalf of
Azat Mukhametshin (azat-msft)
July 14, 2026 13:44
View session
Comment on lines
+238
to
+239
| // The run must complete with the usual summary rather than hang at shutdown. | ||
| ValidateSummaryStatus(2, 1, 1); |
Comment on lines
+251
to
+254
| // The collector emits one attachment per test case that reports a start through the forwarded | ||
| // TestCaseStart events. All four MtpMSTestProject test cases report a start on this path (the | ||
| // skipped one still surfaces a start node), so four attachments must land in the results | ||
| // directory. Exclude the collector's own source directory so only the moved attachments are counted. |
The MtpMSTestProject asset now has five test cases (after the runsettings env-var test merged from main): three pass, one fails, one is skipped. Update the new RunMtpApplicationWithGenericOutOfProcDataCollectorCompletesRun test to assert the 3/1/1 summary and five per-test-case attachments, and correct the stale comment wording. Addresses PR review feedback and fixes the failing Windows Release integration test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e19db317-61c7-4dff-8adf-e0d44088c5eb
Copilot started reviewing on behalf of
Azat Mukhametshin (azat-msft)
July 14, 2026 20:17
View session
…ndardOutput The MtpMSTestProject asset gained a fifth test (RunSettingsEnvironmentVariableIsInjected) via a concurrently-merged PR, but this test's summary assertion still expected four tests. All MtpApp acceptance tests now consistently assert 3 passed / 1 failed / 1 skipped. Also update the stale class-level asset description comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e19db317-61c7-4dff-8adf-e0d44088c5eb
Copilot started reviewing on behalf of
Azat Mukhametshin (azat-msft)
July 14, 2026 20:50
View session
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Azat Mukhametshin (azat-msft)
July 15, 2026 08:08
View session
Jakub Jareš (nohwnd)
approved these changes
Jul 15, 2026
Azat Mukhametshin (azat-msft)
merged commit Jul 15, 2026
f0c2756
into
microsoft:main
20 checks passed
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.
Problem
Running a pure Microsoft.Testing.Platform (MTP) app under
vstest.consolewith a generic out-of-process data collector (Blame, Event Log, or a custom/Collect:collector) completed the run but silently dropped the collector's run-time and session-end messages and attachments from the console — only the startupSessionStartedline appeared.Root cause
On the MTP path there is no testhost, so
MtpProxyExecutionManagerowns the out-of-process data-collection lifecycle directly. The data collector runs indatacollector.exeand its log messages reach the console in two hops:AfterTestRunEndhandshake,DataCollectionRequestSender.SendAfterTestRunEndAndGetResultpulls eachDataCollectionMessageoff the socket and buffers it into the in-process_dataCollectionEventsHandler.Messageslist.The MTP path performed hop 2 only once, in
BeforeTestRun(startup messages). The run-time and session-end batch (TestHostLaunched,TestCaseStarted/Ended,SessionEnded, warnings,Dispose) arrives via hop 1 after the run, so the buffer filled up and was never flushed. The classic path relays these live viaDataCollectionTestRunEventsHandler; the MTP path had no live pump and no post-run drain.Fix
Extract a
SurfaceDataCollectionMessageshelper and flush the buffer again afterAfterTestRuncompletes. General fix for any out-of-process collector — no per-collector special case. The classic testhost path and the working MTP Code Coverage integration are untouched.Test
New acceptance test
RunMtpApplicationWithGenericOutOfProcDataCollectorCompletesRunruns a pure-MTP app with the genericSampleDataCollectorand asserts the run completes with the normal summary and that the collector lifecycle (SessionStarted,TestHostLaunched, per-test-case events,SessionEnded) and per-test-case attachments are surfaced. Fails onmain(missingTestHostLaunched), passes with this change.Validation
MtpUnderVstestTestsfull suite: 10/10 pass (incl. Code Coverage path — no regression)./Blametest: 4/4 pass, run completes in ~1s (no hang).Microsoft.TestPlatform.CrossPlatEngineclean.