Skip to content

Commit 3c00f33

Browse files
committed
Address review comments
1 parent 98b808a commit 3c00f33

File tree

6 files changed

+48
-27
lines changed

6 files changed

+48
-27
lines changed

docs/core/testing/mstest-analyzers/mstest0056.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ A test method attribute uses a string constructor argument instead of the <xref:
3333

3434
## Rule description
3535

36-
When specifying a custom display name for a test method, you should use the <xref:Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute.DisplayName> property instead of passing a string as a constructor argument. This ensures consistent usage across the MSTest framework and improves code readability.
36+
When specifying a custom display name for a test method, you should use the <xref:Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute.DisplayName> property instead of passing a string as a constructor argument. In MSTest 4.0 and later, the string constructor argument is used for source information (caller file path) rather than display name, which is a breaking change from MSTest 3.x. Using the `DisplayName` property ensures your code works correctly and avoids potential confusion.
3737

3838
```csharp
3939
[TestClass]
@@ -65,7 +65,7 @@ public class TestClass
6565

6666
## When to suppress warnings
6767

68-
Do not suppress warnings from this rule. Using the `DisplayName` property is the correct and recommended way to specify custom display names for test methods.
68+
Do not suppress warnings from this rule. Using the `DisplayName` property is the only way to specify custom display names for test methods. If your intent is to explicitly pass a value for the caller file path parameter instead of setting a display name, you can suppress this warning, but this is rarely needed.
6969

7070
## Suppress a warning
7171

docs/core/testing/mstest-analyzers/mstest0057.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ A custom <xref:Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute>
3333

3434
## Rule description
3535

36-
When creating custom test method attributes that derive from <xref:Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute>, you should propagate source information using caller information attributes. This allows MSTest to correctly track the source file and line number for test methods, improving diagnostics and test result reporting.
36+
When creating custom test method attributes that derive from <xref:Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute>, you should propagate source information using caller information attributes. This allows MSTest to correctly track the source file and line number for test methods, improving diagnostics, test result reporting, and Test Explorer behavior.
3737

3838
```csharp
3939
public class MyTestMethodAttribute : TestMethodAttribute

docs/core/testing/mstest-analyzers/mstest0058.md

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,34 +49,27 @@ public class TestClass
4949
// Code that might throw.
5050
DoSomethingThatMightThrow();
5151
}
52-
catch
52+
catch (Exception ex)
5353
{
54-
Assert.Fail("Exception was thrown"); // Violation
54+
Assert.AreEqual("Expected error message", ex.Message); // Violation
5555
}
5656
}
5757
}
5858
```
5959

6060
## How to fix violations
6161

62-
Use `Assert.ThrowsException` or related assertion methods to test for exceptions.
62+
Use `Assert.Throws`, `Assert.ThrowsExactly` or related assertion methods to test for exceptions.
6363

6464
```csharp
6565
[TestClass]
6666
public class TestClass
6767
{
6868
[TestMethod]
69-
public void TestMethod_ThrowsException()
69+
public void TestMethod_ThrowsExceptionWithExpectedMessage()
7070
{
71-
Assert.Throws<InvalidOperationException>(() => DoSomethingThatMightThrow());
72-
}
73-
74-
[TestMethod]
75-
public void TestMethod_DoesNotThrow()
76-
{
77-
// If no exception is expected, just call the method directly
78-
DoSomethingThatMightNotThrow();
79-
// Add positive assertions here
71+
InvalidOperationException exception = Assert.Throws<InvalidOperationException>(() => DoSomethingThatMightThrow());
72+
Assert.AreEqual("Expected error message", exception.Message);
8073
}
8174
}
8275
```

docs/core/testing/mstest-analyzers/mstest0059.md

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@ An assembly contains both <xref:Microsoft.VisualStudio.TestTools.UnitTesting.Par
3333

3434
## Rule description
3535

36-
The <xref:Microsoft.VisualStudio.TestTools.UnitTesting.ParallelizeAttribute> and <xref:Microsoft.VisualStudio.TestTools.UnitTesting.DoNotParallelizeAttribute> attributes are mutually exclusive. Having both attributes in the same assembly creates conflicting configuration that can lead to unpredictable test execution behavior. You should choose one parallelization strategy for your test assembly.
36+
The <xref:Microsoft.VisualStudio.TestTools.UnitTesting.ParallelizeAttribute> and <xref:Microsoft.VisualStudio.TestTools.UnitTesting.DoNotParallelizeAttribute> attributes are mutually exclusive at the assembly level. When both attributes are applied to the same assembly, tests run sequentially. This conflicting configuration indicates unclear intent and should be resolved by choosing one parallelization strategy for your test assembly.
3737

3838
```csharp
3939
using Microsoft.VisualStudio.TestTools.UnitTesting;
4040

41-
[assembly: Parallelize(Workers = 2, Scope = ExecutionScope.MethodLevel)] // Violation
41+
[assembly: Parallelize(Workers = 0, Scope = ExecutionScope.MethodLevel)] // Violation
4242
[assembly: DoNotParallelize]
4343
```
4444

@@ -51,7 +51,7 @@ If you want parallel execution:
5151
```csharp
5252
using Microsoft.VisualStudio.TestTools.UnitTesting;
5353

54-
[assembly: Parallelize(Workers = 2, Scope = ExecutionScope.MethodLevel)]
54+
[assembly: Parallelize(Workers = 0, Scope = ExecutionScope.MethodLevel)]
5555
```
5656

5757
If you want sequential execution:
@@ -62,6 +62,33 @@ using Microsoft.VisualStudio.TestTools.UnitTesting;
6262
[assembly: DoNotParallelize]
6363
```
6464

65+
If you want to enable parallelization at the assembly level but disable it for specific classes or methods, apply `Parallelize` at the assembly level and `DoNotParallelize` at the class or method level:
66+
67+
```csharp
68+
using Microsoft.VisualStudio.TestTools.UnitTesting;
69+
70+
[assembly: Parallelize(Workers = 0, Scope = ExecutionScope.MethodLevel)]
71+
72+
[DoNotParallelize]
73+
[TestClass]
74+
public class SequentialTests
75+
{
76+
[TestMethod]
77+
public void Test1() { }
78+
}
79+
80+
[TestClass]
81+
public class ParallelTests
82+
{
83+
[TestMethod]
84+
public void Test2() { }
85+
86+
[DoNotParallelize]
87+
[TestMethod]
88+
public void Test3() { }
89+
}
90+
```
91+
6592
## When to suppress warnings
6693

6794
Do not suppress warnings from this rule. Having both attributes creates ambiguous test configuration that should be resolved.

docs/core/testing/mstest-analyzers/mstest0060.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@ A test method has multiple <xref:Microsoft.VisualStudio.TestTools.UnitTesting.Te
3333

3434
## Rule description
3535

36-
A test method should have only one attribute that derives from <xref:Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute>. Having multiple test method attributes (such as `[TestMethod]` and `[DataTestMethod]`) on the same method can lead to unexpected behavior and test execution issues.
36+
A test method should have only one attribute that derives from <xref:Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute>. Having multiple test method attributes (such as `[TestMethod]` and `[UITestMethod]`) on the same method causes only one attribute to be used (the first one returned by reflection), which can be confusing and lead to unintended test execution.
3737

3838
```csharp
3939
[TestClass]
4040
public class TestClass
4141
{
4242
[TestMethod]
43-
[DataTestMethod] // Violation
43+
[UITestMethod] // Violation
4444
public void TestMethod1()
4545
{
4646
// Test code
@@ -66,7 +66,7 @@ public class TestClass
6666

6767
## When to suppress warnings
6868

69-
Do not suppress warnings from this rule. Having multiple test method attributes creates ambiguous test configuration that should be resolved.
69+
Do not suppress warnings from this rule. Having multiple test method attributes creates ambiguous test configuration where only one attribute is used, which can cause confusion about which test behavior is actually applied.
7070

7171
## Suppress a warning
7272

docs/core/testing/mstest-analyzers/mstest0062.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ A test method has parameters marked with `out` or `ref` modifiers.
3333

3434
## Rule description
3535

36-
Test methods should not have `out` or `ref` parameters because these modifiers are incompatible with data-driven testing. Data sources provide values to test methods through standard parameter passing, and the `out` and `ref` modifiers create conflicts with this mechanism. Using regular parameters makes tests more maintainable and compatible with parameterized test features.
36+
Test methods should not have `out` or `ref` parameters. MSTest is responsible for calling test methods, and it never reads the values that are passed by reference after the test method finishes. These modifiers are misleading because they suggest the test method returns values that will be used, but MSTest never uses them after the test completes.
3737

3838
```csharp
3939
[TestClass]
@@ -56,7 +56,7 @@ Remove the `out` and `ref` modifiers from test method parameters.
5656
public class TestClass
5757
{
5858
[TestMethod]
59-
public void TestMethod(string s, string s2)
59+
public void TestMethod()
6060
{
6161
// Test code
6262
}
@@ -74,10 +74,11 @@ public class TestClass
7474
{
7575
// Arrange
7676
string result;
77+
string passedByRef = "some value";
7778
var instance = new MyClass();
7879

7980
// Act
80-
bool success = instance.TryGetValue(out result);
81+
bool success = instance.TryGetValue(out result, ref passedByRef);
8182

8283
// Assert
8384
Assert.IsTrue(success);
@@ -92,7 +93,7 @@ For data-driven tests:
9293
[TestClass]
9394
public class TestClass
9495
{
95-
[DataTestMethod]
96+
[TestMethod]
9697
[DataRow("input1", "expected1")]
9798
[DataRow("input2", "expected2")]
9899
public void TestMethod(string input, string expected)
@@ -113,7 +114,7 @@ public class TestClass
113114

114115
## When to suppress warnings
115116

116-
You might suppress this warning if you have a specific advanced scenario that requires `out` or `ref` parameters. However, this is rare, and you should consider alternative test designs first.
117+
Do not suppress warnings from this rule. There is no valid scenario where test methods should use `out` or `ref` parameters.
117118

118119
## Suppress a warning
119120

0 commit comments

Comments
 (0)