Unfortunate interference between coverage_suite.dart and analyze_src_with_lints_git_test

[chloestefantsova]

Consider the following code fragment:

if (condition)
  doSomething();

Suppose, the SDK code was updated making doSomething not covered by any of the existing tests. The coverage test will then fail on CI, asking to run the coverage update routine and change the fragment as follows:

if (condition)
  // Coverage-ignore(suite): Not run.
  doSomething();

When the updated code is uploaded back to the CI, analyze_src_with_lints_git_test will report a test failure. It will say Statements in an if should be enclosed in a block., pointing to the statement updated by the coverage tool. Then, to comply with the linter rules, the code will be updated as follows by the programmer:

if (condition) {
  // Coverage-ignore(suite): Not run.
  doSomething();
}

Then the code is uploaded to CI once more. The coverage tool complains again. This time, it notices the newly appeared curly braces and wants to update the text of the ignore comment as follows:

if (condition) {
  // Coverage-ignore-block(suite): Not run.
  doSomething();
}

The emergent effect of the two tests complicates the routine of uploading CLs, effectively requiring to run the coverage tool, then analyze_src_with_lints_git_test, then the coverage tool once again, to avoid unnecessary re-uploading of the CL and re-running of the CI tests.

A potential solution to the problem is to make the coverage tool detect the case of the "dangling" if-then-else statements and update the code accordingly, inserting the curly braces and the desired form of the comment inside of them.

[chloestefantsova]

/cc @jensjoha @johnniwinther

[FMorschel]

Would it work to add the comment above the condition instead? Maybe that is easier.

E.g.:

// Coverage-ignore(suite): Not run.
if (condition) doSomething();