Extract duplicate banner logic into OutputDeviceBannerHelper

[Evangelink]

Summary

Both TerminalOutputDevice and SimplifiedConsoleOutputDeviceBase independently defined the same TESTINGPLATFORM_CONSOLEOUTPUTDEVICE_SKIP_BANNER constant and implemented nearly identical DisplayBannerAsync logic (~30 lines of duplicated StringBuilder-based banner construction).

Changes

• New file OutputDeviceBannerHelper.cs — a static helper class containing:
  • The shared TESTINGPLATFORM_CONSOLEOUTPUTDEVICE_SKIP_BANNER constant
  • BuildBannerText(IPlatformInformation, IRuntimeFeature, string?, string?) that constructs the platform information banner string
• Updated SimplifiedConsoleOutputDeviceBase.cs — removed the private constant and inline banner construction; now calls the shared helper
• Updated TerminalOutputDevice.cs — same treatment

Validation

• Build succeeded with 0 warnings, 0 errors across all TFMs (net8.0, net9.0, netstandard2.0)
• No public API changes (all types are internal)

Fixes #9170

[Copilot]

Pull request overview

This PR removes duplicated banner display logic from the Microsoft.Testing.Platform output devices by extracting the shared env-var constant and banner string construction into a new internal helper (OutputDeviceBannerHelper).

Changes:

• Added OutputDeviceBannerHelper with the shared TESTINGPLATFORM_CONSOLEOUTPUTDEVICE_SKIP_BANNER constant and BuildBannerText(...) banner formatter.
• Updated TerminalOutputDevice to use the shared constant and helper method instead of inline banner construction.
• Updated SimplifiedConsoleOutputDeviceBase to use the shared constant and helper method instead of inline banner construction.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs	Replaced duplicated banner env-var + StringBuilder banner formatting with OutputDeviceBannerHelper.
src/Platform/Microsoft.Testing.Platform/OutputDevice/SimplifiedConsoleOutputDeviceBase.cs	Same refactor as TerminalOutputDevice: delegate banner formatting and env-var name to the shared helper.
src/Platform/Microsoft.Testing.Platform/OutputDevice/OutputDeviceBannerHelper.cs	New shared helper containing the env-var constant and banner text builder.

Copilot's findings

• Files reviewed: 3/3 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 Summary

Clean, tightly-scoped deduplication — the refactoring is behaviourally faithful to both originals.

#	Dimension	Verdict
1	Algorithmic Correctness	⚠️ 1 NIT (inline)
2	Threading & Concurrency	✅ LGTM
3	Security & IPC Contract Safety	✅ LGTM
4	Public API & Binary Compatibility	✅ LGTM — all internal
5	Performance & Allocations	✅ LGTM
6	Cross-TFM Compatibility	✅ LGTM — Range operator already existed
7	Resource & IDisposable Management	✅ LGTM
8	Defensive Coding at Boundaries	⚠️ 1 NIT (inline) — commitHash[..10]
9	Localization & Resources	✅ LGTM
13	Test Completeness & Coverage	⚠️ 1 observation (below)
15	Code Structure & Simplification	✅ LGTM
16	Naming & Conventions	✅ LGTM
17	Documentation Accuracy	✅ LGTM
20	Build Infrastructure & Dependencies	✅ LGTM
21	Scope & PR Discipline	✅ LGTM

✅ 13/15 applicable dimensions fully clean — 0 blocking findings.

---

📝 Observation: test coverage for BuildBannerText

BuildBannerText is a pure function that accepts injected dependencies — it is now far easier to unit-test than the original inline code was. At the moment no tests target it directly; the formatting behaviour (version/commit/buildDate/architecture combinations) is covered only indirectly through acceptance tests.

Consider adding a small unit-test class (e.g., in Microsoft.Testing.Platform.UnitTests/OutputDevice/) that exercises the branching logic — in particular:

• Version null → no v... suffix
• CommitHash null but Version set → no +... fragment
• BuildDate null → no (UTC ...) fragment
• IsDynamicCodeSupported = false → no [arch - framework] suffix

This would prevent silent regressions if the helper's logic is changed in the future and make the intent explicit.

🤖 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 Expert Code Review (on PR ready) workflow. · 494.9 AIC · ⌖ 12.3 AIC · ◷