Update tests to stop relying on DiagnosticAnalyzerTestBase

[Evangelink]

Motivation

In one of the PR review, @sharwell said that inheriting from DiagnosticAnalyzerTestBase was the old (obsolete?) way of doing tests. Some preparation for the migration was already done by adding the static usings for C# and VB.NET.

This PR tries to replace most of the old usages with the new recommended.

What's left

There is still some work left but I am not sure whether this shall be done within this PR or into some other. Also, I need some input from you to move forward on some points.

Non migrated tests

Some tests are not migrated because:

• usage of the scope notation doesn't seem to be working properly. I didn't spend time to do investigate what was the issue. I will try to do so later on.

• some tests explicitly force the parse option to null, although I think I might have found a way to do the same thing with the new syntax, I am not sure about it.

• something wasn't working properly when multiple "files" were given

• I missed some test classes

AD0001

The following tests from ReviewUnusedParametersTests.cs:

• public async Task DiagnosticForUnusedLocalFunctionParameters_01()

• public async Task DiagnosticForUnusedLocalFunctionParameters_02()

• public async Task NoDiagnosticUsedLocalFunctionParameters()

are failing with the following error: error AD0001: Analyzer 'Microsoft.CodeQuality.Analyzers.Maintainability.ReviewUnusedParametersAnalyzer' threw an exception of type 'System.ArgumentException' with message 'The key already existed in the dictionary.'.

I will create a ticket to handle this issue but I am not sure whether to revert the failing tests to the old way or to skip them for the time being. WDYT?

Miscellaneous

• In some tests the callers always provide the same rule, it could be move directly to the get diagnostic result method.

• Some tests need to be updated to use the descriptor instead of the ID.

• I am not sure whether tests shall be using new DiagnosticResult() or VerifyCS.Diagnostic()

Hope this PR is going the right direction and helps you!

[sharwell]

It looks like your editor is misconfigured to make whitespace changes unrelated to the rest of the pull request. I stopped the review after a few files due to the high level of noise caused by this. If the editor was Visual Studio and there is no extension in use that was causing these changes, we will definitely want to get a bug filed to force it to stop this.

[Evangelink]

@sharwell Sorry for the time waste. I did all the changes without thinking to disable some of the extensions. I am used to rely on them for git development as they help avoiding those kind of space/tab or trailing space "issue".

If that's ok for you I will do a revert, un-stage all spaces and force-push right after.

[sharwell]

@Evangelink If you are OK with it, I should be able to selectively revert those lines as long as you are OK with me force-pushing to your verifycs-test branch. Let me know if you would prefer I do that.

If you are comfortable with it, please feel free to force-push to this PR to make the change.

[Evangelink]

@sharwell Of course I am fine with you doing some changes as needed in this PR.

There were too many conflict with the master branch (because of the culture for the string format) + all those incorrect spaces/tabs issues so I went for a rebase + re-apply of all changes 1 by 1. Hopefully everything shall be alright for you to review.

[sharwell]

@Evangelink I was able to complete the review. Please avoid rebasing at this point so the updates are straightforward to review. 👍

[Evangelink]

It would be nice to have some info about the failing tests in the CI, they are all green locally so it's a bit hard to understand what's wrong.

[sharwell]

Does this show for you? https://dev.azure.com/dnceng/public/_build/results?buildId=411650&view=ms.vss-test-web.build-test-results-tab

It should appear from this link:

[sharwell]

VisualBasic_ProtectedVariable_PublicContainingType

Microsoft.CodeAnalysis.Testing.Verifiers.EqualWithMessageException : Context: Diagnostics of test state
Expected diagnostic message to be \"No declarar campos de instancia visibles\" was \"Do not declare visible instance fields\"

Diagnostic:
 // Test0.vb(3,15): warning CA1051: Do not declare visible instance fields
GetBasicResultAt(3, 15, DoNotDeclareVisibleInstanceFieldsAnalyzer.CA1051)


Assert.Equal() Failure
Expected: No declarar campos de instancia visibles
Actual: Do not declare visible instance fields

   at Microsoft.CodeAnalysis.Testing.Verifiers.XUnitVerifier.Equal[T](T expected, T actual, String message) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Verifiers.XUnit/XUnitVerifier.cs:line 48
   at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.VerifyDiagnosticResults(IEnumerable`1 actualResults, ImmutableArray`1 analyzers, DiagnosticResult[] expectedResults, IVerifier verifier) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs:line 313
   at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.<VerifyDiagnosticsAsync>d__51.MoveNext() in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs:line 194
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable.ConfiguredTaskAwaiter.GetResult()
   at Microsoft.CodeAnalysis.Testing.CodeFixTest`1.<RunAsync>d__38.MoveNext() in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing/CodeFixTest`1.cs:line 189
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at Test.Utilities.VisualBasicCodeFixVerifier`2.<VerifyAnalyzerAsync>d__5.MoveNext() in /_/src/Test.Utilities/VisualBasicCodeFixVerifier`2.cs:line 38
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at Test.Utilities.VisualBasicCodeFixVerifier`2.<VerifyAnalyzerAsync>d__4.MoveNext() in /_/src/Test.Utilities/VisualBasicCodeFixVerifier`2.cs:line 27
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.UnitTests.DoNotDeclareVisibleInstanceFieldsTests.<VisualBasic_ProtectedVariable_PublicContainingType>d__13.MoveNext() in /_/src/Microsoft.CodeQuality.Analyzers/UnitTests/ApiDesignGuidelines/DoNotDeclareVisibleInstanceFieldsTests.cs:line 156
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

[sharwell]

@Evangelink apparently just one test class is causing problems. The two specific changes I mentioned just now should resolve the test failures, and then we can merge this.

[Evangelink]

@sharwell I was actually looking at the logs and couldn't find anything except the error lines (without any detail on the failing tests), I totally forgot to look at the Tests tab. Thanks!