Skip to content

Derive line numbers from source in DiaSession and adapter tests #15528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nohwnd wants to merge 2 commits into from
Closed

Derive line numbers from source in DiaSession and adapter tests #15528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4e1700
Derive line numbers from source in DiaSession and adapter tests
nohwnd Mar 19, 2026
aa9abff
Apply suggestions from code review
nohwnd Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 74 additions & 37 deletions test/Microsoft.TestPlatform.Library.IntegrationTests/DiaSessionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Diagnostics;
using System.IO;

using Microsoft.TestPlatform.TestUtilities;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
Expand Down Expand Up @@ -38,8 +39,15 @@ public void GetNavigationDataShouldReturnCorrectFileNameAndLineNumber()
Assert.IsNotNull(diaNavigationData, "Failed to get navigation data");
StringAssert.EndsWith(diaNavigationData.FileName!.Replace("\\", "/"), @"\SimpleClassLibrary\Class1.cs".Replace("\\", "/"));

ValidateMinLineNumber(11, diaNavigationData.MinLineNumber);
Assert.AreEqual(13, diaNavigationData.MaxLineNumber, "Incorrect max line number");
// Derive expected line numbers from the source file so the test is resilient to code moving around.
var sourceFile = Path.Combine(_testEnvironment.TestAssetsPath, "SimpleClassLibrary", "Class1.cs");
var (bodyStart, bodyEnd) = FindMethodBodyRange(sourceFile, "PassingTest");
// In Debug, PDB min line points to opening brace. In Release, it points to first executable statement.
var expectedMin = IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase)
? bodyStart + 1
: bodyStart;
Assert.AreEqual(expectedMin, diaNavigationData.MinLineNumber, "Incorrect min line number");
Assert.AreEqual(bodyEnd, diaNavigationData.MaxLineNumber, "Incorrect max line number");
Comment on lines +42 to +50
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Release builds, expectedMin = bodyStart + 1 has the same fragility as a hardcoded line number: it assumes the first executable statement is exactly one line after {. If the method body begins with comments/blank lines/directives, this will be wrong. Consider deriving the first executable line by scanning forward from bodyStart (similar to FindFirstExecutableLine in the adapter tests) and use that as expectedMin for Release.

Copilot uses AI. Check for mistakes.

_testEnvironment.TargetFramework = currentTargetFrameWork;
}
Expand All @@ -56,8 +64,13 @@ public void GetNavigationDataShouldReturnCorrectDataForAsyncMethod()
Assert.IsNotNull(diaNavigationData, "Failed to get navigation data");
StringAssert.EndsWith(diaNavigationData.FileName!.Replace("\\", "/"), @"\SimpleClassLibrary\Class1.cs".Replace("\\", "/"));

ValidateMinLineNumber(16, diaNavigationData.MinLineNumber);
Assert.AreEqual(18, diaNavigationData.MaxLineNumber, "Incorrect max line number");
var sourceFile = Path.Combine(_testEnvironment.TestAssetsPath, "SimpleClassLibrary", "Class1.cs");
var (bodyStart, bodyEnd) = FindMethodBodyRange(sourceFile, "AsyncTestMethod");
var expectedMin = IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase)
? bodyStart + 1
: bodyStart;
Assert.AreEqual(expectedMin, diaNavigationData.MinLineNumber, "Incorrect min line number");
Assert.AreEqual(bodyEnd, diaNavigationData.MaxLineNumber, "Incorrect max line number");

_testEnvironment.TargetFramework = currentTargetFrameWork;
}
Expand All @@ -76,7 +89,18 @@ public void GetNavigationDataShouldReturnCorrectDataForOverLoadedMethod()

// Weird why DiaSession is now returning the first overloaded method
// as compared to before when it used to return second method
ValidateLineNumbers(diaNavigationData.MinLineNumber, diaNavigationData.MaxLineNumber);
var sourceFile = Path.Combine(_testEnvironment.TestAssetsPath, "SimpleClassLibrary", "Class1.cs");
var (bodyStart, bodyEnd) = FindMethodBodyRange(sourceFile, "OverLoadedMethod");
if (IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase))
{
// Release builds for empty methods have min == max (closing brace is the only sequence point).
Assert.AreEqual(diaNavigationData.MinLineNumber, diaNavigationData.MaxLineNumber, "Incorrect min line number");
}
else
{
Assert.AreEqual(bodyStart, diaNavigationData.MinLineNumber, "Incorrect min line number");
Assert.AreEqual(bodyEnd, diaNavigationData.MaxLineNumber, "Incorrect max line number");
}
Comment on lines +92 to +103
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the Release branch, the assertion only checks min == max, but does not validate which line that is. This can let real regressions slip through (e.g., DIA returning a completely unrelated line number that happens to be equal for min/max). Since you already compute (bodyStart, bodyEnd), assert that MinLineNumber/MaxLineNumber equals the expected sequence point line (often the closing brace for empty methods), or at least that it falls within [bodyStart, bodyEnd].

Copilot uses AI. Check for mistakes.

_testEnvironment.TargetFramework = currentTargetFrameWork;
}
Expand Down Expand Up @@ -111,51 +135,64 @@ public void DiaSessionPerfTest()
var diaSession = new DiaSession(assemblyPath);
DiaNavigationData? diaNavigationData = diaSession.GetNavigationData("SimpleClassLibrary.HugeMethodSet", "MSTest_D1_01");
watch.Stop();
var diaElapsedMilliseconds = watch.ElapsedMilliseconds;

Assert.IsNotNull(diaNavigationData, "Failed to get navigation data");
StringAssert.EndsWith(diaNavigationData.FileName!.Replace("\\", "/"), @"\SimpleClassLibrary\HugeMethodSet.cs".Replace("\\", "/"));
ValidateMinLineNumber(9, diaNavigationData.MinLineNumber);
Assert.AreEqual(10, diaNavigationData.MaxLineNumber);

var sourceFile = Path.Combine(_testEnvironment.TestAssetsPath, "SimpleClassLibrary", "HugeMethodSet.cs");
var (bodyStart, bodyEnd) = FindMethodBodyRange(sourceFile, "MSTest_D1_01");
var expectedMin = IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase)
? bodyStart + 1
: bodyStart;
Assert.AreEqual(expectedMin, diaNavigationData.MinLineNumber, "Incorrect min line number");
Assert.AreEqual(bodyEnd, diaNavigationData.MaxLineNumber, "Incorrect max line number");

var expectedTime = 150;
Assert.IsTrue(watch.Elapsed.Milliseconds < expectedTime, $"DiaSession Perf test Actual time:{watch.Elapsed.Milliseconds} ms Expected time:{expectedTime} ms");
Assert.IsTrue(diaElapsedMilliseconds < expectedTime, $"DiaSession Perf test Actual time:{diaElapsedMilliseconds} ms Expected time:{expectedTime} ms");

_testEnvironment.TargetFramework = currentTargetFrameWork;
}

private static void ValidateLineNumbers(int min, int max)
/// <summary>
/// Finds the 1-based line numbers of the opening brace and closing brace of a method body.
/// </summary>
private static (int BodyStart, int BodyEnd) FindMethodBodyRange(string sourceFile, string methodName)
{
// Release builds optimize code, hence min line numbers are different.
if (IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase))
{
Assert.AreEqual(min, max, "Incorrect min line number");
}
else
var lines = File.ReadAllLines(sourceFile);
for (int i = 0; i < lines.Length; i++)
{
if (max == 22)
if (lines[i].Contains($" {methodName}("))
{
Assert.AreEqual(min + 1, max, "Incorrect min line number");
}
else if (max == 26)
{
Assert.AreEqual(min + 1, max, "Incorrect min line number");
}
else
{
Assert.Fail($"Incorrect min/max line number. Expected Max to be 22 or 26. And Min to be 21 or 25. But Min was {min}, and Max was {max}.");
int braceDepth = 0;
int bodyStart = -1;
for (int j = i; j < lines.Length; j++)
{
var line = lines[j];
foreach (var ch in line)
{
if (ch == '{')
{
if (bodyStart == -1)
{
bodyStart = j + 1; // 1-based
}

braceDepth++;
}
else if (ch == '}')
{
braceDepth--;
if (braceDepth == 0)
{
return (bodyStart, j + 1); // 1-based
}
}
}
}
}
}
}

private static void ValidateMinLineNumber(int expected, int actual)
{
// Release builds optimize code, hence min line numbers are different.
if (IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase))
{
Assert.AreEqual(expected + 1, actual, "Incorrect min line number");
}
else
{
Assert.AreEqual(expected, actual, "Incorrect min line number");
}
throw new InvalidOperationException($"Could not find method '{methodName}' in '{sourceFile}'.");
}
Comment on lines +160 to 197
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method locator/parsing here is quite brittle: Contains($\" {methodName}(\") can miss valid declarations (e.g., start-of-line, different spacing, generic methods) and can match non-declarations (comments/strings). The brace counter also counts braces inside comments/strings, which can produce incorrect ranges. Since these helpers are specifically to make tests resilient, it’s worth making them a bit more robust by (a) using a regex to identify a method declaration more reliably (as done in DifferentTestFrameworkSimpleTests), and (b) skipping commented lines (at least // ...) when searching, similar to the other helper.

Copilot uses AI. Check for mistakes.
}
92 changes: 80 additions & 12 deletions ...tform.Library.IntegrationTests/TranslationLayerTests/DifferentTestFrameworkSimpleTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;

using Microsoft.TestPlatform.Library.IntegrationTests.TranslationLayerTests.EventHandler;
using Microsoft.TestPlatform.TestUtilities;
Expand Down Expand Up @@ -39,6 +40,36 @@ public void Cleanup()
}


private static int FindFirstExecutableLine(string filePath, int methodBodyOpeningBraceLine)
{
// methodBodyOpeningBraceLine is 1-based. Start scanning from the next line.
var lines = File.ReadAllLines(filePath);
for (int i = methodBodyOpeningBraceLine; i < lines.Length; i++)
{
var trimmed = lines[i].Trim();

if (trimmed.Length == 0)
{
continue;
}

if (trimmed.StartsWith("//", StringComparison.Ordinal))
{
continue;
}

if (trimmed.StartsWith("#", StringComparison.Ordinal))
{
continue;
}

return i + 1; // Convert 0-based index to 1-based line number.
}

// Fallback to the original opening brace line if no executable line is found.
return methodBodyOpeningBraceLine;
}

[TestMethod]
[NetCoreTargetFrameworkDataSource]
public void RunTestsWithNunitAdapter(RunnerInfo runnerInfo)
Expand All @@ -64,15 +95,18 @@ public void RunTestsWithNunitAdapter(RunnerInfo runnerInfo)
Assert.AreEqual(1, _runEventHandler.TestResults.Count(t => t.Outcome == TestOutcome.Passed), _runEventHandler.ToString());
Assert.AreEqual(1, _runEventHandler.TestResults.Count(t => t.Outcome == TestOutcome.Failed), _runEventHandler.ToString());

// Release builds optimize code, hence line numbers are different.
// Derive expected line number from source so the test is resilient to code moving around.
var sourceFilePath = Path.Combine(_testEnvironment.TestAssetsPath, "NUTestProject", "Class1.cs");
var expectedLine = FindMethodBodyStartLine(
sourceFilePath,
"PassTestMethod1");
// In Release, PDB sequence points start at the first executable statement, not the opening brace.
if (IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase))
{
Assert.AreEqual(14, testCase.First().TestCase.LineNumber);
}
else
{
Assert.AreEqual(13, testCase.First().TestCase.LineNumber);
expectedLine = FindFirstExecutableLine(sourceFilePath, expectedLine);
}

Assert.AreEqual(expectedLine, testCase.First().TestCase.LineNumber);
}

[TestMethod]
Expand Down Expand Up @@ -107,15 +141,17 @@ public void RunTestsWithXunitAdapter(RunnerInfo runnerInfo)
Assert.AreEqual(1, _runEventHandler.TestResults.Count(t => t.Outcome == TestOutcome.Passed), _runEventHandler.ToString());
Assert.AreEqual(1, _runEventHandler.TestResults.Count(t => t.Outcome == TestOutcome.Failed), _runEventHandler.ToString());

// Release builds optimize code, hence line numbers are different.
// Derive expected line number from source so the test is resilient to code moving around.
var expectedLine = FindMethodBodyStartLine(
Path.Combine(_testEnvironment.TestAssetsPath, "XUTestProject", "Class1.cs"),
"PassTestMethod1");
// In Release, PDB sequence points start at first executable statement, not the opening brace.
if (IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase))
{
Assert.AreEqual(15, testCase.First().TestCase.LineNumber);
}
else
{
Assert.AreEqual(14, testCase.First().TestCase.LineNumber);
expectedLine++;
}

Assert.AreEqual(expectedLine, testCase.First().TestCase.LineNumber);
Comment on lines +144 to +154
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expectedLine++ assumes the first executable statement is always exactly one line after the opening brace. This will break if the method starts with a blank line, comment, #if, or any non-executable line. Use the same approach as the NUnit test (scan for the first executable line) so Release builds remain resilient to source edits.

Copilot uses AI. Check for mistakes.
}

[TestMethod]
Expand Down Expand Up @@ -148,4 +184,36 @@ public void RunTestsWithNonDllAdapter(RunnerInfo runnerInfo)
Assert.AreEqual(1, _runEventHandler.TestResults.Count(t => t.Outcome == TestOutcome.Passed), _runEventHandler.ToString());
Assert.AreEqual(0, _runEventHandler.TestResults.Count(t => t.Outcome == TestOutcome.Failed), _runEventHandler.ToString());
}

/// <summary>
/// Finds the 1-based line number of the opening brace '{' after a method declaration.
/// </summary>
private static int FindMethodBodyStartLine(string sourceFile, string methodName)
{
var lines = File.ReadAllLines(sourceFile);
var pattern = $@"(?<!\.)\b{Regex.Escape(methodName)}\s*\(";

for (int i = 0; i < lines.Length; i++)
{
var trimmedLine = lines[i].TrimStart();
if (trimmedLine.StartsWith("//", StringComparison.Ordinal))
{
continue;
}

if (Regex.IsMatch(lines[i], pattern))
{
// Find the opening brace after the method declaration.
for (int j = i; j < lines.Length; j++)
{
if (lines[j].Contains('{'))
{
return j + 1; // 1-based
}
}
}
}

throw new InvalidOperationException($"Could not find method '{methodName}' in '{sourceFile}'.");
}
}