diff --git a/src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportEngine.FileWriter.cs b/src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportEngine.FileWriter.cs index b2e7749d82..725d92cfc8 100644 --- a/src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportEngine.FileWriter.cs +++ b/src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportEngine.FileWriter.cs @@ -7,72 +7,19 @@ namespace Microsoft.Testing.Extensions.CtrfReport; internal sealed partial class CtrfReportEngine { - private async Task<(string FileName, string? Warning)> WriteWithRetryAsync(string finalPath, byte[] bytes, bool fileNameExplicitlyProvided) + private async Task<(string FileName, string? Warning)> WriteAsync(string finalPath, byte[] bytes) { - // Explicit file names: use FileMode.Create (overwrite). Default-generated file - // names: use FileMode.CreateNew but retry with disambiguating suffixes when the - // file already exists, so concurrent runs (or two runs within the same second - // sharing the result directory) don't fail with IOException. - if (fileNameExplicitlyProvided) - { - bool willOverwrite = _fileSystem.ExistFile(finalPath); - await WriteBytesAsync(finalPath, FileMode.Create, bytes).ConfigureAwait(false); - return ( - finalPath, - willOverwrite - ? string.Format(CultureInfo.InvariantCulture, ExtensionResources.CtrfReportFileExistsAndWillBeOverwritten, finalPath) - : null); - } - - DateTimeOffset firstTry = _clock.UtcNow; - string directory = Path.GetDirectoryName(finalPath) ?? string.Empty; - string fileName = Path.GetFileName(finalPath); - SplitCtrfExtension(fileName, out string baseName, out string extension); - string candidate = finalPath; - int attempt = 0; - - while (true) - { - _cancellationToken.ThrowIfCancellationRequested(); - - try - { - await WriteBytesAsync(candidate, FileMode.CreateNew, bytes).ConfigureAwait(false); - return (candidate, null); - } - catch (IOException) when (_fileSystem.ExistFile(candidate)) - { - // The IOException was caused by the file already existing. Try a - // suffixed name. Any other IOException (disk full, permission, path - // too long, etc.) is not caught here and will propagate to the caller. - // Bound by both wall-clock (5s) and attempt count (1000) so we never - // spin forever in pathological cases like a clock that doesn't advance. - if (_clock.UtcNow - firstTry > TimeSpan.FromSeconds(5) || attempt >= 1_000) - { - throw; - } - - attempt++; - candidate = Path.Combine(directory, $"{baseName}_{attempt}{extension}"); - } - } - } - - // Split a file name into base + extension while preserving the CTRF - // double-extension convention (`*.ctrf.json`). The disambiguation suffix - // must land before `.ctrf.json` so that downstream CTRF readers continue to - // recognize the file by its conventional extension. - private static void SplitCtrfExtension(string fileName, out string baseName, out string extension) - { - const string ctrfJsonSuffix = ".ctrf.json"; - if (fileName.EndsWith(ctrfJsonSuffix, StringComparison.OrdinalIgnoreCase) && fileName.Length > ctrfJsonSuffix.Length) - { - baseName = fileName.Substring(0, fileName.Length - ctrfJsonSuffix.Length); - extension = fileName.Substring(fileName.Length - ctrfJsonSuffix.Length); - return; - } - - baseName = Path.GetFileNameWithoutExtension(fileName); - extension = Path.GetExtension(fileName); + // Always overwrite (FileMode.Create), regardless of whether the file name was explicitly + // provided or generated from the default + // ____.ctrf.json shape. Emit a warning when overwriting + // so users have a single, predictable rule to reason about — matching the TRX, HTML and + // JUnit report extensions. + bool willOverwrite = _fileSystem.ExistFile(finalPath); + await WriteBytesAsync(finalPath, FileMode.Create, bytes).ConfigureAwait(false); + return ( + finalPath, + willOverwrite + ? string.Format(CultureInfo.InvariantCulture, ExtensionResources.CtrfReportFileExistsAndWillBeOverwritten, finalPath) + : null); } } diff --git a/src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportEngine.cs b/src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportEngine.cs index 6bae80fa86..fc0d1e0e19 100644 --- a/src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportEngine.cs +++ b/src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportEngine.cs @@ -19,12 +19,12 @@ public CtrfReportEngine(ReportEngineContext context) private async Task<(string FileName, string? Warning)> GenerateReportCoreAsync(CapturedTestResult[] results, DateTimeOffset finishTime) { - (string finalPath, bool fileNameExplicitlyProvided) = ResolveOutputPath( + (string finalPath, _) = ResolveOutputPath( CtrfReportGeneratorCommandLine.CtrfReportFileNameOptionName, () => BuildDefaultFileName(finishTime)); byte[] bytes = BuildCtrfJson(results, finishTime); - return await WriteWithRetryAsync(finalPath, bytes, fileNameExplicitlyProvided).ConfigureAwait(false); + return await WriteAsync(finalPath, bytes).ConfigureAwait(false); } } diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportEngineTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportEngineTests.cs index 703595ac35..432b7a00e5 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportEngineTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportEngineTests.cs @@ -393,7 +393,7 @@ public async Task GenerateReportAsync_DefaultFileName_IncludesModuleNameAndTarge { string? pathSeen = null; _ = _fileSystem.Setup(x => x.ExistFile(It.IsAny())).Returns(false); - _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny(), FileMode.CreateNew)) + _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny(), FileMode.Create)) .Returns((path, _) => { pathSeen = path; @@ -485,20 +485,19 @@ public async Task GenerateReportAsync_ExplicitAbsolutePath_OverridesResultsDirec } [TestMethod] - public async Task GenerateReportAsync_AppendsDisambiguatingSuffix_When_DefaultFileExists() + public async Task GenerateReportAsync_OverwritesAndWarns_When_DefaultFileExists() { - var bytesSeen = new List(); - int callCount = 0; - _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny(), FileMode.CreateNew)) + // Default-name path uses the same overwrite-and-warn semantics as the explicit-name + // path: a single, predictable rule. When the file already exists, the engine + // overwrites it (FileMode.Create) and surfaces the CtrfReportFileExistsAndWillBeOverwritten + // warning. + string? pathSeen = null; + _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny(), FileMode.Create)) .Returns((path, _) => { - callCount++; - bytesSeen.Add(path); - return callCount == 1 - ? throw new IOException("file exists") - : new MemoryFileStream(); + pathSeen = path; + return new MemoryFileStream(); }); - _ = _fileSystem.Setup(x => x.ExistFile(It.IsAny())).Returns(true); _ = _configurationMock.SetupGet(_ => _[It.IsAny()]).Returns(string.Empty); @@ -522,18 +521,22 @@ public async Task GenerateReportAsync_AppendsDisambiguatingSuffix_When_DefaultFi 0, CancellationToken.None)); - (string finalPath, _) = await engine.GenerateReportAsync([Captured("a", "A", "passed")]); + (string finalPath, string? warning) = await engine.GenerateReportAsync([Captured("a", "A", "passed")]); - Assert.AreEqual(2, callCount); - Assert.AreEqual(bytesSeen[1], finalPath); - Assert.Contains("_1.ctrf.json", finalPath); + Assert.AreEqual(pathSeen, finalPath); + Assert.DoesNotContain("_1.ctrf.json", finalPath); + Assert.IsNotNull(warning); + Assert.Contains(finalPath, warning!); } [TestMethod] - public async Task GenerateReportAsync_PropagatesIOException_When_FileDoesNotExist() + public async Task GenerateReportAsync_PropagatesIOException_When_WriteFails() { + // An IOException during the write (e.g. disk full, permission denied, path too + // long) must propagate to the caller — there is no longer any disambiguation + // loop that could mask such failures behind a retry budget. int callCount = 0; - _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny(), FileMode.CreateNew)) + _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny(), FileMode.Create)) .Returns((path, _) => { callCount++;