[fix] Fix flaky PortArgumentProcessor test: cache port locally instead of re-reading singleton

[nohwnd]

🤖 This is an automated fix generated by the Issue Repro Triage agent.

Fixes #15730

Root Cause

PortArgumentExecutor.Execute() reads _commandLineOptions.Port from the shared CommandLineOptions.Instance singleton. InProcessVsTestConsoleWrapper starts vstest.console in-process by calling executor.Execute(args) on a background Task.Run thread. This eventually calls PortArgumentExecutor.Initialize("1234"), which sets CommandLineOptions.Instance.Port = 1234.

If this background thread races with another test that called Initialize("2345") and is about to call Execute(), the background thread overwrites CommandLineOptions.Instance.Port from 2345 to 1234 before Execute() reads it — causing the ConnectToClientAndProcessRequests mock to be invoked with the wrong port.

Fix

Capture the port number in a private _port field during Initialize() and use that captured value in Execute() instead of re-reading the shared singleton. The assignment to _commandLineOptions.Port is preserved for callers that read it from CommandLineOptions.Instance.

Test

The 11 existing PortArgumentProcessorTests all pass. The flaky test ExecutorExecuteForValidConnectionReturnsArgumentProcessorResultSuccess is directly covered by the fix — it now uses the locally-captured port rather than the shared mutable singleton.

🔍 Triaged by Issue Repro Triage & Auto-Fix 🔍

🔍 Triaged by Issue Repro Triage & Auto-Fix 🔍

[Copilot]

Pull request overview

This PR fixes a flaky PortArgumentProcessor unit test by removing a race on the shared CommandLineOptions.Instance.Port value when vstest.console is run in-process on background threads.

Changes:

• Capture the port value in PortArgumentExecutor.Initialize() into a private _port field.
• Use the captured _port in Execute() (both for ConnectToClientAndProcessRequests and the timeout error message) instead of re-reading CommandLineOptions.Instance.Port.
• Preserve the existing assignment to _commandLineOptions.Port for compatibility with any other code that reads the singleton.

[nohwnd]

Review: src/vstest.console/ — Process Architecture & Scheduling Safety

Activated dimensions: Parallel Execution & Scheduling Safety, Process Architecture & Host Resolution

Analysis

The fix is correct and minimal. PortArgumentExecutor.Execute() was reading _commandLineOptions.Port from the shared CommandLineOptions.Instance singleton, which is mutable across test runs in InProcessVsTestConsoleWrapper. Caching the value in the instance field _port during Initialize() eliminates the TOCTOU race.

Checked items:

• ✅ Race condition eliminated — _port is an instance field, so each PortArgumentExecutor instance holds its own captured value. The write in Initialize() happens-before the read in Execute() within the same logical call chain.
• ✅ Default value is safe — If Execute() is called without Initialize(), _port is 0, which matches prior behavior (the singleton would also have been 0 or unset). No regression.
• ✅ Singleton assignment preserved — _commandLineOptions.Port = portNumber is still set, so callers reading the singleton continue to work correctly.
• ✅ Both uses of the port updated — Both ConnectToClientAndProcessRequests and the TimeoutException message now use _port. No dangling read of the singleton in Execute().
• ✅ PR description matches the diff — The root cause, fix, and affected test are accurately described.

No issues found.

---

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠