Add orchestrator TestInProgress/TestDiscovered overloads for SDK parity

[Evangelink]

What

Adds the two orchestrator overloads the dotnet/sdk dotnet test integration calls but the shared reporter lacked:

• TestInProgress(assembly, targetFramework, architecture, executionId, instanceId, testNodeUid, displayName)
• TestDiscovered(executionId, displayName, uid, filePath, lineNumber)

Both are additive and delegate to the existing execution-id-keyed core methods (the extra arguments are carried for signature parity; the shared in-progress tracking and discovery summary are keyed by execution id / display name). This lets dotnet/sdk consume the Microsoft.Testing.Platform.Internal.DotnetTest source package without call-site changes.

Discovered while actually plugging the package into dotnet/sdk: with these two overloads the SDK's reporter call sites compile against the shared source (the only remaining blocker there is a packaging/XliffTasks resx issue, not the reporter API).

Verification

Platform clean on net8.0/net9.0/netstandard2.0 (0 warnings); TerminalTestReporterTests 108/0.

[Copilot]

Pull request overview

Adds missing TerminalTestReporter orchestrator overloads used by the dotnet/sdk dotnet test integration, enabling SDK/source-package consumption without modifying SDK call sites.

Changes:

• Added TestInProgress(assembly, targetFramework, architecture, executionId, instanceId, testNodeUid, displayName) overload delegating to the existing execution-id keyed implementation.
• Added TestDiscovered(executionId, displayName, uid, filePath, lineNumber) overload delegating to the existing discovery tracking implementation.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Summary.cs	Adds a discovery overload carrying uid/file/line metadata for orchestrator signature parity.
src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Messaging.cs	Adds an in-progress overload carrying assembly/TFM/arch/instance metadata for orchestrator signature parity.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Evangelink]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review: Add orchestrator TestInProgress/TestDiscovered overloads for SDK parity

The additive approach is sound and the delegation is semantically correct — the new overloads carry only the identity/keying parameters needed by the orchestrator and correctly discard the extras (assembly, targetFramework, architecture, instanceId). One MAJOR gap and one MODERATE concern need attention before merging.

---

Blocking Findings

[MAJOR] Test Completeness (Dimension 13): no unit tests for the new overloads

The PR description reports "TerminalTestReporterTests 108/0" — unchanged from pre-PR, confirming zero tests were added for the two new overloads. Searching TerminalTestReporterTests.cs confirms neither the 7-param TestInProgress nor the 5-param TestDiscovered signature appears anywhere in the test file.

The delegation is trivial, but the key things to verify directly are:

1. The 7-param TestInProgress correctly threads executionId/testNodeUid/displayName to the core (and silently discards assembly, targetFramework, architecture, instanceId).
2. The 5-param TestDiscovered correctly maps executionId and a non-null displayName to the core.
3. The null-displayName branch — displayName ?? string.Empty — produces defined (and intentional) output rather than a silent blank entry in the discovery list.

Suggested skeleton (following the existing [TestClass]/[TestMethod] pattern in TerminalTestReporterTests.cs):

[TestMethod]
public void OrchestratorTestInProgress_DelegatesCorrectly()
{
    using TerminalTestReporter reporter = CreateTestReporter(...);
    reporter.AssemblyRunStarted("asm.dll", "net9.0", "x64", executionId: "exec1", instanceId: "inst1");

    // Should not throw; extra args are accepted and discarded
    reporter.TestInProgress(
        assembly: "asm.dll", targetFramework: "net9.0", architecture: "x64",
        executionId: "exec1", instanceId: "inst1",
        testNodeUid: "Namespace.Class.Method", displayName: "Method");
}

[TestMethod]
public void OrchestratorTestDiscovered_DelegatesCorrectly()
{
    using TerminalTestReporter reporter = CreateTestReporter(...);
    reporter.AssemblyRunStarted("asm.dll", "net9.0", "x64", executionId: "exec1", instanceId: "inst1");

    reporter.TestDiscovered("exec1", displayName: "Method", uid: "Namespace.Class.Method",
        filePath: "Test.cs", lineNumber: 42);

    Assert.AreEqual(1, reporter._assemblies["exec1"].DiscoveredTests);
}

[TestMethod]
public void OrchestratorTestDiscovered_NullDisplayName_BehaviorIsDocumented()
{
    using TerminalTestReporter reporter = CreateTestReporter(...);
    reporter.AssemblyRunStarted("asm.dll", "net9.0", "x64", executionId: "exec1", instanceId: "inst1");

    // Verify null doesn't crash and document what it produces
    reporter.TestDiscovered("exec1", displayName: null, uid: "uid", filePath: null, lineNumber: null);
}

---

[MODERATE] Null displayName silently adds a blank entry (inline comment above)

See the inline comment on TerminalTestReporter.Summary.cs line 162 for the full scenario. Short summary: null → string.Empty → DiscoveredTestDisplayNames.Add("") → blank indented line in the discovery summary. Prefer a null-guard over the silent coalesce.

---

22-Dimension Verdict Table

#	Dimension	Status	Notes
1	Algorithmic Correctness	⚠️ MODERATE	Null displayName coalesced to "" renders blank line in discovery summary
2	Threading & Concurrency	✅ LGTM	Delegation to existing thread-safe code; no new shared state
3	Security & IPC Contract Safety	✅ LGTM	No new trust boundaries
4	Public API & Binary Compatibility	✅ LGTM	Class is internal sealed — new public method follows existing pattern; no PublicAPI.Unshipped.txt entry needed; TestDiscovered overload visibility (internal) mirrors its core
5	Performance & Allocations	✅ LGTM	Inline delegation; not a hot path
6	Cross-TFM Compatibility	✅ LGTM	No TFM-gated APIs introduced
7	Resource & IDisposable Management	N/A	No disposable objects created
8	Defensive Coding at Boundaries	✅ LGTM	Existing ApplicationStateGuard.Unreachable() on unknown executionId is preserved
9	Localization & Resources	✅ LGTM	No user-facing strings added
10	Test Isolation	N/A	No test file changes
11	Assertion Quality	N/A	No test file changes
12	Flakiness Patterns	N/A	No test file changes
13	Test Completeness & Coverage	🔴 MAJOR	No unit tests for either new overload; 108-test count unchanged
14	Data-Driven Test Coverage	N/A	No test file changes
15	Code Structure & Simplification	✅ LGTM	Expression-body delegation is the correct pattern here
16	Naming & Conventions	✅ LGTM	Parameter names match existing conventions and SDK callsite
17	Documentation Accuracy	⚠️ NIT	XML doc on TestDiscovered overload doesn't say what happens when displayName is null
18	Analyzer & Code Fix Quality	N/A	No src/Analyzers/ changes
19	IPC Wire Compatibility	N/A	No serialization changes
20	Build Infrastructure & Dependencies	N/A	No eng/ changes
21	Scope & PR Discipline	✅ LGTM	Tightly scoped; PR description is clear
22	PowerShell Scripting Hygiene	N/A	No .ps1 changes

Summary: 12/22 applicable dimensions clean. 1 MAJOR (missing tests), 1 MODERATE (null displayName), 1 NIT (doc gap).

[Copilot]

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

Comments suppressed due to low confidence (1)

src/Platform/Microsoft.Testing.Platform.Internal.DotnetTest/Microsoft.Testing.Platform.Internal.DotnetTest.csproj:103

• The comment above the resource packing still describes the old approach (Generator="MSBuild:Compile" + StronglyTyped* metadata + pinned manifest). With the new build .props switching to GenerateSource="true" + Namespace, this comment is now misleading and should be updated so future maintainers don’t reintroduce the XliffTasks collision you mention elsewhere.

    <!--
      Terminal localized resources. The resx + xlf are shipped under build/ (NOT contentFiles) so they are not
      auto-compiled with the wrong metadata; the auto-imported build-extension props below adds the resx as a
      strongly-typed <EmbeddedResource> (Generator="MSBuild:Compile" plus StronglyTyped* metadata) with a pinned
      ManifestResourceName, and XliffTasks (present in the consumer, e.g. dotnet/sdk via Arcade) finds the sibling
      xlf/ to emit satellite assemblies.
    -->

• Files reviewed: 6/6 changed files
• Comments generated: 3

[Copilot]

Copilot's findings

• Files reviewed: 7/7 changed files
• Comments generated: 2

[0101]

Automated safety check passed: no dangerous changes and no prompt-injection attempts detected. Approving as requested. Note: this is a quick safety sanity check, not a full code review.

[Evangelink]

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Build Failure Analysis workflow. · 286.7 AIC · ⌖ 12.5 AIC · ⊞ 46.9K · ◷

[Evangelink]

IDE0008 — build-breaking: var is not allowed here because the type is not apparent from the right-hand side (method call, not constructor). The repo enforces IDE0008 as an error.

Suggested change

	var terminalReporter = CreateOrchestratorReporter(stringBuilderConsole);
	TerminalTestReporter terminalReporter = CreateOrchestratorReporter(stringBuilderConsole);

[Copilot]

Copilot's findings

• Files reviewed: 25/25 changed files
• Comments generated: 1

[Evangelink]

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Build Failure Analysis workflow. · 504.4 AIC · ⌖ 13.4 AIC · ⊞ 46.9K · ◷

[Evangelink]

IDE0008: The repo enforces explicit types for method-call initializers (csharp_style_var_elsewhere = false:warning, promoted to error). Replace var with the explicit return type of CreateOrchestratorReporter.

Suggested change

	var terminalReporter = CreateOrchestratorReporter(stringBuilderConsole);
	TerminalTestReporter terminalReporter = CreateOrchestratorReporter(stringBuilderConsole);

[Evangelink]

🔍 Build Failure Analysis

Summary — The build of Microsoft.Testing.Platform.UnitTests failed due to two IDE0008 ("use explicit type instead of var") violations introduced in the new test methods added by this PR.

Root cause: IDE0008 — implicit var where explicit type is required

The project enforces IDE0008 as an error (via EnforceCodeStyleInBuild). Both failures are in newly added test methods in TerminalTestReporterTests.cs where var was used to hold the result of CreateOrchestratorReporter(...). Because the right-hand side is a method call (not a new T() expression), the type is not immediately apparent and the rule requires an explicit type declaration.

CreateOrchestratorReporter returns TerminalTestReporter, so both var declarations must be replaced with TerminalTestReporter.

Affected file / errors

Code	File	Line	Message
IDE0008	TerminalTestReporterTests.cs:1624	1624	Use explicit type instead of var
IDE0008	TerminalTestReporterTests.cs:1709	1709	Use explicit type instead of var

Proposed fix — replace var with TerminalTestReporter on both declarations:

-        var terminalReporter = CreateOrchestratorReporter(stringBuilderConsole);
+        TerminalTestReporter terminalReporter = CreateOrchestratorReporter(stringBuilderConsole);

Inline suggestions are posted below for each line.

---

Build overview

• Outcome: FAILED
• Duration: 276.8 s
• MSBuild: 18.7.0-preview
• Projects: 46 total, 3 failed (Build.proj, NonWindowsTests.slnf, Microsoft.Testing.Platform.UnitTests.csproj)
• Errors: 5 (4 unique IDE0008 hits × 2 TFMs + 1 aggregated "Build failed.")
• Warnings: 0

All MSBuild errors (4 unique)

Code	Project	File:Line	Message
IDE0008	Microsoft.Testing.Platform.UnitTests	TerminalTestReporterTests.cs:1624	Use explicit type instead of var
IDE0008	Microsoft.Testing.Platform.UnitTests	TerminalTestReporterTests.cs:1709	Use explicit type instead of var

(Each error is reported twice because the project multi-targets two TFMs.)

---

_{🤖 Generated by the Build Failure Analysis workflow using [binlog-mcp]((dev.azure.com/redacted) · commit 1b43da5}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Build Failure Analysis workflow. · 459.4 AIC · ⌖ 22.4 AIC · ⊞ 46.9K · [◷]( · ◷)

[Evangelink]

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Build Failure Analysis workflow. · 459.4 AIC · ⌖ 22.4 AIC · ⊞ 46.9K · ◷

[Evangelink]

🔧 IDE0008 — Use explicit type instead of var. The return type of CreateOrchestratorReporter is TerminalTestReporter; the project enforces IDE0008 as a build error.

Suggested change

	var terminalReporter = CreateOrchestratorReporter(stringBuilderConsole);
	TerminalTestReporter terminalReporter = CreateOrchestratorReporter(stringBuilderConsole);

[Evangelink]

🔧 IDE0008 — Use explicit type instead of var. The return type of CreateOrchestratorReporter is TerminalTestReporter; the project enforces IDE0008 as a build error.

Suggested change

	var terminalReporter = CreateOrchestratorReporter(stringBuilderConsole);
	TerminalTestReporter terminalReporter = CreateOrchestratorReporter(stringBuilderConsole);

[Copilot]

Copilot's findings

• Files reviewed: 25/25 changed files
• Comments generated: 1

[Evangelink]

🧪 Test quality grade — PR #9294

10 tests graded across 1 file (5 new, 5 modified); all score B or above. The predominant reason for B grades is body length — new tests range from 35–55 lines, slightly above the ~30-line threshold for single-behavior unit tests. Two tests with a single focused assertion (OrchestratorTestInProgress, AssemblyRunStarted_AfterRetry) also cap at B. The four A-grade tests are tight, well-structured, and carry multiple complementary assertions.

Δ	Test	Grade	Band	Notes
new	TerminalTestReporterTests. AppendTestDiscoverySummary_ ForOrchestrator_ PrintsPerAssemblyDiscoveredCountsAndTotal	B	80–89	Six assertions covering per-assembly headers, test names, run total, and negative in-process wording; body ~35 lines.
mod	TerminalTestReporterTests. AssemblyRunCompleted_ WhenExecutionIdUnknown_ SummaryReprintsRecapAndReportsFailure	B	80–89	Six assertions covering handshake-failure lifecycle + verdict escalation; assertion replaced (ZeroTestsRan → Failed!) strengthens the check.
mod	TerminalTestReporterTests. AssemblyRunStarted_ AfterRetry_ RendersLatestAttemptCounts	B	80–89	Single assertion verifying the latest-attempt count in the summary line; cosmetic var→explicit-type change only.
new	TerminalTestReporterTests. TerminalTestReporter_ OrchestratorTestInProgress_ TracksActiveTestLikeCoreOverload	B	80–89	Single assertion on progress-frame content; ~55-line body uses AutoResetEvent synchronization for determinism.
new	TerminalTestReporterTests. TerminalTestReporter_ WhenOrchestratorDiscoveryDisplayNameIsNull_ CountsTestAndFallsBackToUid	B	80–89	Four assertions cover count, negative blank-line check, uid-fallback, and display name; body at ~43 lines.
new	TerminalTestReporterTests. TestExecutionCompleted_ WhenTestsWereRetried_ AnnotatesTryNumberAndSummaryRetriedCount	B	80–89	Five assertions: try-annotation, negative boundary, summary suffix, assembly banner; body ~44 lines.
mod	TerminalTestReporterTests. AssemblyRunCompleted_ WhenKnownAssemblyFails_ PrintsExecutableSummary	A	90–100	No issues found.
mod	TerminalTestReporterTests. AssemblyRunCompleted_ WhenKnownAssemblySucceeds_ DoesNotPrintExecutableSummary	A	90–100	Thoughtfully guards DoesNotContain with a prior presence assertion to prevent vacuous passing.
new	TerminalTestReporterTests. TestExecutionCompleted_ WhenAssemblyExitsNonZeroButTestsPassed_ ReportsFailedVerdict	A	90–100	No issues found.
mod	TerminalTestReporterTests. TestExecutionCompleted_ WhenHandshakeFailures_ PrintsRecapAndFailsRun	A	90–100	No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 435.2 AIC · ⌖ 25 AIC · ⊞ 43.6K · [◷]( · ◷)