Add --retry-failed-tests-delay option to MTP retry package

[Copilot]

The retry extension had no way to introduce a wait between retry attempts, making it less useful for tests interacting with external systems (message queues, databases, etc.) where a cooldown period reduces flakiness.

Changes

• New CLI option --retry-failed-tests-delay: accepts a timespan value using the same unit-aware format as --hangdump-timeout (e.g. 200, 1s, 2.5m, 1h; default unit is milliseconds)
• RetryCommandLineOptionsProvider: registers the option, validates it requires --retry-failed-tests, validates the argument via TimeSpanParser.TryParse, and enforces a valid range (non-negative, no greater than int.MaxValue milliseconds — the Task.Delay maximum)
• RetryOrchestrator: strips the delay arg from arguments forwarded to child processes (with a bounds check before adding argIndex + 1 to the cleanup list); applies Task.Delay at the top of the retry loop (after attemptCount++, before any per-attempt resources are allocated), skipping the first attempt; uses TryParse with a Length > 0 guard instead of Parse for defensive handling
• Resources: added description and invalid-argument error strings; description now includes a bare-millisecond example (200) alongside unit-suffixed examples
• Tests: coverage for valid/invalid timespan values (including whitespace-only and out-of-range inputs), the dependency-on-retry-option validation, and an integration test (RetryFailedTests_WithDelay_StripsDelayFromChildArgs) that verifies the option is correctly stripped from child-process arguments end-to-end

Usage

dotnet test -- --retry-failed-tests 3 --retry-failed-tests-delay 2s

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a configurable delay between retry attempts for the retry-failed-tests extension to reduce flakiness for external-system-dependent tests.

Changes:

• Introduces --retry-failed-tests-delay CLI option with unit-aware timespan parsing and validation.
• Applies an inter-attempt Task.Delay in RetryOrchestrator and strips the option from forwarded child-process args.
• Updates help text, package docs, resources, and adds unit tests for option validation.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs	Adds unit tests for delay option parsing/validation and dependency on --retry-failed-tests.
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopTestSourceHostTests.cs	Adjusts nullability usage (null!) in mocks.
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs	Updates expected help output to include the new delay option.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs	Removes delay arg from forwarded args; parses delay option and delays between attempts.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryCommandLineOptionsProvider.cs	Registers the new option and validates dependency + timespan format.
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hant.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hans.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.tr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ru.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pt-BR.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pl.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ko.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ja.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.it.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.fr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.es.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.de.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.cs.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/ExtensionResources.resx	Adds delay option description and invalid-argument error string.
src/Platform/Microsoft.Testing.Extensions.Retry/PACKAGE.md	Documents the new delay feature and CLI option usage.

Copilot's findings

• Files reviewed: 20/20 changed files
• Comments generated: 4

[Amaury Levé (Evangelink)]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

1. [Critical — will cause test failure]HelpInfoAllExtensionsTests.cs: The --help output test was updated for --retry-failed-tests-delay, but the Info_WithAllExtensionsRegistered_OutputFullInfoContent test was not. Because AssertOutputMatchesLines performs sequential line-by-line matching, the test will fail in CI — the actual --info output now includes the delay option under RetryCommandLineOptionsProvider, but the expected pattern jumps straight to TerminalTestReporterCommandLineOptionsProvider. Four lines need to be inserted in the pattern.

2. [Minor — coverage gap]RetryTests.cs (IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption): The whitespace-only input (" ") takes a distinct early-return path in TimeSpanParser.TryParse but is not included in the [DataRow] set.

3. [Moderate — missing behavioral coverage] Neither the new unit tests nor the existing RetryFailedTestsTests.cs integration tests verify the actual delay behaviour in RetryOrchestrator: that Task.Delay is applied between retry attempts (not before the first), and that --retry-failed-tests-delay is stripped from the child-process argument list.

Recommendations

• Add the --retry-failed-tests-delay block to the --info expected-output string in HelpInfoAllExtensionsTests.cs (around line 342, after the --retry-failed-tests-max-tests entry).
• Add [DataRow(" ")] to IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption.
• Add an acceptance or unit test that exercises the orchestrator's delay and arg-stripping logic end-to-end.

---

Generated by Test Expert Reviewer

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Amaury Levé (Evangelink)]

Summary

Workflow: Expert Code Reviewer
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

[Defensive] TimeSpanParser.Parse in orchestrator path (RetryOrchestrator.cs:158)
The orchestrator calls TimeSpanParser.Parse (which throws FormatException) rather than the TryParse pattern used elsewhere in the same method. No bounds check on retryDelayArgs before [0] access either, inconsistent with how cmdRetries is guarded at line 98. Low practical risk since validation runs first, but adds a latent uncaught-exception path.

[Resources] Pipe server created before delay fires (RetryOrchestrator.cs:181 / 282)
RetryFailedTestsPipeServer (OS named-pipe listener) is created ~100 lines before the Task.Delay fires. For large delays (minutes/hours), the pipe holds OS resources for the entire wait period before the child process is launched. Moving the delay to the top of the loop body (immediately after attemptCount++) would scope the pipe server to only the window where it's needed.

Positive Observations

• Correctness of the retry-delay logic is sound: delay correctly skips attempt 1 (attemptCount > 1), fires before each subsequent attempt, and applies ConfigureAwait(false) throughout.
• Cancellation token is correctly threaded into Task.Delay — cancellation during the wait will propagate as OperationCanceledException, which is the expected behavior for this framework.
• Argument stripping (indexToCleanup) correctly removes both the option flag and its value, consistent with all other stripped options.
• Validation in RetryCommandLineOptionsProvider is complete: dependency on --retry-failed-tests, format validation via TryParse, and test coverage for valid/invalid inputs.
• TimeSpanParser.TryParse correctly rejects empty strings and invalid formats per the new unit tests.

Overall Assessment

The implementation is logically correct and well-structured. Both findings are minor defensive/resource concerns rather than correctness bugs. No threading issues, security vulnerabilities, or public API breaking changes were identified.

---

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot's findings

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

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

The open review comment about the resource string was addressed in commit 9b43a08 — the error message now mentions the supported range: "requires a valid time span value that is non-negative and no greater than 2147483647ms (~24.20:31:23.647) (e.g. 200, 1s, 2.5m, 1h)". All 14 XLF localization files were updated in the same commit.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

The retry extension had no way to introduce a wait between retry attempts, making it less useful for tests interacting with external systems (message queues, databases, etc.) where a cooldown period reduces flakiness.

Changes

• New CLI option --retry-failed-tests-delay: accepts a timespan value using the same unit-aware format as --hangdump-timeout (e.g. 200, 1s, 2.5m, 1h; default unit is milliseconds)
• RetryCommandLineOptionsProvider: registers the option, validates it requires --retry-failed-tests, validates the argument via TimeSpanParser.TryParse, and enforces a valid range (non-negative, no greater than int.MaxValue milliseconds — the Task.Delay maximum)
• RetryOrchestrator: strips the delay arg from arguments forwarded to child processes (with a bounds check before adding argIndex + 1 to the cleanup list); applies Task.Delay at the top of the retry loop (after attemptCount++, before any per-attempt resources are allocated), skipping the first attempt; uses TryParse with a Length > 0 guard instead of Parse for defensive handling
• Resources: added description and invalid-argument error strings; description now includes a bare-millisecond example (200) alongside unit-suffixed examples
• Tests: coverage for valid/invalid timespan values (including whitespace-only and out-of-range inputs), the dependency-on-retry-option validation, and an integration test (RetryFailedTests_WithDelay_StripsDelayFromChildArgs) that verifies the option is correctly stripped from child-process arguments end-to-end

Usage

dotnet test -- --retry-failed-tests 3 --retry-failed-tests-delay 2s

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a configurable delay between retry attempts for the retry-failed-tests extension to reduce flakiness for external-system-dependent tests.

Changes:

• Introduces --retry-failed-tests-delay CLI option with unit-aware timespan parsing and validation.
• Applies an inter-attempt Task.Delay in RetryOrchestrator and strips the option from forwarded child-process args.
• Updates help text, package docs, resources, and adds unit tests for option validation.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs	Adds unit tests for delay option parsing/validation and dependency on --retry-failed-tests.
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopTestSourceHostTests.cs	Adjusts nullability usage (null!) in mocks.
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs	Updates expected help output to include the new delay option.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs	Removes delay arg from forwarded args; parses delay option and delays between attempts.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryCommandLineOptionsProvider.cs	Registers the new option and validates dependency + timespan format.
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hant.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hans.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.tr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ru.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pt-BR.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pl.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ko.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ja.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.it.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.fr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.es.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.de.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.cs.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/ExtensionResources.resx	Adds delay option description and invalid-argument error string.
src/Platform/Microsoft.Testing.Extensions.Retry/PACKAGE.md	Documents the new delay feature and CLI option usage.

Copilot's findings

• Files reviewed: 20/20 changed files
• Comments generated: 4

[Amaury Levé (Evangelink)]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

1. [Critical — will cause test failure]HelpInfoAllExtensionsTests.cs: The --help output test was updated for --retry-failed-tests-delay, but the Info_WithAllExtensionsRegistered_OutputFullInfoContent test was not. Because AssertOutputMatchesLines performs sequential line-by-line matching, the test will fail in CI — the actual --info output now includes the delay option under RetryCommandLineOptionsProvider, but the expected pattern jumps straight to TerminalTestReporterCommandLineOptionsProvider. Four lines need to be inserted in the pattern.

2. [Minor — coverage gap]RetryTests.cs (IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption): The whitespace-only input (" ") takes a distinct early-return path in TimeSpanParser.TryParse but is not included in the [DataRow] set.

3. [Moderate — missing behavioral coverage] Neither the new unit tests nor the existing RetryFailedTestsTests.cs integration tests verify the actual delay behaviour in RetryOrchestrator: that Task.Delay is applied between retry attempts (not before the first), and that --retry-failed-tests-delay is stripped from the child-process argument list.

Recommendations

• Add the --retry-failed-tests-delay block to the --info expected-output string in HelpInfoAllExtensionsTests.cs (around line 342, after the --retry-failed-tests-max-tests entry).
• Add [DataRow(" ")] to IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption.
• Add an acceptance or unit test that exercises the orchestrator's delay and arg-stripping logic end-to-end.

---

Generated by Test Expert Reviewer

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Amaury Levé (Evangelink)]

Summary

Workflow: Expert Code Reviewer
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

[Defensive] TimeSpanParser.Parse in orchestrator path (RetryOrchestrator.cs:158)
The orchestrator calls TimeSpanParser.Parse (which throws FormatException) rather than the TryParse pattern used elsewhere in the same method. No bounds check on retryDelayArgs before [0] access either, inconsistent with how cmdRetries is guarded at line 98. Low practical risk since validation runs first, but adds a latent uncaught-exception path.

[Resources] Pipe server created before delay fires (RetryOrchestrator.cs:181 / 282)
RetryFailedTestsPipeServer (OS named-pipe listener) is created ~100 lines before the Task.Delay fires. For large delays (minutes/hours), the pipe holds OS resources for the entire wait period before the child process is launched. Moving the delay to the top of the loop body (immediately after attemptCount++) would scope the pipe server to only the window where it's needed.

Positive Observations

• Correctness of the retry-delay logic is sound: delay correctly skips attempt 1 (attemptCount > 1), fires before each subsequent attempt, and applies ConfigureAwait(false) throughout.
• Cancellation token is correctly threaded into Task.Delay — cancellation during the wait will propagate as OperationCanceledException, which is the expected behavior for this framework.
• Argument stripping (indexToCleanup) correctly removes both the option flag and its value, consistent with all other stripped options.
• Validation in RetryCommandLineOptionsProvider is complete: dependency on --retry-failed-tests, format validation via TryParse, and test coverage for valid/invalid inputs.
• TimeSpanParser.TryParse correctly rejects empty strings and invalid formats per the new unit tests.

Overall Assessment

The implementation is logically correct and well-structured. Both findings are minor defensive/resource concerns rather than correctness bugs. No threading issues, security vulnerabilities, or public API breaking changes were identified.

---

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot's findings

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

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

The open review comment about the resource string was addressed in commit 9b43a08 — the error message now mentions the supported range: "requires a valid time span value that is non-negative and no greater than 2147483647ms (~24.20:31:23.647) (e.g. 200, 1s, 2.5m, 1h)". All 14 XLF localization files were updated in the same commit.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

The retry extension had no way to introduce a wait between retry attempts, making it less useful for tests interacting with external systems (message queues, databases, etc.) where a cooldown period reduces flakiness.

Changes

• New CLI option --retry-failed-tests-delay: accepts a timespan value using the same unit-aware format as --hangdump-timeout (e.g. 200, 1s, 2.5m, 1h; default unit is milliseconds)
• RetryCommandLineOptionsProvider: registers the option, validates it requires --retry-failed-tests, validates the argument via TimeSpanParser.TryParse, and enforces a valid range (non-negative, no greater than int.MaxValue milliseconds — the Task.Delay maximum)
• RetryOrchestrator: strips the delay arg from arguments forwarded to child processes (with a bounds check before adding argIndex + 1 to the cleanup list); applies Task.Delay at the top of the retry loop (after attemptCount++, before any per-attempt resources are allocated), skipping the first attempt; uses TryParse with a Length > 0 guard instead of Parse for defensive handling
• Resources: added description and invalid-argument error strings; description now includes a bare-millisecond example (200) alongside unit-suffixed examples
• Tests: coverage for valid/invalid timespan values (including whitespace-only and out-of-range inputs), the dependency-on-retry-option validation, and an integration test (RetryFailedTests_WithDelay_StripsDelayFromChildArgs) that verifies the option is correctly stripped from child-process arguments end-to-end

Usage

dotnet test -- --retry-failed-tests 3 --retry-failed-tests-delay 2s

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a configurable delay between retry attempts for the retry-failed-tests extension to reduce flakiness for external-system-dependent tests.

Changes:

• Introduces --retry-failed-tests-delay CLI option with unit-aware timespan parsing and validation.
• Applies an inter-attempt Task.Delay in RetryOrchestrator and strips the option from forwarded child-process args.
• Updates help text, package docs, resources, and adds unit tests for option validation.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs	Adds unit tests for delay option parsing/validation and dependency on --retry-failed-tests.
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopTestSourceHostTests.cs	Adjusts nullability usage (null!) in mocks.
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs	Updates expected help output to include the new delay option.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs	Removes delay arg from forwarded args; parses delay option and delays between attempts.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryCommandLineOptionsProvider.cs	Registers the new option and validates dependency + timespan format.
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hant.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hans.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.tr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ru.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pt-BR.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pl.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ko.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ja.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.it.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.fr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.es.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.de.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.cs.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/ExtensionResources.resx	Adds delay option description and invalid-argument error string.
src/Platform/Microsoft.Testing.Extensions.Retry/PACKAGE.md	Documents the new delay feature and CLI option usage.

Copilot's findings

• Files reviewed: 20/20 changed files
• Comments generated: 4

[Amaury Levé (Evangelink)]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

1. [Critical — will cause test failure]HelpInfoAllExtensionsTests.cs: The --help output test was updated for --retry-failed-tests-delay, but the Info_WithAllExtensionsRegistered_OutputFullInfoContent test was not. Because AssertOutputMatchesLines performs sequential line-by-line matching, the test will fail in CI — the actual --info output now includes the delay option under RetryCommandLineOptionsProvider, but the expected pattern jumps straight to TerminalTestReporterCommandLineOptionsProvider. Four lines need to be inserted in the pattern.

2. [Minor — coverage gap]RetryTests.cs (IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption): The whitespace-only input (" ") takes a distinct early-return path in TimeSpanParser.TryParse but is not included in the [DataRow] set.

3. [Moderate — missing behavioral coverage] Neither the new unit tests nor the existing RetryFailedTestsTests.cs integration tests verify the actual delay behaviour in RetryOrchestrator: that Task.Delay is applied between retry attempts (not before the first), and that --retry-failed-tests-delay is stripped from the child-process argument list.

Recommendations

• Add the --retry-failed-tests-delay block to the --info expected-output string in HelpInfoAllExtensionsTests.cs (around line 342, after the --retry-failed-tests-max-tests entry).
• Add [DataRow(" ")] to IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption.
• Add an acceptance or unit test that exercises the orchestrator's delay and arg-stripping logic end-to-end.

---

Generated by Test Expert Reviewer

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Amaury Levé (Evangelink)]

Summary

Workflow: Expert Code Reviewer
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

[Defensive] TimeSpanParser.Parse in orchestrator path (RetryOrchestrator.cs:158)
The orchestrator calls TimeSpanParser.Parse (which throws FormatException) rather than the TryParse pattern used elsewhere in the same method. No bounds check on retryDelayArgs before [0] access either, inconsistent with how cmdRetries is guarded at line 98. Low practical risk since validation runs first, but adds a latent uncaught-exception path.

[Resources] Pipe server created before delay fires (RetryOrchestrator.cs:181 / 282)
RetryFailedTestsPipeServer (OS named-pipe listener) is created ~100 lines before the Task.Delay fires. For large delays (minutes/hours), the pipe holds OS resources for the entire wait period before the child process is launched. Moving the delay to the top of the loop body (immediately after attemptCount++) would scope the pipe server to only the window where it's needed.

Positive Observations

• Correctness of the retry-delay logic is sound: delay correctly skips attempt 1 (attemptCount > 1), fires before each subsequent attempt, and applies ConfigureAwait(false) throughout.
• Cancellation token is correctly threaded into Task.Delay — cancellation during the wait will propagate as OperationCanceledException, which is the expected behavior for this framework.
• Argument stripping (indexToCleanup) correctly removes both the option flag and its value, consistent with all other stripped options.
• Validation in RetryCommandLineOptionsProvider is complete: dependency on --retry-failed-tests, format validation via TryParse, and test coverage for valid/invalid inputs.
• TimeSpanParser.TryParse correctly rejects empty strings and invalid formats per the new unit tests.

Overall Assessment

The implementation is logically correct and well-structured. Both findings are minor defensive/resource concerns rather than correctness bugs. No threading issues, security vulnerabilities, or public API breaking changes were identified.

---

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot's findings

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

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

The open review comment about the resource string was addressed in commit 9b43a08 — the error message now mentions the supported range: "requires a valid time span value that is non-negative and no greater than 2147483647ms (~24.20:31:23.647) (e.g. 200, 1s, 2.5m, 1h)". All 14 XLF localization files were updated in the same commit.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

The retry extension had no way to introduce a wait between retry attempts, making it less useful for tests interacting with external systems (message queues, databases, etc.) where a cooldown period reduces flakiness.

Changes

• New CLI option --retry-failed-tests-delay: accepts a timespan value using the same unit-aware format as --hangdump-timeout (e.g. 200, 1s, 2.5m, 1h; default unit is milliseconds)
• RetryCommandLineOptionsProvider: registers the option, validates it requires --retry-failed-tests, validates the argument via TimeSpanParser.TryParse, and enforces a valid range (non-negative, no greater than int.MaxValue milliseconds — the Task.Delay maximum)
• RetryOrchestrator: strips the delay arg from arguments forwarded to child processes (with a bounds check before adding argIndex + 1 to the cleanup list); applies Task.Delay at the top of the retry loop (after attemptCount++, before any per-attempt resources are allocated), skipping the first attempt; uses TryParse with a Length > 0 guard instead of Parse for defensive handling
• Resources: added description and invalid-argument error strings; description now includes a bare-millisecond example (200) alongside unit-suffixed examples
• Tests: coverage for valid/invalid timespan values (including whitespace-only and out-of-range inputs), the dependency-on-retry-option validation, and an integration test (RetryFailedTests_WithDelay_StripsDelayFromChildArgs) that verifies the option is correctly stripped from child-process arguments end-to-end

Usage

dotnet test -- --retry-failed-tests 3 --retry-failed-tests-delay 2s

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a configurable delay between retry attempts for the retry-failed-tests extension to reduce flakiness for external-system-dependent tests.

Changes:

• Introduces --retry-failed-tests-delay CLI option with unit-aware timespan parsing and validation.
• Applies an inter-attempt Task.Delay in RetryOrchestrator and strips the option from forwarded child-process args.
• Updates help text, package docs, resources, and adds unit tests for option validation.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs	Adds unit tests for delay option parsing/validation and dependency on --retry-failed-tests.
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopTestSourceHostTests.cs	Adjusts nullability usage (null!) in mocks.
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs	Updates expected help output to include the new delay option.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs	Removes delay arg from forwarded args; parses delay option and delays between attempts.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryCommandLineOptionsProvider.cs	Registers the new option and validates dependency + timespan format.
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hant.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hans.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.tr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ru.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pt-BR.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pl.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ko.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ja.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.it.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.fr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.es.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.de.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.cs.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/ExtensionResources.resx	Adds delay option description and invalid-argument error string.
src/Platform/Microsoft.Testing.Extensions.Retry/PACKAGE.md	Documents the new delay feature and CLI option usage.

Copilot's findings

• Files reviewed: 20/20 changed files
• Comments generated: 4

[Amaury Levé (Evangelink)]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

1. [Critical — will cause test failure]HelpInfoAllExtensionsTests.cs: The --help output test was updated for --retry-failed-tests-delay, but the Info_WithAllExtensionsRegistered_OutputFullInfoContent test was not. Because AssertOutputMatchesLines performs sequential line-by-line matching, the test will fail in CI — the actual --info output now includes the delay option under RetryCommandLineOptionsProvider, but the expected pattern jumps straight to TerminalTestReporterCommandLineOptionsProvider. Four lines need to be inserted in the pattern.

2. [Minor — coverage gap]RetryTests.cs (IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption): The whitespace-only input (" ") takes a distinct early-return path in TimeSpanParser.TryParse but is not included in the [DataRow] set.

3. [Moderate — missing behavioral coverage] Neither the new unit tests nor the existing RetryFailedTestsTests.cs integration tests verify the actual delay behaviour in RetryOrchestrator: that Task.Delay is applied between retry attempts (not before the first), and that --retry-failed-tests-delay is stripped from the child-process argument list.

Recommendations

• Add the --retry-failed-tests-delay block to the --info expected-output string in HelpInfoAllExtensionsTests.cs (around line 342, after the --retry-failed-tests-max-tests entry).
• Add [DataRow(" ")] to IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption.
• Add an acceptance or unit test that exercises the orchestrator's delay and arg-stripping logic end-to-end.

---

Generated by Test Expert Reviewer

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Amaury Levé (Evangelink)]

Summary

Workflow: Expert Code Reviewer
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

[Defensive] TimeSpanParser.Parse in orchestrator path (RetryOrchestrator.cs:158)
The orchestrator calls TimeSpanParser.Parse (which throws FormatException) rather than the TryParse pattern used elsewhere in the same method. No bounds check on retryDelayArgs before [0] access either, inconsistent with how cmdRetries is guarded at line 98. Low practical risk since validation runs first, but adds a latent uncaught-exception path.

[Resources] Pipe server created before delay fires (RetryOrchestrator.cs:181 / 282)
RetryFailedTestsPipeServer (OS named-pipe listener) is created ~100 lines before the Task.Delay fires. For large delays (minutes/hours), the pipe holds OS resources for the entire wait period before the child process is launched. Moving the delay to the top of the loop body (immediately after attemptCount++) would scope the pipe server to only the window where it's needed.

Positive Observations

• Correctness of the retry-delay logic is sound: delay correctly skips attempt 1 (attemptCount > 1), fires before each subsequent attempt, and applies ConfigureAwait(false) throughout.
• Cancellation token is correctly threaded into Task.Delay — cancellation during the wait will propagate as OperationCanceledException, which is the expected behavior for this framework.
• Argument stripping (indexToCleanup) correctly removes both the option flag and its value, consistent with all other stripped options.
• Validation in RetryCommandLineOptionsProvider is complete: dependency on --retry-failed-tests, format validation via TryParse, and test coverage for valid/invalid inputs.
• TimeSpanParser.TryParse correctly rejects empty strings and invalid formats per the new unit tests.

Overall Assessment

The implementation is logically correct and well-structured. Both findings are minor defensive/resource concerns rather than correctness bugs. No threading issues, security vulnerabilities, or public API breaking changes were identified.

---

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot's findings

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

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

The open review comment about the resource string was addressed in commit 9b43a08 — the error message now mentions the supported range: "requires a valid time span value that is non-negative and no greater than 2147483647ms (~24.20:31:23.647) (e.g. 200, 1s, 2.5m, 1h)". All 14 XLF localization files were updated in the same commit.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

The retry extension had no way to introduce a wait between retry attempts, making it less useful for tests interacting with external systems (message queues, databases, etc.) where a cooldown period reduces flakiness.

Changes

• New CLI option --retry-failed-tests-delay: accepts a timespan value using the same unit-aware format as --hangdump-timeout (e.g. 200, 1s, 2.5m, 1h; default unit is milliseconds)
• RetryCommandLineOptionsProvider: registers the option, validates it requires --retry-failed-tests, validates the argument via TimeSpanParser.TryParse, and enforces a valid range (non-negative, no greater than int.MaxValue milliseconds — the Task.Delay maximum)
• RetryOrchestrator: strips the delay arg from arguments forwarded to child processes (with a bounds check before adding argIndex + 1 to the cleanup list); applies Task.Delay at the top of the retry loop (after attemptCount++, before any per-attempt resources are allocated), skipping the first attempt; uses TryParse with a Length > 0 guard instead of Parse for defensive handling
• Resources: added description and invalid-argument error strings; description now includes a bare-millisecond example (200) alongside unit-suffixed examples
• Tests: coverage for valid/invalid timespan values (including whitespace-only and out-of-range inputs), the dependency-on-retry-option validation, and an integration test (RetryFailedTests_WithDelay_StripsDelayFromChildArgs) that verifies the option is correctly stripped from child-process arguments end-to-end

Usage

dotnet test -- --retry-failed-tests 3 --retry-failed-tests-delay 2s

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a configurable delay between retry attempts for the retry-failed-tests extension to reduce flakiness for external-system-dependent tests.

Changes:

• Introduces --retry-failed-tests-delay CLI option with unit-aware timespan parsing and validation.
• Applies an inter-attempt Task.Delay in RetryOrchestrator and strips the option from forwarded child-process args.
• Updates help text, package docs, resources, and adds unit tests for option validation.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs	Adds unit tests for delay option parsing/validation and dependency on --retry-failed-tests.
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopTestSourceHostTests.cs	Adjusts nullability usage (null!) in mocks.
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs	Updates expected help output to include the new delay option.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs	Removes delay arg from forwarded args; parses delay option and delays between attempts.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryCommandLineOptionsProvider.cs	Registers the new option and validates dependency + timespan format.
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hant.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hans.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.tr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ru.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pt-BR.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pl.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ko.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ja.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.it.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.fr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.es.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.de.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.cs.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/ExtensionResources.resx	Adds delay option description and invalid-argument error string.
src/Platform/Microsoft.Testing.Extensions.Retry/PACKAGE.md	Documents the new delay feature and CLI option usage.

Copilot's findings

• Files reviewed: 20/20 changed files
• Comments generated: 4

[Amaury Levé (Evangelink)]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

1. [Critical — will cause test failure]HelpInfoAllExtensionsTests.cs: The --help output test was updated for --retry-failed-tests-delay, but the Info_WithAllExtensionsRegistered_OutputFullInfoContent test was not. Because AssertOutputMatchesLines performs sequential line-by-line matching, the test will fail in CI — the actual --info output now includes the delay option under RetryCommandLineOptionsProvider, but the expected pattern jumps straight to TerminalTestReporterCommandLineOptionsProvider. Four lines need to be inserted in the pattern.

2. [Minor — coverage gap]RetryTests.cs (IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption): The whitespace-only input (" ") takes a distinct early-return path in TimeSpanParser.TryParse but is not included in the [DataRow] set.

3. [Moderate — missing behavioral coverage] Neither the new unit tests nor the existing RetryFailedTestsTests.cs integration tests verify the actual delay behaviour in RetryOrchestrator: that Task.Delay is applied between retry attempts (not before the first), and that --retry-failed-tests-delay is stripped from the child-process argument list.

Recommendations

• Add the --retry-failed-tests-delay block to the --info expected-output string in HelpInfoAllExtensionsTests.cs (around line 342, after the --retry-failed-tests-max-tests entry).
• Add [DataRow(" ")] to IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption.
• Add an acceptance or unit test that exercises the orchestrator's delay and arg-stripping logic end-to-end.

---

Generated by Test Expert Reviewer

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Amaury Levé (Evangelink)]

Summary

Workflow: Expert Code Reviewer
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

[Defensive] TimeSpanParser.Parse in orchestrator path (RetryOrchestrator.cs:158)
The orchestrator calls TimeSpanParser.Parse (which throws FormatException) rather than the TryParse pattern used elsewhere in the same method. No bounds check on retryDelayArgs before [0] access either, inconsistent with how cmdRetries is guarded at line 98. Low practical risk since validation runs first, but adds a latent uncaught-exception path.

[Resources] Pipe server created before delay fires (RetryOrchestrator.cs:181 / 282)
RetryFailedTestsPipeServer (OS named-pipe listener) is created ~100 lines before the Task.Delay fires. For large delays (minutes/hours), the pipe holds OS resources for the entire wait period before the child process is launched. Moving the delay to the top of the loop body (immediately after attemptCount++) would scope the pipe server to only the window where it's needed.

Positive Observations

• Correctness of the retry-delay logic is sound: delay correctly skips attempt 1 (attemptCount > 1), fires before each subsequent attempt, and applies ConfigureAwait(false) throughout.
• Cancellation token is correctly threaded into Task.Delay — cancellation during the wait will propagate as OperationCanceledException, which is the expected behavior for this framework.
• Argument stripping (indexToCleanup) correctly removes both the option flag and its value, consistent with all other stripped options.
• Validation in RetryCommandLineOptionsProvider is complete: dependency on --retry-failed-tests, format validation via TryParse, and test coverage for valid/invalid inputs.
• TimeSpanParser.TryParse correctly rejects empty strings and invalid formats per the new unit tests.

Overall Assessment

The implementation is logically correct and well-structured. Both findings are minor defensive/resource concerns rather than correctness bugs. No threading issues, security vulnerabilities, or public API breaking changes were identified.

---

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot's findings

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

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

The open review comment about the resource string was addressed in commit 9b43a08 — the error message now mentions the supported range: "requires a valid time span value that is non-negative and no greater than 2147483647ms (~24.20:31:23.647) (e.g. 200, 1s, 2.5m, 1h)". All 14 XLF localization files were updated in the same commit.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

The retry extension had no way to introduce a wait between retry attempts, making it less useful for tests interacting with external systems (message queues, databases, etc.) where a cooldown period reduces flakiness.

Changes

• New CLI option --retry-failed-tests-delay: accepts a timespan value using the same unit-aware format as --hangdump-timeout (e.g. 200, 1s, 2.5m, 1h; default unit is milliseconds)
• RetryCommandLineOptionsProvider: registers the option, validates it requires --retry-failed-tests, validates the argument via TimeSpanParser.TryParse, and enforces a valid range (non-negative, no greater than int.MaxValue milliseconds — the Task.Delay maximum)
• RetryOrchestrator: strips the delay arg from arguments forwarded to child processes (with a bounds check before adding argIndex + 1 to the cleanup list); applies Task.Delay at the top of the retry loop (after attemptCount++, before any per-attempt resources are allocated), skipping the first attempt; uses TryParse with a Length > 0 guard instead of Parse for defensive handling
• Resources: added description and invalid-argument error strings; description now includes a bare-millisecond example (200) alongside unit-suffixed examples
• Tests: coverage for valid/invalid timespan values (including whitespace-only and out-of-range inputs), the dependency-on-retry-option validation, and an integration test (RetryFailedTests_WithDelay_StripsDelayFromChildArgs) that verifies the option is correctly stripped from child-process arguments end-to-end

Usage

dotnet test -- --retry-failed-tests 3 --retry-failed-tests-delay 2s

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a configurable delay between retry attempts for the retry-failed-tests extension to reduce flakiness for external-system-dependent tests.

Changes:

• Introduces --retry-failed-tests-delay CLI option with unit-aware timespan parsing and validation.
• Applies an inter-attempt Task.Delay in RetryOrchestrator and strips the option from forwarded child-process args.
• Updates help text, package docs, resources, and adds unit tests for option validation.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs	Adds unit tests for delay option parsing/validation and dependency on --retry-failed-tests.
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopTestSourceHostTests.cs	Adjusts nullability usage (null!) in mocks.
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs	Updates expected help output to include the new delay option.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs	Removes delay arg from forwarded args; parses delay option and delays between attempts.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryCommandLineOptionsProvider.cs	Registers the new option and validates dependency + timespan format.
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hant.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hans.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.tr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ru.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pt-BR.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pl.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ko.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ja.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.it.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.fr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.es.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.de.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.cs.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/ExtensionResources.resx	Adds delay option description and invalid-argument error string.
src/Platform/Microsoft.Testing.Extensions.Retry/PACKAGE.md	Documents the new delay feature and CLI option usage.

Copilot's findings

• Files reviewed: 20/20 changed files
• Comments generated: 4

[Amaury Levé (Evangelink)]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

1. [Critical — will cause test failure]HelpInfoAllExtensionsTests.cs: The --help output test was updated for --retry-failed-tests-delay, but the Info_WithAllExtensionsRegistered_OutputFullInfoContent test was not. Because AssertOutputMatchesLines performs sequential line-by-line matching, the test will fail in CI — the actual --info output now includes the delay option under RetryCommandLineOptionsProvider, but the expected pattern jumps straight to TerminalTestReporterCommandLineOptionsProvider. Four lines need to be inserted in the pattern.

2. [Minor — coverage gap]RetryTests.cs (IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption): The whitespace-only input (" ") takes a distinct early-return path in TimeSpanParser.TryParse but is not included in the [DataRow] set.

3. [Moderate — missing behavioral coverage] Neither the new unit tests nor the existing RetryFailedTestsTests.cs integration tests verify the actual delay behaviour in RetryOrchestrator: that Task.Delay is applied between retry attempts (not before the first), and that --retry-failed-tests-delay is stripped from the child-process argument list.

Recommendations

• Add the --retry-failed-tests-delay block to the --info expected-output string in HelpInfoAllExtensionsTests.cs (around line 342, after the --retry-failed-tests-max-tests entry).
• Add [DataRow(" ")] to IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption.
• Add an acceptance or unit test that exercises the orchestrator's delay and arg-stripping logic end-to-end.

---

Generated by Test Expert Reviewer

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Amaury Levé (Evangelink)]

Summary

Workflow: Expert Code Reviewer
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

[Defensive] TimeSpanParser.Parse in orchestrator path (RetryOrchestrator.cs:158)
The orchestrator calls TimeSpanParser.Parse (which throws FormatException) rather than the TryParse pattern used elsewhere in the same method. No bounds check on retryDelayArgs before [0] access either, inconsistent with how cmdRetries is guarded at line 98. Low practical risk since validation runs first, but adds a latent uncaught-exception path.

[Resources] Pipe server created before delay fires (RetryOrchestrator.cs:181 / 282)
RetryFailedTestsPipeServer (OS named-pipe listener) is created ~100 lines before the Task.Delay fires. For large delays (minutes/hours), the pipe holds OS resources for the entire wait period before the child process is launched. Moving the delay to the top of the loop body (immediately after attemptCount++) would scope the pipe server to only the window where it's needed.

Positive Observations

• Correctness of the retry-delay logic is sound: delay correctly skips attempt 1 (attemptCount > 1), fires before each subsequent attempt, and applies ConfigureAwait(false) throughout.
• Cancellation token is correctly threaded into Task.Delay — cancellation during the wait will propagate as OperationCanceledException, which is the expected behavior for this framework.
• Argument stripping (indexToCleanup) correctly removes both the option flag and its value, consistent with all other stripped options.
• Validation in RetryCommandLineOptionsProvider is complete: dependency on --retry-failed-tests, format validation via TryParse, and test coverage for valid/invalid inputs.
• TimeSpanParser.TryParse correctly rejects empty strings and invalid formats per the new unit tests.

Overall Assessment

The implementation is logically correct and well-structured. Both findings are minor defensive/resource concerns rather than correctness bugs. No threading issues, security vulnerabilities, or public API breaking changes were identified.

---

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot's findings

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

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

The open review comment about the resource string was addressed in commit 9b43a08 — the error message now mentions the supported range: "requires a valid time span value that is non-negative and no greater than 2147483647ms (~24.20:31:23.647) (e.g. 200, 1s, 2.5m, 1h)". All 14 XLF localization files were updated in the same commit.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

The retry extension had no way to introduce a wait between retry attempts, making it less useful for tests interacting with external systems (message queues, databases, etc.) where a cooldown period reduces flakiness.

Changes

• New CLI option --retry-failed-tests-delay: accepts a timespan value using the same unit-aware format as --hangdump-timeout (e.g. 200, 1s, 2.5m, 1h; default unit is milliseconds)
• RetryCommandLineOptionsProvider: registers the option, validates it requires --retry-failed-tests, validates the argument via TimeSpanParser.TryParse, and enforces a valid range (non-negative, no greater than int.MaxValue milliseconds — the Task.Delay maximum)
• RetryOrchestrator: strips the delay arg from arguments forwarded to child processes (with a bounds check before adding argIndex + 1 to the cleanup list); applies Task.Delay at the top of the retry loop (after attemptCount++, before any per-attempt resources are allocated), skipping the first attempt; uses TryParse with a Length > 0 guard instead of Parse for defensive handling
• Resources: added description and invalid-argument error strings; description now includes a bare-millisecond example (200) alongside unit-suffixed examples
• Tests: coverage for valid/invalid timespan values (including whitespace-only and out-of-range inputs), the dependency-on-retry-option validation, and an integration test (RetryFailedTests_WithDelay_StripsDelayFromChildArgs) that verifies the option is correctly stripped from child-process arguments end-to-end

Usage

dotnet test -- --retry-failed-tests 3 --retry-failed-tests-delay 2s

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a configurable delay between retry attempts for the retry-failed-tests extension to reduce flakiness for external-system-dependent tests.

Changes:

• Introduces --retry-failed-tests-delay CLI option with unit-aware timespan parsing and validation.
• Applies an inter-attempt Task.Delay in RetryOrchestrator and strips the option from forwarded child-process args.
• Updates help text, package docs, resources, and adds unit tests for option validation.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs	Adds unit tests for delay option parsing/validation and dependency on --retry-failed-tests.
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopTestSourceHostTests.cs	Adjusts nullability usage (null!) in mocks.
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs	Updates expected help output to include the new delay option.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs	Removes delay arg from forwarded args; parses delay option and delays between attempts.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryCommandLineOptionsProvider.cs	Registers the new option and validates dependency + timespan format.
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hant.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hans.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.tr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ru.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pt-BR.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pl.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ko.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ja.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.it.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.fr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.es.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.de.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.cs.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/ExtensionResources.resx	Adds delay option description and invalid-argument error string.
src/Platform/Microsoft.Testing.Extensions.Retry/PACKAGE.md	Documents the new delay feature and CLI option usage.

Copilot's findings

• Files reviewed: 20/20 changed files
• Comments generated: 4

[Amaury Levé (Evangelink)]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

1. [Critical — will cause test failure]HelpInfoAllExtensionsTests.cs: The --help output test was updated for --retry-failed-tests-delay, but the Info_WithAllExtensionsRegistered_OutputFullInfoContent test was not. Because AssertOutputMatchesLines performs sequential line-by-line matching, the test will fail in CI — the actual --info output now includes the delay option under RetryCommandLineOptionsProvider, but the expected pattern jumps straight to TerminalTestReporterCommandLineOptionsProvider. Four lines need to be inserted in the pattern.

2. [Minor — coverage gap]RetryTests.cs (IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption): The whitespace-only input (" ") takes a distinct early-return path in TimeSpanParser.TryParse but is not included in the [DataRow] set.

3. [Moderate — missing behavioral coverage] Neither the new unit tests nor the existing RetryFailedTestsTests.cs integration tests verify the actual delay behaviour in RetryOrchestrator: that Task.Delay is applied between retry attempts (not before the first), and that --retry-failed-tests-delay is stripped from the child-process argument list.

Recommendations

• Add the --retry-failed-tests-delay block to the --info expected-output string in HelpInfoAllExtensionsTests.cs (around line 342, after the --retry-failed-tests-max-tests entry).
• Add [DataRow(" ")] to IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption.
• Add an acceptance or unit test that exercises the orchestrator's delay and arg-stripping logic end-to-end.

---

Generated by Test Expert Reviewer

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Amaury Levé (Evangelink)]

Summary

Workflow: Expert Code Reviewer
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

[Defensive] TimeSpanParser.Parse in orchestrator path (RetryOrchestrator.cs:158)
The orchestrator calls TimeSpanParser.Parse (which throws FormatException) rather than the TryParse pattern used elsewhere in the same method. No bounds check on retryDelayArgs before [0] access either, inconsistent with how cmdRetries is guarded at line 98. Low practical risk since validation runs first, but adds a latent uncaught-exception path.

[Resources] Pipe server created before delay fires (RetryOrchestrator.cs:181 / 282)
RetryFailedTestsPipeServer (OS named-pipe listener) is created ~100 lines before the Task.Delay fires. For large delays (minutes/hours), the pipe holds OS resources for the entire wait period before the child process is launched. Moving the delay to the top of the loop body (immediately after attemptCount++) would scope the pipe server to only the window where it's needed.

Positive Observations

• Correctness of the retry-delay logic is sound: delay correctly skips attempt 1 (attemptCount > 1), fires before each subsequent attempt, and applies ConfigureAwait(false) throughout.
• Cancellation token is correctly threaded into Task.Delay — cancellation during the wait will propagate as OperationCanceledException, which is the expected behavior for this framework.
• Argument stripping (indexToCleanup) correctly removes both the option flag and its value, consistent with all other stripped options.
• Validation in RetryCommandLineOptionsProvider is complete: dependency on --retry-failed-tests, format validation via TryParse, and test coverage for valid/invalid inputs.
• TimeSpanParser.TryParse correctly rejects empty strings and invalid formats per the new unit tests.

Overall Assessment

The implementation is logically correct and well-structured. Both findings are minor defensive/resource concerns rather than correctness bugs. No threading issues, security vulnerabilities, or public API breaking changes were identified.

---

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot's findings

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

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

The open review comment about the resource string was addressed in commit 9b43a08 — the error message now mentions the supported range: "requires a valid time span value that is non-negative and no greater than 2147483647ms (~24.20:31:23.647) (e.g. 200, 1s, 2.5m, 1h)". All 14 XLF localization files were updated in the same commit.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

The retry extension had no way to introduce a wait between retry attempts, making it less useful for tests interacting with external systems (message queues, databases, etc.) where a cooldown period reduces flakiness.

Changes

• New CLI option --retry-failed-tests-delay: accepts a timespan value using the same unit-aware format as --hangdump-timeout (e.g. 200, 1s, 2.5m, 1h; default unit is milliseconds)
• RetryCommandLineOptionsProvider: registers the option, validates it requires --retry-failed-tests, validates the argument via TimeSpanParser.TryParse, and enforces a valid range (non-negative, no greater than int.MaxValue milliseconds — the Task.Delay maximum)
• RetryOrchestrator: strips the delay arg from arguments forwarded to child processes (with a bounds check before adding argIndex + 1 to the cleanup list); applies Task.Delay at the top of the retry loop (after attemptCount++, before any per-attempt resources are allocated), skipping the first attempt; uses TryParse with a Length > 0 guard instead of Parse for defensive handling
• Resources: added description and invalid-argument error strings; description now includes a bare-millisecond example (200) alongside unit-suffixed examples
• Tests: coverage for valid/invalid timespan values (including whitespace-only and out-of-range inputs), the dependency-on-retry-option validation, and an integration test (RetryFailedTests_WithDelay_StripsDelayFromChildArgs) that verifies the option is correctly stripped from child-process arguments end-to-end

Usage

dotnet test -- --retry-failed-tests 3 --retry-failed-tests-delay 2s

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a configurable delay between retry attempts for the retry-failed-tests extension to reduce flakiness for external-system-dependent tests.

Changes:

• Introduces --retry-failed-tests-delay CLI option with unit-aware timespan parsing and validation.
• Applies an inter-attempt Task.Delay in RetryOrchestrator and strips the option from forwarded child-process args.
• Updates help text, package docs, resources, and adds unit tests for option validation.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs	Adds unit tests for delay option parsing/validation and dependency on --retry-failed-tests.
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopTestSourceHostTests.cs	Adjusts nullability usage (null!) in mocks.
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs	Updates expected help output to include the new delay option.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs	Removes delay arg from forwarded args; parses delay option and delays between attempts.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryCommandLineOptionsProvider.cs	Registers the new option and validates dependency + timespan format.
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hant.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.zh-Hans.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.tr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ru.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pt-BR.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.pl.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ko.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.ja.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.it.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.fr.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.es.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.de.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/xlf/ExtensionResources.cs.xlf	Adds new resource strings for delay option description/error (untranslated “new” entries).
src/Platform/Microsoft.Testing.Extensions.Retry/Resources/ExtensionResources.resx	Adds delay option description and invalid-argument error string.
src/Platform/Microsoft.Testing.Extensions.Retry/PACKAGE.md	Documents the new delay feature and CLI option usage.

Copilot's findings

• Files reviewed: 20/20 changed files
• Comments generated: 4

[Amaury Levé (Evangelink)]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

1. [Critical — will cause test failure]HelpInfoAllExtensionsTests.cs: The --help output test was updated for --retry-failed-tests-delay, but the Info_WithAllExtensionsRegistered_OutputFullInfoContent test was not. Because AssertOutputMatchesLines performs sequential line-by-line matching, the test will fail in CI — the actual --info output now includes the delay option under RetryCommandLineOptionsProvider, but the expected pattern jumps straight to TerminalTestReporterCommandLineOptionsProvider. Four lines need to be inserted in the pattern.

2. [Minor — coverage gap]RetryTests.cs (IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption): The whitespace-only input (" ") takes a distinct early-return path in TimeSpanParser.TryParse but is not included in the [DataRow] set.

3. [Moderate — missing behavioral coverage] Neither the new unit tests nor the existing RetryFailedTestsTests.cs integration tests verify the actual delay behaviour in RetryOrchestrator: that Task.Delay is applied between retry attempts (not before the first), and that --retry-failed-tests-delay is stripped from the child-process argument list.

Recommendations

• Add the --retry-failed-tests-delay block to the --info expected-output string in HelpInfoAllExtensionsTests.cs (around line 342, after the --retry-failed-tests-max-tests entry).
• Add [DataRow(" ")] to IsInvalid_If_InvalidTimeSpan_Is_Provided_For_DelayOption.
• Add an acceptance or unit test that exercises the orchestrator's delay and arg-stripping logic end-to-end.

---

Generated by Test Expert Reviewer

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Amaury Levé (Evangelink)]

Summary

Workflow: Expert Code Reviewer
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

[Defensive] TimeSpanParser.Parse in orchestrator path (RetryOrchestrator.cs:158)
The orchestrator calls TimeSpanParser.Parse (which throws FormatException) rather than the TryParse pattern used elsewhere in the same method. No bounds check on retryDelayArgs before [0] access either, inconsistent with how cmdRetries is guarded at line 98. Low practical risk since validation runs first, but adds a latent uncaught-exception path.

[Resources] Pipe server created before delay fires (RetryOrchestrator.cs:181 / 282)
RetryFailedTestsPipeServer (OS named-pipe listener) is created ~100 lines before the Task.Delay fires. For large delays (minutes/hours), the pipe holds OS resources for the entire wait period before the child process is launched. Moving the delay to the top of the loop body (immediately after attemptCount++) would scope the pipe server to only the window where it's needed.

Positive Observations

• Correctness of the retry-delay logic is sound: delay correctly skips attempt 1 (attemptCount > 1), fires before each subsequent attempt, and applies ConfigureAwait(false) throughout.
• Cancellation token is correctly threaded into Task.Delay — cancellation during the wait will propagate as OperationCanceledException, which is the expected behavior for this framework.
• Argument stripping (indexToCleanup) correctly removes both the option flag and its value, consistent with all other stripped options.
• Validation in RetryCommandLineOptionsProvider is complete: dependency on --retry-failed-tests, format validation via TryParse, and test coverage for valid/invalid inputs.
• TimeSpanParser.TryParse correctly rejects empty strings and invalid formats per the new unit tests.

Overall Assessment

The implementation is logically correct and well-structured. Both findings are minor defensive/resource concerns rather than correctness bugs. No threading issues, security vulnerabilities, or public API breaking changes were identified.

---

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot's findings

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

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

The open review comment about the resource string was addressed in commit 9b43a08 — the error message now mentions the supported range: "requires a valid time span value that is non-negative and no greater than 2147483647ms (~24.20:31:23.647) (e.g. 200, 1s, 2.5m, 1h)". All 14 XLF localization files were updated in the same commit.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.