dotnet / roslyn-analyzers-contrib

Replaced by dotnet/roslyn-analyzers.
https://github.com/dotnet/roslyn-analyzers
Apache License 2.0
16 stars 8 forks source link

Document testing pattern for diagnostics which include code fixes #19

Open sharwell opened 8 years ago

sharwell commented 8 years ago

In our experience, the ideal way to implement a test case for a diagnostic which includes a code fix is the following:

  1. Declare the "before" code (which results in one or more diagnostics) as testCode.
  2. Declare the "after" code (which is produced by the code fix) as fixedCode.
  3. Declare the expected diagnostics for testCode in a variable expected.
  4. If fixedCode still contains diagnostics, declare those in expectedFixed.
  5. Verify that testCode produces the expected diagnostics:

    await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
  6. Verify that fixedCode produces the expected diagnostics (use expectedFixed instead of EmptyDiagnosticResults when required):

    await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
  7. Verify that application of the code fix to testCode produces fixedCode:

    await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
giggio commented 8 years ago

I believe each assertion should be it's own test:

Number 6 does not seem necessary to me. If the output of the code action is known it seems redundant to verify the diagnostics it would produce.

sharwell commented 8 years ago

I believe each assertion should be it's own test:

This was our original approach. We eventually realized that it's way too easy to have one of the two deviate from the other, with the result being the code fix test isn't actually testing what you think it's testing. Even if you have a separate test for analyzers, step 5 is extremely valuable for understanding the precise behavior of a code fix test.

As an added benefit, the combined approach makes it very easy to update analyzer tests to include code fix tests if a code fix is added in the future. It's easy to see exactly which tests have been updated to include code fix testing, and which ones haven't. The separated approach requires a more complicated and tiresome review of the relations between pairs of methods (one code fix test corresponding to each analyzer test).

Step 6 allows easy identification of cases where code fixes do not correct all violations in the original code. When you have over 6,000 tests, this is a very valuable piece of information which is otherwise difficult to detect. Note that in the vast majority of cases, step 4 is not required and step 6 is copy/paste without alterations.

As a long term development strategy, the maintainability and scalability of the combined approach has proven immensely more valuable than the benefit of being able to identify an analyzer or code fix as the source of a specific test failure based solely on the name of the test. In practice, the messages produced by the assertions in step 5-7 still makes it clear from the complete test output which one is at fault.