[efficiency-improver] perf: avoid GroupBy lazy evaluation for empty-check in AppendTestRunSummary

[Evangelink]

Goal and Rationale

Replace artifactGroups.Any() with _artifacts.Count > 0 in TerminalTestReporter.Summary.cs.

Focus area: Code-Level Efficiency

The original code:

IEnumerable<IGrouping<bool, TestRunArtifact>> artifactGroups = _artifacts.GroupBy(a => a.OutOfProcess);
if (artifactGroups.Any())   // ← triggers first GroupBy traversal
{
    terminal.AppendLine();
}

foreach (IGrouping<bool, TestRunArtifact> artifactGroup in artifactGroups)  // ← triggers second GroupBy traversal

GroupBy is lazy. Calling Any() on the resulting IEnumerable<IGrouping<...>> iterates _artifacts once to find the first element. The foreach below then re-evaluates the GroupBy from scratch, iterating _artifacts a second time. In total: two full passes over _artifacts plus two GroupBy enumerator allocations for every call to AppendTestRunSummary.

Approach

_artifacts is declared as List<TestRunArtifact> (line 41 of TerminalTestReporter.cs). List<T>.Count is an O(1) property read with no allocation. Replacing artifactGroups.Any() with _artifacts.Count > 0:

• Eliminates the first GroupBy traversal entirely
• Eliminates the first GroupBy enumerator allocation
• Leaves the foreach as the sole consumer of artifactGroups (one pass, unchanged)

The semantics are equivalent: if _artifacts is empty, GroupBy produces no groups; if non-empty, it always produces at least one group.

Energy Efficiency Evidence

Proxy metric: Memory allocation count + iteration count (proxy for CPU energy draw)

Condition	Before	After
Empty _artifacts	GroupBy() iter alloc + Any() call	O(1) Count read, zero alloc
Non-empty _artifacts (n artifacts)	2 × full pass over n elements + 2 GroupBy enumerator allocs	1 × full pass over n elements + 1 GroupBy enumerator alloc

This is an end-of-run path called once per test run. The absolute savings are modest, but the pattern is a clean correctness improvement (avoids a common lazy-LINQ double-evaluation pitfall). The same idiom (_artifacts.Count == 0) is already used at line 140 of TerminalTestReporter.cs.

GSF principle — Hardware Efficiency: reducing unnecessary object allocations and loop iterations makes the same hardware do less work per functional unit (test run).

Trade-offs

None. The change reduces computational work, makes the code slightly clearer (intent: "do we have any artifacts?"), and is consistent with the existing _artifacts.Count == 0 guard in the same file.

Reproducibility

No benchmarks needed for this path — the improvement is analytically demonstrable from the lazy-LINQ semantics. For profiling: run a test suite that produces artifacts and use dotnet-trace/PerfView to count allocations in AppendTestRunSummary.

Test Status

The change is a one-line expression substitution with no logic change. CI will run the full test suite.

🤖 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 Efficiency Improver workflow. · 2K AIC · ⌖ 35.1 AIC · ⊞ 58.6K · [◷]( · ◷)

Add this agentic workflows to your repo

To install this agentic workflow, run

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

[Copilot]

Pull request overview

This PR makes a small performance improvement in the Microsoft.Testing.Platform terminal reporter by avoiding a deferred-LINQ double-enumeration pattern when checking whether any artifacts exist before printing the artifacts section.

Changes:

• Replace artifactGroups.Any() with _artifacts.Count > 0 to avoid an extra GroupBy enumeration and enumerator allocation.
• Preserve existing output semantics while reducing redundant work in AppendTestRunSummary.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Summary.cs	Avoids enumerating a lazy GroupBy sequence solely to check emptiness, reducing redundant iteration/allocation in the run summary rendering path.

Copilot's findings

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

[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.

✅ 22/22 dimensions clean — no findings.

Review notes (selected dimensions):

• D1 — Algorithmic Correctness: The substitution is semantically equivalent. LINQ GroupBy on _artifacts produces at least one grouping for every non-empty source; no filtering occurs. Therefore artifactGroups.Any() ↔ _artifacts.Count > 0 for all reachable inputs, including the empty-list case where both evaluate to false.

• D5 — Performance & Allocations: The fix is correct and well-motivated. Enumerable.GroupBy buffers all source elements on the first MoveNext() call; calling .Any() on the resulting IEnumerable<IGrouping<...>> triggers that full buffering pass (plus one GroupBy enumerator allocation), and the subsequent foreach triggers a second independent pass. Replacing the guard with _artifacts.Count collapses to a single O(1) property read with no allocation, and the foreach becomes the sole traversal. The optimization is consistent with the existing PrintOutOfProcessArtifacts method (line 140), which already uses if (_artifacts.Count == 0) { return; } for the same list.

• D2 — Threading & Concurrency: _artifacts is a List<TestRunArtifact> populated via ArtifactAdded. No thread-safety concern is introduced by this PR — both the old and new paths read _artifacts unsynchronized within AppendTestRunSummary, which is the pre-existing design. The change does not alter the threading topology.

• D6 — Cross-TFM Compatibility: List<T>.Count is available on all targeted frameworks (net462, netstandard2.0, net8.0, net9.0) — no compatibility issue.

• D21 — Scope & PR Discipline: Single-line, single-concern change. Well-scoped.