Add diagnostic and code fix verification helpers #131
Conversation
sharwell
force-pushed
the
nuget-testing
branch
from
718fada
to
36e487d
Compare
* Make CancellationToken optional for RunAsync in test scenarios * Handle VB compiler diagnostics in the same manner as CS * Use IsEmpty instead of testing Length
sharwell
force-pushed
the
nuget-testing
branch
from
f94a3b2
to
c0f8098
Compare
| /// <item><description>No diagnostics are reported in the input.</description></item> | ||
| /// <item><description>No code fixes are provided for the diagnostics reported in the input.</description></item> | ||
| /// <item><description>The code fix applied for the diagnostics does not produce a change in the source file(s).</description></item> | ||
| /// <item><description>The maximum number of allowed iterations is exceeded.</description></item> |
There was a problem hiding this comment.
@dotnet/csharplangdesign This is less pleasant than it need to be: dotnet/csharplang#1765
| /// <item><description>Otherwise, the default value is treated as the negative of the number of fixable diagnostics appearing in the input source file(s).</description></item> | ||
| /// </list> | ||
| /// | ||
| /// <note> |
There was a problem hiding this comment.
@dotnet/csharplangdesign This is an ideal situation where dotnet/csharplang#1767 would help
| /// <summary> | ||
| /// Gets or sets the number of code fix iterations expected during code fix testing. | ||
| /// </summary> | ||
| /// <remarks> |
There was a problem hiding this comment.
@dotnet/roslyn-ide @kuhlenh This documentation will never be seen by users because it's in a <remarks> section.
| public static DiagnosticResult CompilerError(string errorIdentifier) | ||
| => DiagnosticVerifier<TAnalyzer>.CompilerError(errorIdentifier); | ||
|
|
||
| public static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult expected, CancellationToken cancellationToken) |
There was a problem hiding this comment.
Remove the cancellation token or make it optional? Only keep it if there is a use case for testing cancellation.
There was a problem hiding this comment.
➡️ Should be fixed now 👍
| public static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult expected, CancellationToken cancellationToken) | ||
| => VerifyCSharpDiagnosticAsync(source, new[] { expected }, cancellationToken); | ||
|
|
||
| public static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult[] expected, CancellationToken cancellationToken) |
There was a problem hiding this comment.
Can be IEnumerable<DiagnosticResult. Also cancellation token needed?
There was a problem hiding this comment.
Can be
IEnumerable<DiagnosticResult>
I'll file a follow-up issue to consider this.
| private ImmutableArray<FileLinePositionSpan> _spans; | ||
| private string _message; | ||
|
|
||
| public DiagnosticResult(string id, DiagnosticSeverity severity) |
There was a problem hiding this comment.
This constructor is rarely required. Severity does not need to be specified in any of the three primary locations where this is constructed:
- By calling
Diagnostic() - By calling
CompilerError("CS0000") - Starting with commit 56d48d9, by using markup syntax in the input and/or output
| /// <summary> | ||
| /// Metadata references used to create test projects. | ||
| /// </summary> | ||
| public static class MetadataReferences |
There was a problem hiding this comment.
This is a pretty useful helper method that can fit in this class.
There was a problem hiding this comment.
➡️ I'll leave this as a follow-up item. I agree it seems useful.
|
Also try removing the need for async in the asserts. Worth it to do |
Will make
I have no intention to pursue a synchronous API for testing inherently asynchronous code, but if you'd like I can bring it to the broader team for consideration. |
|
@sharwell My understanding is that this is completely CPU-bound work. I've also had some internal resistance to the async ceremony in analyzer tests. Would you consider synchronous APIs out of scope for this unless they were to be implemented at the analysis level? |
|
@jnm2 It would be easy to add synchronous support as an extension. For testing StyleCop Analyzers, only one asynchronous method in this library is ever actually called by tests ( |
| @@ -46,6 +46,27 @@ public static Task VerifyCSharpFixAsync(string source, DiagnosticResult[] expect | |||
| return test.RunAsync(cancellationToken); | |||
| } | |||
|
|
|||
| public static Task VerifyVisualBasicDiagnosticAsync(string source, DiagnosticResult expected, CancellationToken cancellationToken = default) | |||
There was a problem hiding this comment.
Maybe code is a better name than source?
GetOfferedCSharpFixesAsync (Adds codefix that creates default settings file DotNetAnalyzers/StyleCopAnalyzers#1250)(likely not required now that iteration limit properties support the value0)Framework testing (Add tests for DiagnosticVerifier DotNetAnalyzers/StyleCopAnalyzers#2425)(initial project added with a few tests, but the majority of this item moved to a follow-up item)Extract CreateSolution(omitted in favor ofSolutionTransforms)Expose CreateProject (Implement SA1412 store files as utf8 DotNetAnalyzers/StyleCopAnalyzers#1151)(omitted in favor ofSolutionTransforms)Expose formatting options (Enable tests for code fix behavior with alternate indentation DotNetAnalyzers/StyleCopAnalyzers#1135)(left for library authors)SolutionTransforms)SolutionTransforms)SolutionTransforms)Restrict use of batch fixer (Verify during testing that code fixes do not use the batch fixer DotNetAnalyzers/StyleCopAnalyzers#1195)(left for library authors)Look at Improve DocumentBasedFixAllProvider performance for multiple documents DotNetAnalyzers/StyleCopAnalyzers#1983?(no action necessary)SolutionTransforms)Test help links (Initial conversion of HTML to markdown DotNetAnalyzers/StyleCopAnalyzers#1183)(left for library authors)Settings file (Improve test environment DotNetAnalyzers/StyleCopAnalyzers#1360)(left for library authors)Formatting options (Move indentation options from workspace settings to stylecop.json DotNetAnalyzers/StyleCopAnalyzers#2039)(left for library authors)TestEmptySourceAsync (Move TestEmptySourceAsync into DiagnosticVerifier DotNetAnalyzers/StyleCopAnalyzers#1047)(left for library authors)Closes #123
Items to file specific follow-up issues:
Allow(implemented in 56d48d9)MarkupTestFileto populate diagnostic resultsIEnumerable<DiagnosticResult>instead ofDiagnosticResult[]Add diagnostic and code fix verification helpers #131 (comment)