Enable anti-flicker progress version check in AnsiTerminalTestProgressFrame

[Evangelink]

Description

In AnsiTerminalTestProgressFrame.Render the TestProgressState branch had && false hard-coded in the comparison against the previously rendered line, which made the optimization always fall through to the full re-render branch (CSI K erase + AppendTestWorkerProgress).

The intent of the optimization is documented in the comment that follows the check ("only update the timestamp if possible to avoid flicker") and the parallel TestDetailState branch a few lines below already does the correct comparison:

if (previouslyRenderedLine.ProgressId == detailItem.Id
    && previouslyRenderedLine.ProgressVersion == detailItem.Version)

Why this is safe

TestProgressState.Version is bumped in TestProgressStateAwareTerminal.UpdateWorker only when there is new data for that worker:

internal void UpdateWorker(int slotIndex)
{
    if (GetShowProgress())
    {
        _counter++;
        TestProgressState? progress = _progressItems[slotIndex];
        progress?.Version = _counter;
    }
}

So when no test result has come in between two 500 ms refresh ticks the Id+Version pair matches and we can rewrite just the duration cell at the right of the line instead of erasing and re-rendering the whole counters/assembly-name segment. This is exactly what the comment and the symmetric TestDetailState branch describe.

Risk

Low. && false was introduced as part of the original "Show running tests" change (#4221) and looks like a leftover/oversight that disabled the optimization without ever re-enabling it. With the change, when nothing has happened we now write a much shorter ANSI sequence (just the duration) instead of CSI K + re-emitting the counters and assembly name — reducing flicker and bandwidth without changing observable output.

If for some reason the assumption about Version accuracy ever fails, the worst-case is a one-tick stale render of the counters; the very next tick that bumps Version will trigger a full re-render again.

Context

Spotted while investigating #6753.

[Copilot]

Pull request overview

This PR re-enables an intended ANSI progress rendering optimization in AnsiTerminalTestProgressFrame.Render by comparing the previously rendered line’s progress Id and Version, allowing duration-only updates when the progress data has not changed (reducing flicker/bandwidth in terminal output).

Changes:

• Replace the hard-coded && false condition with a real (Id && Version) comparison for TestProgressState lines to enable partial re-rendering.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/AnsiTerminalTestProgressFrame.cs	Enables version-based “duration-only” updates for progress lines to reduce flicker and unnecessary full line re-renders.

Copilot's findings

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

[Evangelink]

Expert Review — 21 Dimensions

Verdict: ✅ All Clear — no issues found.

#	Dimension	Status	Notes
1	Algorithmic Correctness	✅ Clean	Fix mirrors the TestDetailState branch exactly. Id + Version equality is a correct stable-render gate because Version is only bumped on new data in UpdateWorker. Worst-case stale render is one tick, self-healed on next frame.
2	Threading & Concurrency	N/A	Rendering is single-threaded per frame.
3	Security & IPC Contract Safety	N/A	Terminal output only.
4	Public API Surface	✅ Clean	AnsiTerminalTestProgressFrame is internal sealed; no public API changed.
5	Backward Compatibility	✅ Clean	Observable terminal output is unchanged or improved (less flicker). Not a behavioral regression.
6	Resource Management	N/A	No allocations added. The fix reduces ANSI output per tick.
7	Error Handling	N/A	No error paths changed.
8	Localization	N/A	No user-facing strings.
9	Performance	✅ Improvement	Removing the erroneous && false re-enables the optimization that writes only the duration cell instead of CSI K + full line re-render. Net improvement.
10	Test Coverage	⚠️ Minor	No new tests added. The optimization branch (ProgressId == Id && ProgressVersion == Version) was unreachable before; it would be nice to have a unit test covering the "same Id + Version → duration-only update" path, but not a blocker for a leftover-dead-code fix.
11	Code Style & Conventions	✅ Clean	Consistent with the adjacent TestDetailState block and .editorconfig rules.
12	Naming & Readability	✅ Clean	No changes.
13	Documentation / Comments	✅ Clean	Existing comment ("only update the timestamp if possible to avoid flicker") now accurately describes the live code path.
14	Cross-TFM Correctness	N/A	Single file, no conditional compilation.
15	Dependency & Package Changes	N/A	No package or project file changes.
16	init Accessor Policy	N/A	No new public API.
17	PublicAPI.Unshipped.txt	N/A	No public surface area changed.
18	CLI Help/Info Acceptance Tests	N/A	No CLI options changed.
19	IPC Contract Stability	N/A	No wire format touched.
20	Scope Discipline	✅ Clean	Single-concern, single-file change. Well-justified in the PR description.
21	Root Cause vs. Symptom	✅ Clean	The root cause (&& false leftover) is directly addressed. PR description correctly traces the origin to #4221.

Summary

This is a clean, well-documented bug fix that re-enables a deliberately designed optimization. The && false was clearly an oversight from the original "Show running tests" PR. The fix is symmetric with the already-correct TestDetailState branch and carries a very low regression risk.

The only non-blocking note is the absence of a unit test covering the re-enabled path — adding one would improve confidence but is not required for this type of rendering optimization fix.

Generated by Expert Code Review (on open) for issue #8348 · ● 3.3M