roslyn-sdk icon indicating copy to clipboard operation
roslyn-sdk copied to clipboard

New testing framework does not deal with 1 analyzer: n codefix scenario as expected

Open taori opened this issue 5 years ago • 5 comments

@sharwell

In my github repo in this branch and the method FixCommentsOnClassTests.SimpleRemoval i ran into an issue with my test case. The ordinary short syntax does not work and it appears like the inherited CodeFixTest FixedState.ExpectedDiagnostics is not being used.

I suppose this might be related to the scenario that i have one analyzer which spawns multiple diagnostics, while one codefix will remove the occurence of all reported diagnostics.

When debugging the testing framework i can see, that it expects just one of the diagnostics to be gone, instead of both like specified in the unit test.

See:

			var codeFixTest = new CodeFixTest<CommentAnalyzer, FixByRemovingClassComments>()
			{
				TestBehaviors = TestBehaviors.SkipGeneratedCodeCheck,
				CompilerDiagnostics = CompilerDiagnostics.Errors,
				TestState =
				{
					Sources = {test},
					ExpectedDiagnostics =
					{
						// Test0.cs(9,11): info ACA0005: Comments can be removed from this namespace.
						Verifier.Diagnostic(CommentAnalyzer.NamespaceRule).WithSpan(9, 11, 9, 30),
// Test0.cs(11,11): info ACA0002: Comments can be removed from this class.
						Verifier.Diagnostic(CommentAnalyzer.ClassRule).WithSpan(11, 11, 11, 19)
					},
				},
				FixedState =
				{
					ExpectedDiagnostics =
					{
					},
					Sources = {fixtest}
				},
			};
			await codeFixTest.RunAsync();

After a brief debugging session it appears that the issue might be that at the following place fixedState is being used instead of the FixedState property.

https://github.com/dotnet/roslyn-sdk/blob/869a894010b0d41b7175a13c6758e65ee3d79f6c/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing/CodeFixTest%601.cs#L211

taori avatar Jan 20 '20 21:01 taori

If FixedState.ExpectedDiagnostics is empty (like you have in your test), the default behavior is to derive the expected diagnostics from the analyzer and/or code fix. In cases where you want to explicitly indicate that the expected diagnostics is empty (inheritance does not work), you can set FixedState.InheritanceMode to StateInheritanceMode.Explicit.

sharwell avatar Jan 20 '20 21:01 sharwell

What do you think about adding some sort of detection to the code to point this out to the developer? I know this is an early stage of the component and i might have overlooked some docs, but some as mentioned in another issue, pointers help a lot + less questions here 😄

Setting it to Explicit fixes my issue though! so thanks for that!

taori avatar Jan 20 '20 22:01 taori

No objection to anything that can make it easier for developers to figure things out

sharwell avatar Jan 20 '20 22:01 sharwell

Is that something you would want to do yourself once you find the time or is that a "help wanted" kind of thing? :)

taori avatar Jan 20 '20 22:01 taori

If you want to, please feel free 😄

If it's trivial to implement you can just submit a PR and we can discuss it there. If it's something more complicated and you want to confirm the design before finishing the implementation, you can propose the specifics here.

sharwell avatar Jan 20 '20 23:01 sharwell