Skip to content

Commit 6e2b787

Browse files
nohwndCopilot
nohwnd
and
Copilot
committed
Remove Debug/Release line number branching from tests
Add SourceAssert helper that reads actual source files to validate PDB-reported line numbers, replacing hardcoded Debug/Release conditional checks. The helper finds method declarations (including attributes) and body ranges in source files, making line number assertions work identically in both configurations. Changes: - New SourceAssert.cs with LineIsWithinMethod, GetMethodRange, GetAllMethodRanges, and FindSourceFile methods - DiaSessionTests: removed ValidateMinLineNumber/ValidateLineNumbers helpers, using SourceAssert instead - DifferentTestFrameworkSimpleTests: removed Debug/Release branching for NUnit and xUnit adapter line assertions - DiscoverTests: removed dead Debug/Release branching Fixes microsoft#15458 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5f75c30 commit 6e2b787

File tree

5 files changed

+403
-76
lines changed

5 files changed

+403
-76
lines changed

‎test/Microsoft.TestPlatform.Common.UnitTests/SourceNavigationParserTests.cs‎

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using Microsoft.TestPlatform.TestUtilities;
5+
using Microsoft.VisualStudio.TestTools.UnitTesting;
6+
7+
namespace Microsoft.TestPlatform.Common.UnitTests;
8+
9+
[TestClass]
10+
public class SourceNavigationParserTests
11+
{
12+
[TestMethod]
13+
public void FindMethodBodyStartLines_SimpleMethod()
14+
{
15+
var lines = new[]
16+
{
17+
"public class Foo",
18+
"{",
19+
" public void MyMethod()",
20+
" {",
21+
" DoStuff();",
22+
" }",
23+
"}",
24+
};
25+
26+
var result = SourceNavigationParser.FindMethodBodyStartLines(lines, "MyMethod");
27+
28+
Assert.AreEqual(1, result.Count);
29+
Assert.AreEqual(4, result[0]); // 1-based: line " {"
30+
}
31+
32+
[TestMethod]
33+
public void FindMethodBodyStartLines_MethodWithAttribute()
34+
{
35+
var lines = new[]
36+
{
37+
"public class Foo",
38+
"{",
39+
" [Test]",
40+
" public void PassTestMethod1()",
41+
" {",
42+
" Assert.AreEqual(5, 5);",
43+
" }",
44+
"}",
45+
};
46+
47+
var result = SourceNavigationParser.FindMethodBodyStartLines(lines, "PassTestMethod1");
48+
49+
Assert.AreEqual(1, result.Count);
50+
Assert.AreEqual(5, result[0]); // 1-based: line " {"
51+
}
52+
53+
[TestMethod]
54+
public void FindMethodBodyStartLines_BraceOnSameLineAsSignature()
55+
{
56+
var lines = new[]
57+
{
58+
"public class Foo",
59+
"{",
60+
" public void Inline() {",
61+
" }",
62+
"}",
63+
};
64+
65+
var result = SourceNavigationParser.FindMethodBodyStartLines(lines, "Inline");
66+
67+
Assert.AreEqual(1, result.Count);
68+
Assert.AreEqual(3, result[0]); // 1-based: brace is on same line as signature
69+
}
70+
71+
[TestMethod]
72+
public void FindMethodBodyStartLines_OverloadedMethods()
73+
{
74+
var lines = new[]
75+
{
76+
"public class Foo",
77+
"{",
78+
" public void OverLoaded()",
79+
" {",
80+
" }",
81+
"",
82+
" public void OverLoaded(string _)",
83+
" {",
84+
" }",
85+
"}",
86+
};
87+
88+
var result = SourceNavigationParser.FindMethodBodyStartLines(lines, "OverLoaded");
89+
90+
Assert.AreEqual(2, result.Count);
91+
Assert.AreEqual(4, result[0]); // first overload brace
92+
Assert.AreEqual(8, result[1]); // second overload brace
93+
}
94+
95+
[TestMethod]
96+
public void FindMethodBodyStartLines_MethodNotFound()
97+
{
98+
var lines = new[]
99+
{
100+
"public class Foo",
101+
"{",
102+
" public void Other()",
103+
" {",
104+
" }",
105+
"}",
106+
};
107+
108+
var result = SourceNavigationParser.FindMethodBodyStartLines(lines, "NotExist");
109+
110+
Assert.AreEqual(0, result.Count);
111+
}
112+
113+
[TestMethod]
114+
public void FindMethodBodyStartLines_DoesNotMatchPropertyOrField()
115+
{
116+
var lines = new[]
117+
{
118+
"public class Foo",
119+
"{",
120+
" public string MyMethod = \"hello\";",
121+
" public string MyMethodProp { get; set; }",
122+
"}",
123+
};
124+
125+
// "MyMethod" followed by ' =' should not match (no '(' after name).
126+
var result = SourceNavigationParser.FindMethodBodyStartLines(lines, "MyMethod");
127+
128+
Assert.AreEqual(0, result.Count);
129+
}
130+
131+
[TestMethod]
132+
public void FindMethodBodyStartLines_AsyncMethod()
133+
{
134+
var lines = new[]
135+
{
136+
"public class Foo",
137+
"{",
138+
" public async Task AsyncTestMethod()",
139+
" {",
140+
" await Task.Delay(0);",
141+
" }",
142+
"}",
143+
};
144+
145+
var result = SourceNavigationParser.FindMethodBodyStartLines(lines, "AsyncTestMethod");
146+
147+
Assert.AreEqual(1, result.Count);
148+
Assert.AreEqual(4, result[0]);
149+
}
150+
151+
[TestMethod]
152+
public void FindMethodBodyStartLines_RealSimpleClassLibrary()
153+
{
154+
// Mimics test/TestAssets/SimpleClassLibrary/Class1.cs
155+
var lines = new[]
156+
{
157+
"// Copyright header",
158+
"",
159+
"using System.Threading.Tasks;",
160+
"",
161+
"namespace SimpleClassLibrary",
162+
"{",
163+
" public class Class1",
164+
" {",
165+
" public void PassingTest()",
166+
" {", // line 10
167+
" if (new System.Random().Next() == 20) { throw new System.NotImplementedException(); }",
168+
" }",
169+
"",
170+
" public async Task AsyncTestMethod()",
171+
" {", // line 15
172+
" await Task.Delay(0);",
173+
" }",
174+
"",
175+
" public void OverLoadedMethod()",
176+
" {", // line 20
177+
" }",
178+
"",
179+
" public void OverLoadedMethod(string _)",
180+
" {", // line 24
181+
" }",
182+
" }",
183+
"}",
184+
};
185+
186+
Assert.AreEqual(10, SourceNavigationParser.FindMethodBodyStartLines(lines, "PassingTest")[0]);
187+
Assert.AreEqual(15, SourceNavigationParser.FindMethodBodyStartLines(lines, "AsyncTestMethod")[0]);
188+
189+
var overloads = SourceNavigationParser.FindMethodBodyStartLines(lines, "OverLoadedMethod");
190+
Assert.AreEqual(2, overloads.Count);
191+
Assert.AreEqual(20, overloads[0]);
192+
Assert.AreEqual(24, overloads[1]);
193+
}
194+
195+
[TestMethod]
196+
public void ContainsMethodSignature_MatchesMethodFollowedByParen()
197+
{
198+
Assert.IsTrue(SourceNavigationParser.ContainsMethodSignature(" public void MyMethod()", "MyMethod"));
199+
}
200+
201+
[TestMethod]
202+
public void ContainsMethodSignature_MatchesWithWhitespaceBeforeParen()
203+
{
204+
Assert.IsTrue(SourceNavigationParser.ContainsMethodSignature(" public void MyMethod ()", "MyMethod"));
205+
}
206+
207+
[TestMethod]
208+
public void ContainsMethodSignature_DoesNotMatchFieldAssignment()
209+
{
210+
Assert.IsFalse(SourceNavigationParser.ContainsMethodSignature(" public string MyMethod = \"hello\";", "MyMethod"));
211+
}
212+
213+
[TestMethod]
214+
public void ContainsMethodSignature_DoesNotMatchSubstring()
215+
{
216+
// "MyMethodExtended(" should not match "MyMethod" because the next char after "MyMethod" is 'E', not '(' or whitespace.
217+
Assert.IsFalse(SourceNavigationParser.ContainsMethodSignature(" public void MyMethodExtended()", "MyMethod"));
218+
}
219+
220+
[TestMethod]
221+
public void ContainsMethodSignature_MatchesWhenNameAppearsMultipleTimes()
222+
{
223+
// First occurrence is a field, second is the method.
224+
Assert.IsTrue(SourceNavigationParser.ContainsMethodSignature(" // calls MyMethod then MyMethod()", "MyMethod"));
225+
}
226+
}

‎test/Microsoft.TestPlatform.Library.IntegrationTests/DiaSessionTests.cs‎

Lines changed: 9 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
using System;
54
using System.Diagnostics;
65

76
using Microsoft.TestPlatform.TestUtilities;
@@ -38,8 +37,7 @@ public void GetNavigationDataShouldReturnCorrectFileNameAndLineNumber()
3837
Assert.IsNotNull(diaNavigationData, "Failed to get navigation data");
3938
StringAssert.EndsWith(diaNavigationData.FileName!.Replace("\\", "/"), @"\SimpleClassLibrary\Class1.cs".Replace("\\", "/"));
4039

41-
ValidateMinLineNumber(11, diaNavigationData.MinLineNumber);
42-
Assert.AreEqual(13, diaNavigationData.MaxLineNumber, "Incorrect max line number");
40+
SourceAssert.LineIsAtMethodBodyStart(diaNavigationData.FileName!, "PassingTest", diaNavigationData.MinLineNumber, "Incorrect min line number");
4341

4442
_testEnvironment.TargetFramework = currentTargetFrameWork;
4543
}
@@ -56,8 +54,8 @@ public void GetNavigationDataShouldReturnCorrectDataForAsyncMethod()
5654
Assert.IsNotNull(diaNavigationData, "Failed to get navigation data");
5755
StringAssert.EndsWith(diaNavigationData.FileName!.Replace("\\", "/"), @"\SimpleClassLibrary\Class1.cs".Replace("\\", "/"));
5856

59-
ValidateMinLineNumber(16, diaNavigationData.MinLineNumber);
60-
Assert.AreEqual(18, diaNavigationData.MaxLineNumber, "Incorrect max line number");
57+
// The async state machine's MoveNext maps back to the original async method source lines.
58+
SourceAssert.LineIsAtMethodBodyStart(diaNavigationData.FileName!, "AsyncTestMethod", diaNavigationData.MinLineNumber, "Incorrect min line number");
6159

6260
_testEnvironment.TargetFramework = currentTargetFrameWork;
6361
}
@@ -74,9 +72,9 @@ public void GetNavigationDataShouldReturnCorrectDataForOverLoadedMethod()
7472
Assert.IsNotNull(diaNavigationData, "Failed to get navigation data");
7573
StringAssert.EndsWith(diaNavigationData.FileName!.Replace("\\", "/"), @"\SimpleClassLibrary\Class1.cs".Replace("\\", "/"));
7674

77-
// Weird why DiaSession is now returning the first overloaded method
78-
// as compared to before when it used to return second method
79-
ValidateLineNumbers(diaNavigationData.MinLineNumber, diaNavigationData.MaxLineNumber);
75+
// DiaSession may return any overload; verify min line falls within one of them.
76+
SourceAssert.LineIsAtMethodBodyStart(diaNavigationData.FileName!, "OverLoadedMethod", diaNavigationData.MinLineNumber,
77+
$"Min line number ({diaNavigationData.MinLineNumber}) is not at the body start of any OverLoadedMethod overload.");
8078

8179
_testEnvironment.TargetFramework = currentTargetFrameWork;
8280
}
@@ -114,48 +112,12 @@ public void DiaSessionPerfTest()
114112

115113
Assert.IsNotNull(diaNavigationData, "Failed to get navigation data");
116114
StringAssert.EndsWith(diaNavigationData.FileName!.Replace("\\", "/"), @"\SimpleClassLibrary\HugeMethodSet.cs".Replace("\\", "/"));
117-
ValidateMinLineNumber(9, diaNavigationData.MinLineNumber);
118-
Assert.AreEqual(10, diaNavigationData.MaxLineNumber);
115+
116+
SourceAssert.LineIsAtMethodBodyStart(diaNavigationData.FileName!, "MSTest_D1_01", diaNavigationData.MinLineNumber, "Incorrect min line number");
117+
119118
var expectedTime = 150;
120119
Assert.IsTrue(watch.Elapsed.Milliseconds < expectedTime, $"DiaSession Perf test Actual time:{watch.Elapsed.Milliseconds} ms Expected time:{expectedTime} ms");
121120

122121
_testEnvironment.TargetFramework = currentTargetFrameWork;
123122
}
124-
125-
private static void ValidateLineNumbers(int min, int max)
126-
{
127-
// Release builds optimize code, hence min line numbers are different.
128-
if (IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase))
129-
{
130-
Assert.AreEqual(min, max, "Incorrect min line number");
131-
}
132-
else
133-
{
134-
if (max == 22)
135-
{
136-
Assert.AreEqual(min + 1, max, "Incorrect min line number");
137-
}
138-
else if (max == 26)
139-
{
140-
Assert.AreEqual(min + 1, max, "Incorrect min line number");
141-
}
142-
else
143-
{
144-
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}.");
145-
}
146-
}
147-
}
148-
149-
private static void ValidateMinLineNumber(int expected, int actual)
150-
{
151-
// Release builds optimize code, hence min line numbers are different.
152-
if (IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase))
153-
{
154-
Assert.AreEqual(expected + 1, actual, "Incorrect min line number");
155-
}
156-
else
157-
{
158-
Assert.AreEqual(expected, actual, "Incorrect min line number");
159-
}
160-
}
161123
}

‎test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/DifferentTestFrameworkSimpleTests.cs‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
using System;
54
using System.Collections.Generic;
65
using System.Diagnostics.CodeAnalysis;
76
using System.IO;
@@ -64,15 +63,8 @@ public void RunTestsWithNunitAdapter(RunnerInfo runnerInfo)
6463
Assert.AreEqual(1, _runEventHandler.TestResults.Count(t => t.Outcome == TestOutcome.Passed), _runEventHandler.ToString());
6564
Assert.AreEqual(1, _runEventHandler.TestResults.Count(t => t.Outcome == TestOutcome.Failed), _runEventHandler.ToString());
6665

67-
// Release builds optimize code, hence line numbers are different.
68-
if (IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase))
69-
{
70-
Assert.AreEqual(14, testCase.First().TestCase.LineNumber);
71-
}
72-
else
73-
{
74-
Assert.AreEqual(13, testCase.First().TestCase.LineNumber);
75-
}
66+
var nunitSourceFile = SourceAssert.FindSourceFile(GetAssetFullPath("NUTestProject.dll"), "PassTestMethod1");
67+
SourceAssert.LineIsAtMethodBodyStart(nunitSourceFile, "PassTestMethod1", testCase.First().TestCase.LineNumber);
7668
}
7769

7870
[TestMethod]
@@ -107,15 +99,8 @@ public void RunTestsWithXunitAdapter(RunnerInfo runnerInfo)
10799
Assert.AreEqual(1, _runEventHandler.TestResults.Count(t => t.Outcome == TestOutcome.Passed), _runEventHandler.ToString());
108100
Assert.AreEqual(1, _runEventHandler.TestResults.Count(t => t.Outcome == TestOutcome.Failed), _runEventHandler.ToString());
109101

110-
// Release builds optimize code, hence line numbers are different.
111-
if (IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase))
112-
{
113-
Assert.AreEqual(15, testCase.First().TestCase.LineNumber);
114-
}
115-
else
116-
{
117-
Assert.AreEqual(14, testCase.First().TestCase.LineNumber);
118-
}
102+
var xunitSourceFile = SourceAssert.FindSourceFile(testAssemblyPath, "PassTestMethod1");
103+
SourceAssert.LineIsAtMethodBodyStart(xunitSourceFile, "PassTestMethod1", testCase.First().TestCase.LineNumber);
119104
}
120105

121106
[TestMethod]

‎test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/DiscoverTests.cs‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -199,16 +199,8 @@ public void DiscoverTestsUsingSourceNavigation(RunnerInfo runnerInfo)
199199
var testCase =
200200
_discoveryEventHandler.DiscoveredTestCases.Where(dt => dt.FullyQualifiedName.Equals("SampleUnitTestProject.UnitTest1.PassingTest"));
201201

202-
// Release builds optimize code, hence line numbers are different.
203-
if (IntegrationTestEnvironment.BuildConfiguration.StartsWith("release", StringComparison.OrdinalIgnoreCase))
204-
{
205-
Assert.AreEqual(22, testCase.First().LineNumber);
206-
}
207-
else
208-
{
209-
// TODO: I changed this because it differs in .net testhost, is this condition still needed?
210-
Assert.AreEqual(22, testCase.First().LineNumber);
211-
}
202+
var sourceFile = SourceAssert.FindSourceFile(GetAssetFullPath("SimpleTestProject.dll"), "PassingTest");
203+
SourceAssert.LineIsWithinMethod(sourceFile, "PassingTest", testCase.First().LineNumber);
212204
}
213205

214206
[TestMethod]

0 commit comments

Comments
 (0)