From b34f32abf9dd608b28895bd353215f294d0fe67b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 3 Sep 2026 12:54:08 +0200 Subject: [PATCH 1/3] Address retry response file review feedback Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../InternalAPI/InternalAPI.Unshipped.txt | 2 + .../RetryArgumentsBuilder.cs | 13 +++- .../RetryOrchestrator.cs | 12 ++- .../CommandLine/ResponseFileHelper.cs | 74 +++++++++++++----- .../RetryFailedTestsTests.cs | 3 + .../RetryArgumentsBuilderTests.cs | 75 +++++++++++++++++++ .../CommandLine/ResponseFileHelperTests.cs | 53 +++++++++++++ 7 files changed, 208 insertions(+), 24 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt b/src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt index 81a34b7ee1..ffa48213c9 100644 --- a/src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt +++ b/src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt @@ -1,2 +1,4 @@ #nullable enable static Microsoft.Testing.Extensions.Policy.RetryArgumentsBuilder.BuildAttemptArgumentsAsync(Microsoft.Testing.Platform.Helpers.IFileSystem! fileSystem, string![]! executableArguments, string![]! originalExecutableArguments, System.Collections.Generic.List! indexToCleanup, string! currentTryResultFolder, string! retryRootFolder, string! pipeName, string![]? lastListOfFailedId, int attemptCount) -> System.Threading.Tasks.Task!>! +static Microsoft.Testing.Extensions.Policy.RetryArgumentsBuilder.GetArgumentsResponseFilePath(string! retryRootFolder, int attemptCount) -> string! +static Microsoft.Testing.Extensions.Policy.RetryArgumentsBuilder.GetFilterUidsResponseFilePath(string! retryRootFolder, int attemptCount) -> string! diff --git a/src/Platform/Microsoft.Testing.Extensions.Retry/RetryArgumentsBuilder.cs b/src/Platform/Microsoft.Testing.Extensions.Retry/RetryArgumentsBuilder.cs index 902522d6ba..9261d8477c 100644 --- a/src/Platform/Microsoft.Testing.Extensions.Retry/RetryArgumentsBuilder.cs +++ b/src/Platform/Microsoft.Testing.Extensions.Retry/RetryArgumentsBuilder.cs @@ -20,6 +20,12 @@ internal static class RetryArgumentsBuilder private const int CommandLineLengthLimit = 30_000; private const int PerArgumentOverhead = 3; + internal static string GetArgumentsResponseFilePath(string retryRootFolder, int attemptCount) + => Path.Combine(retryRootFolder, $"retry-arguments-{attemptCount}.rsp"); + + internal static string GetFilterUidsResponseFilePath(string retryRootFolder, int attemptCount) + => Path.Combine(retryRootFolder, $"retry-filter-uids-{attemptCount}.rsp"); + /// /// Computes the indices of the original executable arguments that must be dropped when restarting the test /// host, namely the retry-specific options and the result-directory option (which is re-injected per attempt). @@ -143,9 +149,10 @@ public static async Task> BuildAttemptArgumentsAsync( } } - if (directPrefixArguments is not null && !finalArguments.Any(argument => argument.IndexOf('"') >= 0)) + if (directPrefixArguments is not null + && !finalArguments.Skip(directPrefixArguments.Count).Any(argument => argument.IndexOf('"') >= 0)) { - string responseFilePath = Path.Combine(retryRootFolder, $"retry-arguments-{attemptCount}.rsp"); + string responseFilePath = GetArgumentsResponseFilePath(retryRootFolder, attemptCount); using (IFileStream stream = fileSystem.NewFileStream(responseFilePath, FileMode.Create, FileAccess.Write)) using (var writer = new StreamWriter(stream.Stream)) { @@ -203,7 +210,7 @@ public static async Task> BuildAttemptArgumentsAsync( // 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"); + string responseFilePath = GetFilterUidsResponseFilePath(retryRootFolder, attemptCount); using (IFileStream stream = fileSystem.NewFileStream(responseFilePath, FileMode.Create, FileAccess.Write)) using (var writer = new StreamWriter(stream.Stream)) { diff --git a/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs b/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs index 0f0d639958..240e7ee9c4 100644 --- a/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs +++ b/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs @@ -223,8 +223,8 @@ await outputDevice.DisplayAsync( RetryTestHostRunner.AttemptResult attemptResult; string[] generatedResponseFilePaths = [ - Path.Combine(retryRootFolder, $"retry-arguments-{attemptCount}.rsp"), - Path.Combine(retryRootFolder, $"retry-filter-uids-{attemptCount}.rsp"), + RetryArgumentsBuilder.GetArgumentsResponseFilePath(retryRootFolder, attemptCount), + RetryArgumentsBuilder.GetFilterUidsResponseFilePath(retryRootFolder, attemptCount), ]; try { @@ -239,6 +239,14 @@ await outputDevice.DisplayAsync( lastListOfFailedId, attemptCount).ConfigureAwait(false); + if (originalExecutableArguments.Any(argument => argument.StartsWith("@", StringComparison.Ordinal)) + && !finalArguments.Contains($"@{generatedResponseFilePaths[0]}")) + { + await logger.LogWarningAsync( + "Retry arguments could not be regenerated in a response file because an argument contains a literal double quote. " + + "The retry command line may exceed the operating system limit.").ConfigureAwait(false); + } + attemptResult = await RetryTestHostRunner.RunAttemptAsync( _serviceProvider, this, diff --git a/src/Platform/Microsoft.Testing.Platform/CommandLine/ResponseFileHelper.cs b/src/Platform/Microsoft.Testing.Platform/CommandLine/ResponseFileHelper.cs index 8530b3ebc7..9391f37d49 100644 --- a/src/Platform/Microsoft.Testing.Platform/CommandLine/ResponseFileHelper.cs +++ b/src/Platform/Microsoft.Testing.Platform/CommandLine/ResponseFileHelper.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using Microsoft.Testing.Platform.CommandLine; using Microsoft.Testing.Platform.Resources; // Most of the core logic is from: @@ -20,19 +21,22 @@ internal static bool TryReadResponseFile( ICollection errors, [NotNullWhen(true)] out string[]? newArguments) { + var readContext = new ResponseFileReadContext(rspFilePath, diagnosticPath); try { newArguments = [.. ExpandResponseFile( rspFilePath, + diagnosticPath, [with( RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? StringComparer.OrdinalIgnoreCase - : StringComparer.Ordinal)])]; + : StringComparer.Ordinal)], + readContext)]; return true; } catch (FileNotFoundException) { - errors.Add(string.Format(CultureInfo.InvariantCulture, PlatformResources.CommandLineParserResponseFileNotFound, diagnosticPath)); + errors.Add(string.Format(CultureInfo.InvariantCulture, PlatformResources.CommandLineParserResponseFileNotFound, readContext.DiagnosticPath)); } catch (Exception e) when (e is IOException or UnauthorizedAccessException) { @@ -40,8 +44,8 @@ internal static bool TryReadResponseFile( string.Format( CultureInfo.InvariantCulture, PlatformResources.CommandLineParserFailedToReadResponseFile, - diagnosticPath, - GetExceptionDetail(e, rspFilePath, diagnosticPath))); + readContext.DiagnosticPath, + GetExceptionDetail(e, readContext.ActualPath, readContext.DiagnosticPath))); } catch (FormatException e) { @@ -52,8 +56,8 @@ internal static bool TryReadResponseFile( string.Format( CultureInfo.InvariantCulture, PlatformResources.CommandLineParserFailedToReadResponseFile, - diagnosticPath, - GetExceptionDetail(e, rspFilePath, diagnosticPath))); + readContext.DiagnosticPath, + GetExceptionDetail(e, readContext.ActualPath, readContext.DiagnosticPath))); } newArguments = null; @@ -63,8 +67,13 @@ internal static bool TryReadResponseFile( static string GetExceptionDetail(Exception exception, string actualPath, string diagnosticPath) => actualPath == diagnosticPath ? exception.ToString() : exception.GetType().Name; - static IEnumerable ExpandResponseFile(string filePath, HashSet activeResponseFiles) + static IEnumerable ExpandResponseFile( + string filePath, + string diagnosticPath, + HashSet activeResponseFiles, + ResponseFileReadContext readContext) { + readContext.SetCurrentFile(filePath, diagnosticPath); string fullPath = Path.GetFullPath(filePath); if (!activeResponseFiles.Add(fullPath)) { @@ -74,24 +83,38 @@ static IEnumerable ExpandResponseFile(string filePath, HashSet a try { string[] lines = File.ReadAllLines(filePath); - + List arguments = []; for (int i = 0; i < lines.Length; i++) { - string line = lines[i]; + arguments.AddRange(SplitLine(lines[i], i + 1)); + } - foreach (string argument in SplitLine(line, i + 1)) + for (int argumentIndex = 0; argumentIndex < arguments.Count; argumentIndex++) + { + string argument = arguments[argumentIndex]; + if (argument.StartsWith("@", StringComparison.Ordinal)) { - if (argument.StartsWith("@", StringComparison.Ordinal)) - { - foreach (string nestedArgument in ExpandResponseFile(argument.Substring(1), activeResponseFiles)) - { - yield return nestedArgument; - } - } - else + string redactedNestedArgument = CommandLineArgumentsRedactor.RedactArgument([.. arguments], argumentIndex); + string nestedDiagnosticPath = redactedNestedArgument.StartsWith("@", StringComparison.Ordinal) + ? redactedNestedArgument[1..] + : redactedNestedArgument; + + // Nested response files intentionally use the process working directory, just like + // top-level response files, rather than the containing response file's directory. + foreach (string nestedArgument in ExpandResponseFile( + argument[1..], + nestedDiagnosticPath, + activeResponseFiles, + readContext)) { - yield return argument; + yield return nestedArgument; } + + readContext.SetCurrentFile(filePath, diagnosticPath); + } + else + { + yield return argument; } } } @@ -117,6 +140,19 @@ static IEnumerable SplitLine(string line, int lineNumber) } } + private sealed class ResponseFileReadContext(string actualPath, string diagnosticPath) + { + public string ActualPath { get; private set; } = actualPath; + + public string DiagnosticPath { get; private set; } = diagnosticPath; + + public void SetCurrentFile(string currentActualPath, string currentDiagnosticPath) + { + ActualPath = currentActualPath; + DiagnosticPath = currentDiagnosticPath; + } + } + private enum Boundary { TokenStart, diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/RetryFailedTestsTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/RetryFailedTestsTests.cs index d229320019..4d1b4c43ed 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/RetryFailedTestsTests.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/RetryFailedTestsTests.cs @@ -169,6 +169,9 @@ public async Task RetryFailedTests_WithArgumentsInResponseFile_Succeeds(bool ret testHostResult.AssertExitCodeIs(ExitCode.Success); testHostResult.AssertOutputContains("Retry summary: Passed! after 2/2 attempts"); + Assert.IsEmpty( + Directory.GetFiles(resultDirectory, "retry-*.rsp", SearchOption.AllDirectories), + "Generated retry response files must be deleted after each attempt."); } finally { diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryArgumentsBuilderTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryArgumentsBuilderTests.cs index 222017e330..4bfc9daece 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryArgumentsBuilderTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryArgumentsBuilderTests.cs @@ -231,6 +231,81 @@ public async Task BuildAttemptArgumentsAsync_WithOriginalResponseFile_WritesClea Encoding.UTF8.GetString(memoryStream.ToArray())); } + [TestMethod] + public async Task BuildAttemptArgumentsAsync_WithQuotedDirectPrefix_WritesSuffixToResponseFile() + { + string retryRoot = Path.Combine("results", "Retries", "run"); + string responseFilePath = Path.Combine(retryRoot, "retry-arguments-1.rsp"); + string[] executableArguments = ["exec", "quoted\"prefix", "--keep", "value"]; + using var memoryStream = new MemoryStream(); + var fileStream = new Mock(MockBehavior.Strict); + fileStream.SetupGet(stream => stream.Stream).Returns(memoryStream); + fileStream.Setup(stream => stream.Dispose()); + var fileSystem = new Mock(MockBehavior.Strict); + fileSystem + .Setup(fs => fs.NewFileStream(responseFilePath, FileMode.Create, FileAccess.Write)) + .Returns(fileStream.Object); + + List actual = await RetryArgumentsBuilder.BuildAttemptArgumentsAsync( + fileSystem.Object, + executableArguments, + ["exec", "quoted\"prefix", "@original.rsp"], + [], + Path.Combine(retryRoot, "1"), + retryRoot, + "pipe-name", + lastListOfFailedId: null, + attemptCount: 1).ConfigureAwait(false); + + Assert.AreSequenceEqual( + [ + "exec", + "quoted\"prefix", + $"@{responseFilePath}", + $"--{PlatformCommandLineProvider.ResultDirectoryOptionKey}", + Path.Combine(retryRoot, "1"), + $"--{RetryCommandLineOptionsProvider.RetryFailedTestsPipeNameOptionName}", + "pipe-name", + ], + actual); + Assert.AreEqual( + $"\"--keep\"{Environment.NewLine}\"value\"{Environment.NewLine}", + Encoding.UTF8.GetString(memoryStream.ToArray())); + } + + [TestMethod] + public async Task BuildAttemptArgumentsAsync_WithMultipleOriginalResponseFiles_WritesEntireExpandedSuffix() + { + string retryRoot = Path.Combine("results", "Retries", "run"); + string responseFilePath = Path.Combine(retryRoot, "retry-arguments-1.rsp"); + string[] executableArguments = ["exec", "--first", "a", "--between", "b", "--second", "c", "--after", "d"]; + using var memoryStream = new MemoryStream(); + var fileStream = new Mock(MockBehavior.Strict); + fileStream.SetupGet(stream => stream.Stream).Returns(memoryStream); + fileStream.Setup(stream => stream.Dispose()); + var fileSystem = new Mock(MockBehavior.Strict); + fileSystem + .Setup(fs => fs.NewFileStream(responseFilePath, FileMode.Create, FileAccess.Write)) + .Returns(fileStream.Object); + + List actual = await RetryArgumentsBuilder.BuildAttemptArgumentsAsync( + fileSystem.Object, + executableArguments, + ["exec", "@first.rsp", "--between", "b", "@second.rsp", "--after", "d"], + [], + Path.Combine(retryRoot, "1"), + retryRoot, + "pipe-name", + lastListOfFailedId: null, + attemptCount: 1).ConfigureAwait(false); + + Assert.AreEqual("exec", actual[0]); + Assert.AreEqual($"@{responseFilePath}", actual[1]); + Assert.AreEqual( + string.Join(Environment.NewLine, executableArguments.Skip(1).Select(argument => $"\"{argument}\"")) + Environment.NewLine, + Encoding.UTF8.GetString(memoryStream.ToArray())); + } + [TestMethod] public Task BuildAttemptArgumentsAsync_WithNullFailedIds_KeepsOriginalFiltersAndMinimumExpectedTests() => AssertFirstAttemptKeepsOriginalFiltersAsync(lastListOfFailedId: null); diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs index 2cad353381..c46899b1df 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using Microsoft.Testing.Platform.CommandLine; + namespace Microsoft.Testing.Platform.UnitTests; [TestClass] @@ -208,6 +210,57 @@ public void TryReadResponseFile_EmptyFile_ReturnsTrueWithEmptyArray() } } + [TestMethod] + public void TryReadResponseFile_MissingNestedFile_ReportsRedactedNestedPath() + { + string outerPath = Path.GetTempFileName(); + const string SensitiveNestedPath = "sensitive-nested-path.rsp"; + try + { + File.WriteAllText( + outerPath, + $"--{PlatformCommandLineProvider.DotNetTestHttpTokenOptionKey}{Environment.NewLine}@{SensitiveNestedPath}"); + var errors = new List(); + + bool result = ResponseFileHelper.TryReadResponseFile(outerPath, errors, out string[]? args); + + Assert.IsFalse(result); + Assert.IsNull(args); + Assert.HasCount(1, errors); + Assert.Contains("***REDACTED***", errors[0]); + Assert.DoesNotContain(SensitiveNestedPath, errors[0]); + Assert.DoesNotContain(outerPath, errors[0]); + } + finally + { + File.Delete(outerPath); + } + } + + [TestMethod] + public void TryReadResponseFile_MissingNestedFile_ReportsNestedPath() + { + string outerPath = Path.GetTempFileName(); + string nestedPath = $"{Guid.NewGuid():N}-missing.rsp"; + try + { + File.WriteAllText(outerPath, $"@{nestedPath}"); + var errors = new List(); + + bool result = ResponseFileHelper.TryReadResponseFile(outerPath, errors, out string[]? args); + + Assert.IsFalse(result); + Assert.IsNull(args); + Assert.HasCount(1, errors); + Assert.Contains(nestedPath, errors[0]); + Assert.DoesNotContain(outerPath, errors[0]); + } + finally + { + File.Delete(outerPath); + } + } + [TestMethod] public void SplitCommandLine_EmptyString_ReturnsEmpty() { From 5958a7c00283208e57e2b2a42ac78355eef114e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 3 Sep 2026 13:46:27 +0200 Subject: [PATCH 2/3] Add retry response file regression coverage Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../InternalAPI/InternalAPI.Unshipped.txt | 1 + .../RetryOrchestrator.cs | 27 ++++++-- .../RetryFailedTestsTests.cs | 8 +++ .../RetryTests.cs | 36 ++++++++++ .../CommandLine/ResponseFileHelperTests.cs | 65 ++++++++++++++++++- 5 files changed, 127 insertions(+), 10 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt b/src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt index ffa48213c9..86abd98eda 100644 --- a/src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt +++ b/src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt @@ -2,3 +2,4 @@ static Microsoft.Testing.Extensions.Policy.RetryArgumentsBuilder.BuildAttemptArgumentsAsync(Microsoft.Testing.Platform.Helpers.IFileSystem! fileSystem, string![]! executableArguments, string![]! originalExecutableArguments, System.Collections.Generic.List! indexToCleanup, string! currentTryResultFolder, string! retryRootFolder, string! pipeName, string![]? lastListOfFailedId, int attemptCount) -> System.Threading.Tasks.Task!>! static Microsoft.Testing.Extensions.Policy.RetryArgumentsBuilder.GetArgumentsResponseFilePath(string! retryRootFolder, int attemptCount) -> string! static Microsoft.Testing.Extensions.Policy.RetryArgumentsBuilder.GetFilterUidsResponseFilePath(string! retryRootFolder, int attemptCount) -> string! +static Microsoft.Testing.Extensions.Policy.RetryOrchestrator.LogResponseFileFallbackWarningAsync(Microsoft.Testing.Platform.Logging.ILogger! logger, string![]! originalExecutableArguments, System.Collections.Generic.List! finalArguments, string! generatedResponseFilePath) -> System.Threading.Tasks.Task! diff --git a/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs b/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs index 240e7ee9c4..869da5e415 100644 --- a/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs +++ b/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs @@ -239,13 +239,11 @@ await outputDevice.DisplayAsync( lastListOfFailedId, attemptCount).ConfigureAwait(false); - if (originalExecutableArguments.Any(argument => argument.StartsWith("@", StringComparison.Ordinal)) - && !finalArguments.Contains($"@{generatedResponseFilePaths[0]}")) - { - await logger.LogWarningAsync( - "Retry arguments could not be regenerated in a response file because an argument contains a literal double quote. " - + "The retry command line may exceed the operating system limit.").ConfigureAwait(false); - } + await LogResponseFileFallbackWarningAsync( + logger, + originalExecutableArguments, + finalArguments, + generatedResponseFilePaths[0]).ConfigureAwait(false); attemptResult = await RetryTestHostRunner.RunAttemptAsync( _serviceProvider, @@ -515,6 +513,21 @@ private static bool IsHotReloadEnabled(IEnvironment environment) => environment.GetEnvironmentVariable(EnvironmentVariableConstants.DOTNET_WATCH) == "1" || environment.GetEnvironmentVariable(EnvironmentVariableConstants.TESTINGPLATFORM_HOTRELOAD_ENABLED) == "1"; + internal static async Task LogResponseFileFallbackWarningAsync( + ILogger logger, + string[] originalExecutableArguments, + List finalArguments, + string generatedResponseFilePath) + { + if (originalExecutableArguments.Any(argument => argument.StartsWith("@", StringComparison.Ordinal)) + && !finalArguments.Contains($"@{generatedResponseFilePath}")) + { + await logger.LogWarningAsync( + "Retry arguments could not be regenerated in a response file because an argument contains a literal double quote. " + + "The retry command line may exceed the operating system limit.").ConfigureAwait(false); + } + } + private static void CollectRecoveredArtifacts( IFileSystem fileSystem, string manifestPath, diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/RetryFailedTestsTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/RetryFailedTestsTests.cs index 4d1b4c43ed..e493660ee1 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/RetryFailedTestsTests.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/RetryFailedTestsTests.cs @@ -164,6 +164,7 @@ public async Task RetryFailedTests_WithArgumentsInResponseFile_Succeeds(bool ret { "METHOD1", "1" }, { "FAIL", "0" }, { "RESULTDIR", resultDirectory }, + { "CHECK_RETRY_RESPONSE_FILE_CLEANUP", "1" }, }, cancellationToken: TestContext.CancellationToken); @@ -1004,6 +1005,13 @@ public async Task ExecuteRequestAsync(ExecuteRequestContext context) var uidFilter = filter as TestNodeUidListFilter; var treeNodeFilter = filter as TreeNodeFilter; + if (Environment.GetEnvironmentVariable("CHECK_RETRY_RESPONSE_FILE_CLEANUP") == "1" + && Environment.GetEnvironmentVariable("TESTINGPLATFORM_DOTNETTEST_ATTEMPTNUMBER") == "2" + && Directory.GetFiles(Path.Combine(resultDir, "Retries"), "retry-arguments-1.rsp", SearchOption.AllDirectories).Length != 0) + { + throw new InvalidOperationException("The response file from retry attempt 1 still exists during attempt 2."); + } + var testMethod1Identifier = new TestMethodIdentifierProperty(string.Empty, string.Empty, "DummyClassName", "TestMethod1", 0, Array.Empty(), string.Empty); var testMethod2Identifier = new TestMethodIdentifierProperty(string.Empty, string.Empty, "DummyClassName", "TestMethod2", 0, Array.Empty(), string.Empty); var testMethod3Identifier = new TestMethodIdentifierProperty(string.Empty, string.Empty, "DummyClassName", "TestMethod3", 0, Array.Empty(), string.Empty); diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs index af44d9ebd5..ce3ce44d18 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/RetryTests.cs @@ -30,6 +30,42 @@ public class RetryTests { private const string ContosoPackageSid = "S-1-15-2-1990679259-4123976751-842158434-3026549936-2944832882-252165955-409282942"; + [DataRow(true)] + [DataRow(false)] + [TestMethod] + public async Task LogResponseFileFallbackWarningAsync_OnlyQuotedSuffixLogsWarning(bool quoteIsInDirectPrefix) + { + const string GeneratedResponseFilePath = "retry-arguments-1.rsp"; + string[] originalArguments = quoteIsInDirectPrefix + ? ["quoted\"prefix", "@original.rsp"] + : ["@original.rsp", "quoted\"suffix"]; + List finalArguments = quoteIsInDirectPrefix + ? ["quoted\"prefix", $"@{GeneratedResponseFilePath}"] + : ["quoted\"suffix"]; + var logger = new Mock(); + logger + .Setup(value => value.LogAsync( + LogLevel.Warning, + It.IsAny(), + null, + It.IsAny>())) + .Returns(Task.CompletedTask); + + await RetryOrchestrator.LogResponseFileFallbackWarningAsync( + logger.Object, + originalArguments, + finalArguments, + GeneratedResponseFilePath); + + logger.Verify( + value => value.LogAsync( + LogLevel.Warning, + It.Is(message => message.Contains("literal double quote", StringComparison.Ordinal)), + null, + It.IsAny>()), + quoteIsInDirectPrefix ? Times.Never : Times.Once); + } + [TestMethod] [OSCondition(ConditionMode.Include, OperatingSystems.Windows, IgnoreMessage = "AppContainer pipe authorization is Windows-only.")] public void RetryPipeServer_UsesControllerAuthorizedSecurityIdentities() diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs index c46899b1df..36f0c987ca 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs @@ -214,12 +214,12 @@ public void TryReadResponseFile_EmptyFile_ReturnsTrueWithEmptyArray() public void TryReadResponseFile_MissingNestedFile_ReportsRedactedNestedPath() { string outerPath = Path.GetTempFileName(); - const string SensitiveNestedPath = "sensitive-nested-path.rsp"; + string sensitiveNestedPath = $"{Guid.NewGuid():N}-sensitive-missing.rsp"; try { File.WriteAllText( outerPath, - $"--{PlatformCommandLineProvider.DotNetTestHttpTokenOptionKey}{Environment.NewLine}@{SensitiveNestedPath}"); + $"--{PlatformCommandLineProvider.DotNetTestHttpTokenOptionKey}{Environment.NewLine}@{sensitiveNestedPath}"); var errors = new List(); bool result = ResponseFileHelper.TryReadResponseFile(outerPath, errors, out string[]? args); @@ -228,7 +228,7 @@ public void TryReadResponseFile_MissingNestedFile_ReportsRedactedNestedPath() Assert.IsNull(args); Assert.HasCount(1, errors); Assert.Contains("***REDACTED***", errors[0]); - Assert.DoesNotContain(SensitiveNestedPath, errors[0]); + Assert.DoesNotContain(sensitiveNestedPath, errors[0]); Assert.DoesNotContain(outerPath, errors[0]); } finally @@ -261,6 +261,65 @@ public void TryReadResponseFile_MissingNestedFile_ReportsNestedPath() } } + [TestMethod] + public void TryReadResponseFile_MalformedNestedFile_ReportsRedactedNestedPathWithoutDetails() + { + string outerPath = Path.GetTempFileName(); + string sensitiveNestedPath = Path.GetTempFileName(); + try + { + File.WriteAllText(sensitiveNestedPath, "--filter \"unclosed"); + File.WriteAllText( + outerPath, + $"--{PlatformCommandLineProvider.DotNetTestHttpTokenOptionKey}{Environment.NewLine}@{sensitiveNestedPath}"); + var errors = new List(); + + bool result = ResponseFileHelper.TryReadResponseFile(outerPath, errors, out string[]? args); + + Assert.IsFalse(result); + Assert.IsNull(args); + Assert.HasCount(1, errors); + Assert.Contains("***REDACTED***", errors[0]); + Assert.Contains(nameof(FormatException), errors[0]); + Assert.DoesNotContain(sensitiveNestedPath, errors[0]); + Assert.DoesNotContain(outerPath, errors[0]); + Assert.DoesNotContain(nameof(ResponseFileHelper.TryReadResponseFile), errors[0]); + } + finally + { + File.Delete(sensitiveNestedPath); + File.Delete(outerPath); + } + } + + [TestMethod] + public void TryReadResponseFile_MalformedNestedFile_ReportsNestedPathWithDetails() + { + string outerPath = Path.GetTempFileName(); + string nestedPath = Path.GetTempFileName(); + try + { + File.WriteAllText(nestedPath, "--filter \"unclosed"); + File.WriteAllText(outerPath, $"@{nestedPath}"); + var errors = new List(); + + bool result = ResponseFileHelper.TryReadResponseFile(outerPath, errors, out string[]? args); + + Assert.IsFalse(result); + Assert.IsNull(args); + Assert.HasCount(1, errors); + Assert.Contains(nestedPath, errors[0]); + Assert.DoesNotContain(outerPath, errors[0]); + Assert.Contains(nameof(FormatException), errors[0]); + Assert.Contains(nameof(ResponseFileHelper.TryReadResponseFile), errors[0]); + } + finally + { + File.Delete(nestedPath); + File.Delete(outerPath); + } + } + [TestMethod] public void SplitCommandLine_EmptyString_ReturnsEmpty() { From 9a05ae9d7bf293d102cd7aca4be1c9813dcb840e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 3 Sep 2026 15:58:39 +0200 Subject: [PATCH 3/3] Preserve redaction across nested response files Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../CommandLine/ResponseFileHelper.cs | 16 +++++++--- .../CommandLine/ResponseFileHelperTests.cs | 31 +++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/CommandLine/ResponseFileHelper.cs b/src/Platform/Microsoft.Testing.Platform/CommandLine/ResponseFileHelper.cs index 9391f37d49..e86d09ab24 100644 --- a/src/Platform/Microsoft.Testing.Platform/CommandLine/ResponseFileHelper.cs +++ b/src/Platform/Microsoft.Testing.Platform/CommandLine/ResponseFileHelper.cs @@ -94,10 +94,18 @@ static IEnumerable ExpandResponseFile( string argument = arguments[argumentIndex]; if (argument.StartsWith("@", StringComparison.Ordinal)) { - string redactedNestedArgument = CommandLineArgumentsRedactor.RedactArgument([.. arguments], argumentIndex); - string nestedDiagnosticPath = redactedNestedArgument.StartsWith("@", StringComparison.Ordinal) - ? redactedNestedArgument[1..] - : redactedNestedArgument; + string nestedDiagnosticPath; + if (filePath != diagnosticPath) + { + nestedDiagnosticPath = diagnosticPath; + } + else + { + string redactedNestedArgument = CommandLineArgumentsRedactor.RedactArgument([.. arguments], argumentIndex); + nestedDiagnosticPath = redactedNestedArgument.StartsWith("@", StringComparison.Ordinal) + ? redactedNestedArgument[1..] + : redactedNestedArgument; + } // Nested response files intentionally use the process working directory, just like // top-level response files, rather than the containing response file's directory. diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs index 36f0c987ca..074950309e 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ResponseFileHelperTests.cs @@ -261,6 +261,37 @@ public void TryReadResponseFile_MissingNestedFile_ReportsNestedPath() } } + [TestMethod] + public void TryReadResponseFile_MissingNestedFileBelowRedactedFile_InheritsRedaction() + { + string outerPath = Path.GetTempFileName(); + string middlePath = Path.GetTempFileName(); + string sensitiveNestedPath = $"{Guid.NewGuid():N}-sensitive-missing.rsp"; + try + { + File.WriteAllText(middlePath, $"@{sensitiveNestedPath}"); + File.WriteAllText( + outerPath, + $"--{PlatformCommandLineProvider.DotNetTestHttpTokenOptionKey}{Environment.NewLine}@{middlePath}"); + var errors = new List(); + + bool result = ResponseFileHelper.TryReadResponseFile(outerPath, errors, out string[]? args); + + Assert.IsFalse(result); + Assert.IsNull(args); + Assert.HasCount(1, errors); + Assert.Contains("***REDACTED***", errors[0]); + Assert.DoesNotContain(sensitiveNestedPath, errors[0]); + Assert.DoesNotContain(middlePath, errors[0]); + Assert.DoesNotContain(outerPath, errors[0]); + } + finally + { + File.Delete(middlePath); + File.Delete(outerPath); + } + } + [TestMethod] public void TryReadResponseFile_MalformedNestedFile_ReportsRedactedNestedPathWithoutDetails() {