Incorrect branch coverage when using implicit else post #633

[alan-agius4]

Post #633 the implicit else is not being counted as a branch.

Before

{
  "branchMap": {
    "0": {
      "loc": {
        "start": { "line": 2, "column": 2 },
        "end": { "line": 5, "column": null }
      },
      "type": "if",
      "locations": [
        {
          "start": { "line": 2, "column": 2 },
          "end": { "line": 5, "column": null }
        },
        {
          "start": { "line": 2, "column": 2 },
          "end": { "line": 5, "column": null }
        }
      ]
    }
  }
}

After

{
  "branchMap": {
    "0": {
      "loc": {
        "start": { "line": 2, "column": 2 },
        "end": { "line": 5, "column": null }
      },
      "type": "if",
      "locations": [
        {
          "start": { "line": 2, "column": 2 },
          "end": { "line": 5, "column": null }
        }
      ]
    }
  }
}

Reproduction

https://github.com/alan-agius4/coverage-issue-istanbul

git clone https://github.com/alan-agius4/coverage-issue-istanbul.git
cd coverage-issue-istanbul
yarn
yarn test

Reported in Angular CLI: angular/angular-cli#22652

[bcoe]

This was hopefully fixed by:

#663

Are you able to test against the latest release of the instrumenter?

CC: @adrian-burlacu-software

[alan-agius4]

@bcoe, I am using the latest version and the issue is still persists.

[luismartinez85]

@bcoe, I am using the latest version and the issue is still persists.

[phyr0s]

Using lastest version and not working @bcoe

[adrian-burlacu-software]

@bcoe @alan-agius4 @phyr0s @luismartinez85

Ok so I originally fixed the coverage map in #633 because of the erroneous output.
Then, being a new contributor I figured downstream reporting would be updating by the rest of the genius crowd like yous guys :)
Tried and failed? to fix #663, even made tests last time I checked.

Now based on the help I've been getting from @bcoe on publishing #673 and #675 I think we're both busy. I'm certainly not fixing this for a more than two months so I'd consider picking up the genius quill and fix this case in the downstream reporting too.

Now I do notice one thing: Around January, my expectation was the output from the API to be this:

"locations": [
        {
          "start": { "line": 2, "column": 2 },
          "end": { "line": 5, "column": null }
        },
        {
          "start": { // PLACEHOLDER },
          "end": {  // PLACEHOLDER }
        },
      ]

It seems that the placeholder that I assumed would be left there in order to account for the implicit else was most likely giggle implicitly removed. So that's probably the adjustment or misalignment that is the source of this issue.

Cheerio, fellow collaborators ;)

[phyr0s]

@adrian-burlacu-software hello. I tried lastest version but branch coverage is broken.

[adrian-burlacu-software]

@adrian-burlacu-software hello. I tried lastest version but branch coverage is broken.

@phyr0s
have you tried reading my comment above?? The placeholder for the implied else is probably broken. Feel free to file a pull request!

[adrian-burlacu-software]

@bcoe @alan-agius4 @phyr0s @luismartinez85 @djok1

Just now I added the necessary placeholder to fix this in the reporting in pull #679.

When this gets released, I'll watch, but two things need to happen:

• bcoe needs to merge the release branch
• bcoe needs to approve the release branch.

[phyr0s]

@bcoe when you can check the @adrian-burlacu-software job? Thanks

[adrian-burlacu-software]

@bcoe
Fast-Fuzz 4.1.3 is now faster and mostly debugged: https://www.npmjs.com/package/fast-fuzz
That might come in handy in re-designing this poorly tested code map feature.

[bcoe]

This is hopefully fixed by #679

[antch]

Correct me if I'm wrong, but I don't think this is fixed...

$ npm ls istanbul-reports
ng-test@0.0.0 /Users/antch/angular
├── istanbul-reports@3.1.5 
└─┬ karma-coverage@2.0.3
  └── istanbul-reports@3.1.5  deduped

[acieroid]

I can confirm that it is not fixed, after upgrading to Angular 14 and reproducing the same as the initial bug report (angular/angular-cli#22652), coverage is still incorrectly computed.

[acieroid]

After some investigation, it seems that:

• it seems to apply to all reporters: the lcov reporter is not containing information on the else branch. Similarly for the json reporter (the branch coverage is described is [1] rather than [1, 0]).
• it does not apply to istanbul-lib-instrument: there is already a test for implicit else (first test of if.yaml), and it correctly produces [1, 0] as coverage

It seems that either the branch coverage gets modified between the instrumentation and the reporter, or that the reporter does not use info from the instrumentation?

I'm not familiar with the structure of this project, in case some guidance can be provided, I'd be happy to investigate further. Currently, I don't know where to look for the actual problem.

[AriPerkkio]

@antch + @acieroid, the karma-coverage used by angular seems to be using istanbul-lib-source-maps. That one does not preserve the implicit else. Exactly as described above - istanbul-lib-instrument is able to generate uncovered branch for implicit else, but once mapped back to sources through istanbul-lib-source-maps, the non-existing branch is lost. This will be fixed by #706.