From 3583519bc2865e28583700c913e7c2eb95aa1d25 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Tue, 14 Jan 2025 19:32:00 +0100 Subject: [PATCH 1/4] Avoid keeping trx stream open in unit tests --- .../TrxTests.cs | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs index 23b29e8e5a..16eee72f7a 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs @@ -39,7 +39,7 @@ public async Task TrxReportEngine_GenerateReportAsyncWithNullAdapterSupportTrxCa TrxReportEngine trxReportEngine = GenerateTrxReportEngine(1, 0, propertyBag, memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -58,7 +58,7 @@ public async Task TrxReportEngine_GenerateReportAsyncWithNotExecutedTests_TrxExe TrxReportEngine trxReportEngine = GenerateTrxReportEngine(1, 0, propertyBag, memoryStream, notExecutedTestsCount: 1); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -77,7 +77,7 @@ public async Task TrxReportEngine_GenerateReportAsyncWithTimeoutTests_TrxTimeout TrxReportEngine trxReportEngine = GenerateTrxReportEngine(1, 0, propertyBag, memoryStream, timeoutTestsCount: 1); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -98,7 +98,7 @@ public async Task TrxReportEngine_GenerateReportAsync_WithArgumentTrxReportFileN TrxReportEngine trxReportEngine = GenerateTrxReportEngine(1, 0, propertyBag, memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert Assert.IsTrue(fileName.Equals("argumentTrxReportFileName", StringComparison.OrdinalIgnoreCase)); @@ -117,7 +117,7 @@ public async Task TrxReportEngine_GenerateReportAsync_WithInvalidArgumentValueFo TrxReportEngine trxReportEngine = GenerateTrxReportEngine(1, 0, propertyBag, memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert Assert.IsTrue(fileName.Equals("_NUL", StringComparison.OrdinalIgnoreCase)); @@ -134,7 +134,7 @@ public async Task TrxReportEngine_GenerateReportAsync_WithTestHostCrash_ResultSu TrxReportEngine trxReportEngine = GenerateTrxReportEngine(1, 0, propertyBag, memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(isTestHostCrashed: true, keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(isTestHostCrashed: true); // Assert AssertExpectedTrxFileName(fileName); @@ -151,7 +151,7 @@ public async Task TrxReportEngine_GenerateReportAsync_WithTestSkipped_ResultSumm TrxReportEngine trxReportEngine = GenerateTrxReportEngine(0, 0, propertyBag, memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -170,7 +170,7 @@ public async Task TrxReportEngine_GenerateReportAsync_WithTestFailed_WithStandar TrxReportEngine trxReportEngine = GenerateTrxReportEngine(0, 1, propertyBag, memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -203,7 +203,7 @@ public async Task TrxReportEngine_GenerateReportAsync_WithTestFailed_WithoutStan TrxReportEngine trxReportEngine = GenerateTrxReportEngine(0, 1, propertyBag, memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -231,7 +231,7 @@ public async Task TrxReportEngine_GenerateReportAsync_WithTestFailed_WithoutStan TrxReportEngine trxReportEngine = GenerateTrxReportEngine(0, 1, propertyBag, memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -262,7 +262,7 @@ public async Task TrxReportEngine_GenerateReportAsync_PassedTestWithTestCategory TrxReportEngine trxReportEngine = GenerateTrxReportEngine(1, 0, propertyBag, memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -289,7 +289,7 @@ public async Task TrxReportEngine_GenerateReportAsync_FailedTestWithTestCategory TrxReportEngine trxReportEngine = GenerateTrxReportEngine(0, 1, propertyBag, memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -317,7 +317,7 @@ public async Task TrxReportEngine_GenerateReportAsync_WithAdapterSupportTrxCapab propertyBag, memoryStream, true); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -337,7 +337,7 @@ public async Task TrxReportEngine_GenerateReportAsync_WithArtifactsByTestNode_Tr new(new PassedTestNodeStateProperty()), memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -366,7 +366,7 @@ public async Task TrxReportEngine_GenerateReportAsync_WithArtifactsByExtension_T new(new PassedTestNodeStateProperty()), memoryStream); // Act - string fileName = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + string fileName = await trxReportEngine.GenerateReportAsync(); // Assert AssertExpectedTrxFileName(fileName); @@ -413,7 +413,7 @@ public async Task TrxReportEngine_GenerateReportAsync_FileAlreadyExists_WillRetr isCopyingFileAllowed: false); // Act - _ = await trxReportEngine.GenerateReportAsync(keepReportFileStreamOpen: true); + _ = await trxReportEngine.GenerateReportAsync(); // Assert Assert.AreEqual(4, retryCount); From b582137a93e35e6d929a5e61fd028b350c636483 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Tue, 14 Jan 2025 19:56:59 +0100 Subject: [PATCH 2/4] Get the XDocument on dispose --- .../TrxTests.cs | 70 ++++++++++++------- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs index 16eee72f7a..57c0909c35 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs @@ -43,7 +43,8 @@ public async Task TrxReportEngine_GenerateReportAsyncWithNullAdapterSupportTrxCa // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Completed"); string trxContent = xml.ToString(); Assert.IsFalse(trxContent.Contains(@"className=")); @@ -62,7 +63,8 @@ public async Task TrxReportEngine_GenerateReportAsyncWithNotExecutedTests_TrxExe // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Completed"); string trxContent = xml.ToString(); Assert.IsTrue(trxContent.Contains(@"notExecuted=""1""")); @@ -81,7 +83,8 @@ public async Task TrxReportEngine_GenerateReportAsyncWithTimeoutTests_TrxTimeout // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Completed"); string trxContent = xml.ToString(); Assert.IsTrue(trxContent.Contains(@"timeout=""1""")); @@ -102,7 +105,8 @@ public async Task TrxReportEngine_GenerateReportAsync_WithArgumentTrxReportFileN // Assert Assert.IsTrue(fileName.Equals("argumentTrxReportFileName", StringComparison.OrdinalIgnoreCase)); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Completed"); } @@ -121,7 +125,8 @@ public async Task TrxReportEngine_GenerateReportAsync_WithInvalidArgumentValueFo // Assert Assert.IsTrue(fileName.Equals("_NUL", StringComparison.OrdinalIgnoreCase)); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Completed"); } @@ -138,7 +143,8 @@ public async Task TrxReportEngine_GenerateReportAsync_WithTestHostCrash_ResultSu // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Failed"); } @@ -155,7 +161,8 @@ public async Task TrxReportEngine_GenerateReportAsync_WithTestSkipped_ResultSumm // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Completed"); } @@ -174,7 +181,8 @@ public async Task TrxReportEngine_GenerateReportAsync_WithTestFailed_WithStandar // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Failed"); XElement? testRun = xml.Root; @@ -207,7 +215,8 @@ public async Task TrxReportEngine_GenerateReportAsync_WithTestFailed_WithoutStan // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Failed"); string trxContent = xml.ToString(); string trxContentsPattern = @" @@ -235,7 +244,8 @@ public async Task TrxReportEngine_GenerateReportAsync_WithTestFailed_WithoutStan // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Failed"); string trxContent = xml.ToString(); string trxContentsPattern = @" @@ -266,7 +276,8 @@ public async Task TrxReportEngine_GenerateReportAsync_PassedTestWithTestCategory // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Completed"); string trxContent = xml.ToString(); string trxContentsPattern = @" @@ -293,7 +304,8 @@ public async Task TrxReportEngine_GenerateReportAsync_FailedTestWithTestCategory // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Failed"); string trxContent = xml.ToString(); string trxContentsPattern = @" @@ -321,7 +333,8 @@ public async Task TrxReportEngine_GenerateReportAsync_WithAdapterSupportTrxCapab // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Completed"); string trxContent = xml.ToString(); Assert.IsTrue(trxContent.Contains(@"className=""TrxFullyQualifiedTypeName"), trxContent); @@ -341,7 +354,8 @@ public async Task TrxReportEngine_GenerateReportAsync_WithArtifactsByTestNode_Tr // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Completed"); string trxContent = xml.ToString(); string trxContentsPattern = @" @@ -370,7 +384,8 @@ public async Task TrxReportEngine_GenerateReportAsync_WithArtifactsByExtension_T // Assert AssertExpectedTrxFileName(fileName); - XDocument xml = GetTrxContent(memoryStream); + Assert.IsNotNull(memoryStream.TrxContent); + XDocument xml = memoryStream.TrxContent; AssertTrxOutcome(xml, "Completed"); string trxContent = xml.ToString(); string trxContentsPattern = @" @@ -419,13 +434,6 @@ public async Task TrxReportEngine_GenerateReportAsync_FileAlreadyExists_WillRetr Assert.AreEqual(4, retryCount); } - private static XDocument GetTrxContent(MemoryFileStream memoryStream) - { - Assert.IsNotNull(memoryStream); - _ = memoryStream.Stream.Seek(0, SeekOrigin.Begin); - return XDocument.Load(memoryStream.Stream); - } - private static void AssertTrxOutcome(XDocument xml, string expectedOutcome) { Assert.IsNotNull(xml); @@ -473,16 +481,30 @@ private sealed class MemoryFileStream : IFileStream public MemoryStream Stream { get; } + public XDocument? TrxContent { get; private set; } + Stream IFileStream.Stream => Stream; string IFileStream.Name => string.Empty; + private void SetTrxContent() + { + _ = Stream.Seek(0, SeekOrigin.Begin); + TrxContent = XDocument.Load(Stream); + } + void IDisposable.Dispose() - => Stream.Dispose(); + { + SetTrxContent(); + Stream.Dispose(); + } #if NETCOREAPP ValueTask IAsyncDisposable.DisposeAsync() - => Stream.DisposeAsync(); + { + SetTrxContent(); + return Stream.DisposeAsync(); + } #endif } } From d8244d012a7a1eab791b032a478aff8d6354b0f9 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Wed, 15 Jan 2025 08:21:03 +0100 Subject: [PATCH 3/4] Fix failures --- .../TrxReportEngine.cs | 25 ++++++------------- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.cs b/src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.cs index db577d90bd..a359624799 100644 --- a/src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.cs +++ b/src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.cs @@ -142,7 +142,7 @@ public TrxReportEngine(IFileSystem fileSystem, ITestApplicationModuleInfo testAp _isCopyingFileAllowed = isCopyingFileAllowed; } - public async Task GenerateReportAsync(string testHostCrashInfo = "", bool isTestHostCrashed = false, bool keepReportFileStreamOpen = false) + public async Task GenerateReportAsync(string testHostCrashInfo = "", bool isTestHostCrashed = false) => await RetryWhenIOExceptionAsync(async () => { string testAppModule = _testApplicationModuleInfo.GetCurrentTestApplicationFullPath(); @@ -189,23 +189,12 @@ public async Task GenerateReportAsync(string testHostCrashInfo = "", boo string outputDirectory = _configuration.GetTestResultDirectory(); // add var for this string finalFileName = Path.Combine(outputDirectory, trxFileName); - Stream stream = _fileSystem.NewFileStream(finalFileName, FileMode.CreateNew).Stream; - try - { - await document.SaveAsync(stream, SaveOptions.None, _cancellationToken); - return finalFileName; - } - finally - { - if (!keepReportFileStreamOpen) - { -#if NET - await stream.DisposeAsync(); -#else - stream.Dispose(); -#endif - } - } + + // Note that we need to dispose the IFileStream, not the inner stream. + // IFileStream implementations will be responsible to dispose their inner stream. + using IFileStream stream = _fileSystem.NewFileStream(finalFileName, FileMode.CreateNew); + await document.SaveAsync(stream.Stream, SaveOptions.None, _cancellationToken); + return finalFileName; }); private async Task RetryWhenIOExceptionAsync(Func> func) From 95791a94b186eaf7fe9effa1e1fe975aefb001a1 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Wed, 15 Jan 2025 09:26:52 +0100 Subject: [PATCH 4/4] Small fix --- .../Microsoft.Testing.Extensions.UnitTests/TrxTests.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs index 57c0909c35..da7a7862f8 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs @@ -489,8 +489,11 @@ private sealed class MemoryFileStream : IFileStream private void SetTrxContent() { - _ = Stream.Seek(0, SeekOrigin.Begin); - TrxContent = XDocument.Load(Stream); + if (TrxContent is null) + { + _ = Stream.Seek(0, SeekOrigin.Begin); + TrxContent = XDocument.Load(Stream); + } } void IDisposable.Dispose()