[Bug]: 12.2.2 breaks branch coverage

[CSchulz]

Version

12.2.2

Steps to reproduce

1. Create a sample class with decorator like this

   import { Pipe, PipeTransform } from '@angular/core';
   import { ConfigService } from './config.service';
   
   @Pipe({
     name: 'config',
   })
   export class ConfigPipe implements PipeTransform {
     constructor(private service: ConfigService) {}

2. Execute and wondering why there are four branches.

Expected behavior

Expect that there are still zero branches.

Actual behavior

Four branches are counted.

Additional context

No response

Environment

System:
    OS: Windows 10 10.0.19043
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Binaries:
    Node: 16.14.2 - C:\nodejs\node.EXE
    npm: 8.5.0 - C:\nodejs\npm.CMD
  npmPackages:
    jest: ~28.1.0 => 28.1.3

[ahnpnl]

Please note that this preset doesn’t do anything with code coverage. Code coverage is a part of Jest itself. You can try v8 coverage instead of default one from Jest (Istanbul)

[CSchulz]

Sure, but why does it works with 12.1.0 and 12.2.0 and breaks with 12.2.2?

I didn't find any special change in the dependencies.

[ahnpnl]

Code coverage is decided by Babel instanbul plug-in, which is not used in this preset. I would suggest to use v8 coverage which is more stable than Babel.

Why it is different than 12.2.0 can only be due to how Babel processed the codes which are transformed by ts, and we don’t have the control over it either.

[minijus]

I have noticed exactly same issue on our codebase, but it breaks when updating to any 12.x version (right now using 11.1.2).

Tried overriding some versions of transitive dependencies to pin down which dependency and which version exactly introduced this change. But I was not successful at this.

@ahnpnl do you have any exact transitive dependencies in mind that could be verified for this coverage change?

[ahnpnl]

Most likely it comes from Jest and its deps. I don't know exactly which ones, code coverage is a combination of: transpiled codes produced by TypeScript + babel istanbul.

[minijus]

It seems that some transitive dependency produces different transformed result. There is nice work and writeup by @mandarini on https://github.com/mandarini/isolatedm-jest

Would it be possible to consider incorporating work above into jest-preset-angular (maybe under additional option)? It would simplify patching huge workspaces with many projects.

[ahnpnl]

PR is welcome:)

[mandarini]

Hi @ahnpnl, thank you for all the work! Do you think it would make more sense to raise this issue with istanbul, then?

[ahnpnl]

I think it’s more on Istanbul side to solve it completely. The solution which alters the compiled output looks to me like a workaround rather than a right way. I think we should push Istanbul to fix it.

In the meantime, maybe using v8 coverage is better?

[mandarini]

I'll give the v8 a shot, thanks.

Yeah, it's definitely a hacky workaround, and it's definitely impacting speed, since it's doing a lookup and replacement each time. There is already an issue open in the istanbul side, but no luck so far :(

[michaelfaith]

Oh man, I just hit this today, upgrading Angular from 14.1.0 to 14.2.2. Glad it's not just me. That issue in istanbul is like 5 years old though, so it's curious that a bug that old would rear its head with a minor angular version update. @mandarini did you have success with v8?

[michaelfaith]

@ahnpnl I'm seeing the same issues with v8 set as coverageProvider.

Am I missing something?

[ahnpnl]

Are you using isolatedModules: true?

It seems like it goes fine with Jest 29, not quite sure. I checked with https://github.com/thymikee/jest-preset-angular/tree/main/examples/example-app-v14

[michaelfaith]

No, I'm not. But I just tried adding that, and 174 of 252 tests are failing now. All with something similar to this:

Component 'FabSidenavItemComponent' is not resolved:
     - templateUrl: ./sidenav-item.component.html
     - styleUrls: ["./sidenav-item.component.scss"]
    Did you run and wait for 'resolveComponentResources()'?

      21 |
      22 |   beforeEach(() => {
    > 23 |     fixture = TestBed.createComponent(FabSidenavItemComponent);
         |                       ^
      24 |     component = fixture.componentInstance;
      25 |     fixture.detectChanges();
      26 |   });

So that seems like a step back.

This is what I added, taking inspiration from your example app

globals: {
    'ts-jest': {
      ...defaults.globals['ts-jest'],
      isolatedModules: true,
    },
  },

[michaelfaith]

We're also using yarn workspaces, and I notice your example app for yarn workspaces is still at Angular 13. It may be worth updating that example and checking to see if your results match mine after the 14.2 update.

[michaelfaith]

I think I finally discovered the culprit for my setup. After taking a second look at the example app you have, and all of the config (not just the jest config), I noticed that our tsconfig.spec had "emitDecoratorMetadata": true,, while yours doesn't. After removing that, the coverage doesn't break anymore. It looks like we've had that in our config for a while, so I'm not sure the exact lineage. But if you think that's an appropriate solution, then I think my issue is solved. Sorry for all the noise on this.

[ahnpnl]

Angular 12 removed the need of emitDecoratorMetadata so I think that was why the example app doesn’t have it. That option in general shouldn’t be needed anymore.

FYI with that option true, the emitted codes will be like in this example https://www.typescriptlang.org/tsconfig#emitDecoratorMetadata, and notice that design:paramtypes is in emitted codes. This design:paramtypes was replaced in the workaround mentioned in https://github.com/istanbuljs/istanbuljs/issues/70#issuecomment-975654329

[CSchulz]

I can confirm the issue is related to the forgotten emitDecoratorMetadata in our case. Fun fact the angular migration does not remove it automatically from the existing tsconfig.json files.

[CSchulz]

Where can I place issues with the coverage v8 provider? I have noticed some interesting behavior resulting in missing branches for import statements.