Simplify retry extension logic to use built-in filters#7511
Merged
Evangelink merged 8 commits intomainfrom Apr 24, 2026
Merged
Conversation
Youssef1313
reviewed
Mar 9, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR simplifies the Retry extension by removing the custom test execution filter factory and instead relying on the platform’s built-in --filter-uid functionality when re-running failed tests.
Changes:
- Update
RetryOrchestratorto remove any existing test filter options and apply--filter-uidwith the failed test UIDs on retry. - Remove the Retry extension’s custom
ITestExecutionFilterFactoryintegration (and the defaultTestHostManagerrequirement). - Add/adjust acceptance coverage to validate retry behavior when an initial filter is present.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/RetryFailedTestsTests.cs | Adds new acceptance scenarios for retry behavior with pre-existing filters and updates the embedded test asset to respect UID filtering. |
| src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs | Replaces the previous in-process filter approach by rewriting command line filters on retry and introduces helper logic to remove existing options. |
| src/Platform/Microsoft.Testing.Extensions.Retry/RetryExtensions.cs | Removes registration of the custom retry execution filter factory and the dependency on the default TestHostManager. |
| src/Platform/Microsoft.Testing.Extensions.Retry/RetryExecutionFilterFactory.cs | Deletes the custom filter factory implementation (no longer needed with built-in filtering). |
Youssef1313
reviewed
Mar 9, 2026
Youssef1313
reviewed
Mar 9, 2026
Youssef1313
reviewed
Mar 17, 2026
This was referenced Apr 1, 2026
Youssef1313
reviewed
Apr 9, 2026
…r-retry # Conflicts: # test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/RetryFailedTestsTests.cs
… netstandard2.0 - Write one UID per line in RSP file with quotes to handle whitespace/comments - Fix StartsWith(char) in RemoveOption to use [0] != '-' for netstandard2.0 compat
Youssef1313
reviewed
Apr 20, 2026
Youssef1313
reviewed
Apr 20, 2026
Contributor
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs:220
- The response file created for long UID lists is never cleaned up. Since it contains test UIDs (potentially sensitive) and is written under the results directory, consider deleting it after the retry attempt (or after the process has started), and/or placing it in a more appropriate temp location with restricted permissions to avoid leaving artifacts behind.
// Use a response file to avoid exceeding command-line length limits.
// Write to retryRootFolder (not the per-attempt folder) so it won't be included
// in the final results move.
string responseFilePath = Path.Combine(retryRootFolder, $"retry-filter-uids-{attemptCount}.rsp");
using (IFileStream stream = _fileSystem.NewFileStream(responseFilePath, FileMode.Create, FileAccess.Write))
using (var writer = new StreamWriter(stream.Stream))
{
// Write one UID per line. The RSP parser (ResponseFileHelper.SplitCommandLine) splits
// each line by whitespace and uses '"' for grouping. Wrapping each UID in quotes
// handles UIDs containing whitespace or starting with '#' (comment marker).
// Note: UIDs containing literal '"' characters cannot be safely round-tripped
// through the RSP parser because it strips all quote characters from tokens.
await writer.WriteLineAsync($"--{PlatformCommandLineProvider.FilterUidOptionKey}").ConfigureAwait(false);
foreach (string uid in lastListOfFailedId)
{
await writer.WriteLineAsync($"\"{uid}\"").ConfigureAwait(false);
}
}
finalArguments.Add($"@{responseFilePath}");
}
- Files reviewed: 18/18 changed files
- Comments generated: 2
…imation - UIDs with literal '"' characters now skip RSP and use inline args, since ResponseFileHelper.SplitCommandLine strips all '"' from tokens. - Adjusted command-line length estimation to account for PasteArguments quoting overhead on pre-.NET 8 targets (+3 per argument). - Simplified RSP format: write all UIDs on a single line instead of one per line, which is clearer and equivalent for the RSP parser.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7500