AzureDevOpsReporter: skip Assert frames by type name instead of filename (fixes #6925)

src/Platform/Microsoft.Testing.Extensions.AzureDevOpsReport/AzureDevOpsReporter.cs

@@ -21,6 +21,19 @@ internal sealed class AzureDevOpsReporter :
 {
     private const string DeterministicBuildRoot = "/_/";
 
+    // Fully-qualified type prefixes for MSTest assertion implementations. A stack frame whose
+    // 'code' (i.e., the "Namespace.Type.Method(args)" portion) starts with any of these is treated
+    // as framework internals and skipped when looking for the user's call site to annotate.
+    // Matching on the type name (rather than the source file name) is robust to partial-class
+    // splits (e.g. Assert.AreEqual.cs, Assert.IComparable.cs) and avoids false positives on user
+    // files innocently named *Assert.cs. See https://github.com/microsoft/testfx/issues/6925.
+    private static readonly string[] AssertionImplementationCodePrefixes =
+    [
+        "Microsoft.VisualStudio.TestTools.UnitTesting.Assert.",
+        "Microsoft.VisualStudio.TestTools.UnitTesting.CollectionAssert.",
+        "Microsoft.VisualStudio.TestTools.UnitTesting.StringAssert.",
+    ];
+
     private readonly IOutputDevice _outputDisplay;
     private readonly ILogger _logger;
     private static readonly char[] NewlineCharacters = ['\r', '\n'];
@@ -210,13 +223,13 @@ private async Task WriteExceptionAsync(string testDisplayName, string? explanati
             }
 
             string file = location.Value.File;
+            string code = location.Value.Code;
 
-            // TODO: We need better rule for stackframes to opt out from being interesting.
-            if (file.EndsWith("Assert.cs", StringComparison.Ordinal))
+            if (IsAssertionImplementationFrame(code))
             {
                 if (logger.IsEnabled(LogLevel.Trace))
                 {
-                    logger.LogTrace("StackFrame location ends with 'Assert.cs' this is a special pattern that we skip, continuing to next.");
+                    logger.LogTrace($"StackFrame code '{code}' is an MSTest assertion implementation, continuing to next.");
                 }
 
                 continue;
@@ -311,6 +324,19 @@ private static string GetTargetFrameworkMoniker()
         => TargetFrameworkParser.GetShortTargetFramework(Assembly.GetEntryAssembly()?.GetCustomAttribute<TargetFrameworkAttribute>()?.FrameworkDisplayName)
             ?? TargetFrameworkParser.GetShortTargetFramework(RuntimeInformation.FrameworkDescription);
 
+    private static bool IsAssertionImplementationFrame(string code)
+    {
+        foreach (string prefix in AssertionImplementationCodePrefixes)
+        {
+            if (code.StartsWith(prefix, StringComparison.Ordinal))
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     private static (string Code, string File, int LineNumber)? GetStackFrameLocation(string stackTraceLine)
     {
         Match match = StackTraceHelper.GetFrameRegex().Match(stackTraceLine);

test/IntegrationTests/MSTest.Acceptance.IntegrationTests/TelemetryTests.cs

@@ -8,6 +8,7 @@
 namespace MSTest.Acceptance.IntegrationTests;
 
 [TestClass]
+[DoNotParallelize]
 public sealed class TelemetryTests : AcceptanceTestBase<TelemetryTests.TestAssetFixture>
 {
     private const string MTPAssetName = "TelemetryMTPProject";

test/UnitTests/Microsoft.Testing.Extensions.UnitTests/AzureDevOpsTests.cs

@@ -4,6 +4,9 @@
 using Microsoft.Testing.Extensions.AzureDevOpsReport;
 using Microsoft.Testing.Platform.Helpers;
 using Microsoft.Testing.Platform.Logging;
+using Microsoft.Testing.TestInfrastructure;
+
+using Moq;
 
 namespace Microsoft.Testing.Extensions.UnitTests;
 
@@ -26,7 +29,7 @@ public void ReportsTheFirstExistingFileInStackTraceWithTheRightLineNumberAndEsca
         // Trim ##. If we keep it, then when the test fails, the assertion failure will get printed to screen and picked up incorrectly by AzDO, because it scans all output for the ##vso... pattern
         var logger = new TextLogger();
         string? text = AzureDevOpsReporter.GetErrorText("MyTestDisplayName", null, error, "severity", new SystemFileSystem(), logger, "net9.0")?.TrimStart('#');
-        Assert.AreEqual("vso[task.logissue type=severity;sourcepath=test/UnitTests/Microsoft.Testing.Extensions.UnitTests/AzureDevOpsTests.cs;linenumber=19;columnnumber=1][MyTestDisplayName] [net9.0] this is an error%0Awith%0Dnewline", text, $"\nLogs:\n{string.Join("\n", logger.Logs)}");
+        Assert.AreEqual("vso[task.logissue type=severity;sourcepath=test/UnitTests/Microsoft.Testing.Extensions.UnitTests/AzureDevOpsTests.cs;linenumber=22;columnnumber=1][MyTestDisplayName] [net9.0] this is an error%0Awith%0Dnewline", text, $"\nLogs:\n{string.Join("\n", logger.Logs)}");
     }
 
     [TestMethod]
@@ -45,7 +48,147 @@ public void ReportsTheFirstExistingFileInStackTraceWithTheRightLineNumberAndEsca
         // Trim ##. If we keep it, then when the test fails, the assertion failure will get printed to screen and picked up incorrectly by AzDO, because it scans all output for the ##vso... pattern
         var logger = new TextLogger();
         string? text = AzureDevOpsReporter.GetErrorText("MyTestDisplayName", "Some custom reason\nwith\rnewline", error, "severity", new SystemFileSystem(), logger, "net9.0")?.TrimStart('#');
-        Assert.AreEqual("vso[task.logissue type=severity;sourcepath=test/UnitTests/Microsoft.Testing.Extensions.UnitTests/AzureDevOpsTests.cs;linenumber=38;columnnumber=1][MyTestDisplayName] [net9.0] Some custom reason%0Awith%0Dnewline", text, $"\nLogs:\n{string.Join("\n", logger.Logs)}");
+        Assert.AreEqual("vso[task.logissue type=severity;sourcepath=test/UnitTests/Microsoft.Testing.Extensions.UnitTests/AzureDevOpsTests.cs;linenumber=41;columnnumber=1][MyTestDisplayName] [net9.0] Some custom reason%0Awith%0Dnewline", text, $"\nLogs:\n{string.Join("\n", logger.Logs)}");
+    }
+
+    [TestMethod]
+    public void SkipsMSTestAssertImplementationFrameInPartialClassFile()
+    {
+        // Regression test for https://github.com/microsoft/testfx/issues/6925.
+        //
+        // The Assert class is split into partial-class files such as Assert.IComparable.cs and
+        // Assert.AreEqual.cs. Their file names do not end with "Assert.cs", so the previous
+        // filename-based skip heuristic missed them and the reporter incorrectly annotated the
+        // framework implementation. The fix matches on the fully-qualified type prefix in the
+        // 'code' capture instead, so every Assert/CollectionAssert/StringAssert partial is
+        // correctly recognized as framework internals.
+        (string userFile, int userLine) = GetCurrentLocation();
+
+        // The Assert.IComparable.cs file actually exists in the repo, so the file-existence
+        // check would happily accept the framework frame if we did not skip it.
+        string stackTrace = string.Join(
+            Environment.NewLine,
+            "   at Microsoft.VisualStudio.TestTools.UnitTesting.Assert.IsLessThan[T](T upperBound, T value, String message) in /_/src/TestFramework/TestFramework/Assertions/Assert.IComparable.cs:line 138",
+            $"   at Microsoft.Testing.Extensions.UnitTests.AzureDevOpsTests.SkipsMSTestAssertImplementationFrameInPartialClassFile() in {userFile}:line {userLine}");
+
+        var error = new SyntheticStackTraceException("boom", stackTrace);
+
+        var logger = new TextLogger();
+        string? text = AzureDevOpsReporter.GetErrorText("MyTestDisplayName", null, error, "severity", new SystemFileSystem(), logger, "net9.0")?.TrimStart('#');
+
+        Assert.AreEqual(
+            $"vso[task.logissue type=severity;sourcepath=test/UnitTests/Microsoft.Testing.Extensions.UnitTests/AzureDevOpsTests.cs;linenumber={userLine};columnnumber=1][MyTestDisplayName] [net9.0] boom",
+            text,
+            $"\nLogs:\n{string.Join("\n", logger.Logs)}");
+    }
+
+    [TestMethod]
+    public void SkipsMSTestCollectionAssertImplementationFrameInPartialClassFile()
+    {
+        (string userFile, int userLine) = GetCurrentLocation();
+
+        string stackTrace = string.Join(
+            Environment.NewLine,
+            "   at Microsoft.VisualStudio.TestTools.UnitTesting.CollectionAssert.AreEqual(ICollection expected, ICollection actual, String message) in /_/src/TestFramework/TestFramework/Assertions/CollectionAssert.Equality.cs:line 42",
+            $"   at Microsoft.Testing.Extensions.UnitTests.AzureDevOpsTests.SkipsMSTestCollectionAssertImplementationFrameInPartialClassFile() in {userFile}:line {userLine}");
+
+        var error = new SyntheticStackTraceException("boom", stackTrace);
+
+        var logger = new TextLogger();
+        string? text = AzureDevOpsReporter.GetErrorText("MyTestDisplayName", null, error, "severity", new SystemFileSystem(), logger, "net9.0")?.TrimStart('#');
+
+        Assert.AreEqual(
+            $"vso[task.logissue type=severity;sourcepath=test/UnitTests/Microsoft.Testing.Extensions.UnitTests/AzureDevOpsTests.cs;linenumber={userLine};columnnumber=1][MyTestDisplayName] [net9.0] boom",
+            text,
+            $"\nLogs:\n{string.Join("\n", logger.Logs)}");
+    }
+
+    [TestMethod]
+    public void SkipsMSTestStringAssertImplementationFrame()
+    {
+        (string userFile, int userLine) = GetCurrentLocation();
+
+        string stackTrace = string.Join(
+            Environment.NewLine,
+            "   at Microsoft.VisualStudio.TestTools.UnitTesting.StringAssert.Contains(String value, String substring, String message) in /_/src/TestFramework/TestFramework/Assertions/StringAssert.cs:line 17",
+            $"   at Microsoft.Testing.Extensions.UnitTests.AzureDevOpsTests.SkipsMSTestStringAssertImplementationFrame() in {userFile}:line {userLine}");
+
+        var error = new SyntheticStackTraceException("boom", stackTrace);
+
+        var logger = new TextLogger();
+        string? text = AzureDevOpsReporter.GetErrorText("MyTestDisplayName", null, error, "severity", new SystemFileSystem(), logger, "net9.0")?.TrimStart('#');
+
+        Assert.AreEqual(
+            $"vso[task.logissue type=severity;sourcepath=test/UnitTests/Microsoft.Testing.Extensions.UnitTests/AzureDevOpsTests.cs;linenumber={userLine};columnnumber=1][MyTestDisplayName] [net9.0] boom",
+            text,
+            $"\nLogs:\n{string.Join("\n", logger.Logs)}");
+    }
+
+    [TestMethod]
+    public void DoesNotSkipUserFrameWhoseFileNameEndsWithAssertCs()
+    {
+        // The previous heuristic skipped any frame whose file path ended with "Assert.cs",
+        // which incorrectly hid user code in files named e.g. MyAssert.cs from PR annotations.
+        // The fix is based on the fully-qualified type name in the frame, so user types are
+        // never confused with the framework's Assert/CollectionAssert/StringAssert types.
+        string repoRoot = RootFinder.Find();
+        string userFile = Path.Combine(repoRoot, "src", "MyCompany", "MyAssert.cs");
+
+        string stackTrace =
+            $"   at MyCompany.Verification.MyAssert.Verify(Object value) in {userFile}:line 17";
+
+        var error = new SyntheticStackTraceException("boom", stackTrace);
+
+        var fileSystem = new Mock<IFileSystem>();
+        fileSystem.Setup(fs => fs.ExistFile(It.IsAny<string>())).Returns(true);
+
+        var logger = new TextLogger();
+        string? text = AzureDevOpsReporter.GetErrorText("MyTestDisplayName", null, error, "severity", fileSystem.Object, logger, "net9.0")?.TrimStart('#');
+
+        Assert.AreEqual(
+            "vso[task.logissue type=severity;sourcepath=src/MyCompany/MyAssert.cs;linenumber=17;columnnumber=1][MyTestDisplayName] [net9.0] boom",
+            text,
+            $"\nLogs:\n{string.Join("\n", logger.Logs)}");
+    }
+
+    [TestMethod]
+    public void DoesNotSkipUserFrameWhoseTypeNameStartsWithAssert()
+    {
+        // A user type literally named "Assert" (e.g. MyCompany.Tests.Assert) must not be
+        // mistaken for Microsoft.VisualStudio.TestTools.UnitTesting.Assert. The prefix check
+        // is anchored on the full MSTest namespace, so user types in other namespaces are safe.
+        string repoRoot = RootFinder.Find();
+        string userFile = Path.Combine(repoRoot, "src", "MyCompany", "MyAssertions.cs");
+
+        string stackTrace =
+            $"   at MyCompany.Tests.Assert.Equal[T](T expected, T actual) in {userFile}:line 25";
+
+        var error = new SyntheticStackTraceException("boom", stackTrace);
+
+        var fileSystem = new Mock<IFileSystem>();
+        fileSystem.Setup(fs => fs.ExistFile(It.IsAny<string>())).Returns(true);
+
+        var logger = new TextLogger();
+        string? text = AzureDevOpsReporter.GetErrorText("MyTestDisplayName", null, error, "severity", fileSystem.Object, logger, "net9.0")?.TrimStart('#');
+
+        Assert.AreEqual(
+            "vso[task.logissue type=severity;sourcepath=src/MyCompany/MyAssertions.cs;linenumber=25;columnnumber=1][MyTestDisplayName] [net9.0] boom",
+            text,
+            $"\nLogs:\n{string.Join("\n", logger.Logs)}");
+    }
+
+    private static (string FilePath, int LineNumber) GetCurrentLocation(
+        [CallerFilePath] string? filePath = null,
+        [CallerLineNumber] int lineNumber = 0)
+        => (filePath!, lineNumber);
+
+    private sealed class SyntheticStackTraceException : Exception
+    {
+        public SyntheticStackTraceException(string message, string stackTrace)
+            : base(message)
+            => StackTrace = stackTrace;
+
+        public override string? StackTrace { get; }
     }
 
     private class TextLogger : ILogger