Reporting wrong coverage with jest, nestjs and nestjs/graphql attributes

[Shereef]

Steps to reproduce

Steps:

1. Clone https://github.com/Shereef/graphql-jest-coverage-attribute
2. run npm run test:cov

Expected behavior

Should get 100% coverage

Actual behavior

It's reporting 85.71% because of an attribute @Query(() => String)

Additional context

If you look at this file and delete the attribute parameter to @Query() you will get 100% coverage but the project will not work.

import { Resolver, Query } from '@nestjs/graphql';

@Resolver()
export class AppResolver {
  @Query(() => String)
  getHello(): string {
    return 'Hello World!';
  }
}

Environment

System:
    OS: macOS 13.0.1
    CPU: (10) arm64 Apple M1 Pro
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    npm: 8.19.2 - ~/node_modules/.bin/npm
    npmPackages:
                "istanbul-lib-coverage": "^3.0.0",
                "istanbul-lib-instrument": "^5.1.0",
                "istanbul-lib-report": "^3.0.0",
                "istanbul-lib-source-maps": "^4.0.0",
                "istanbul-reports": "^3.1.3",

[Shereef]

I didn't know which package introduced this so I also filed this jestjs/jest#13627

[rklos]

+1
I have also found this problem in my Vue project and it's caused by 5.2.1 version of the istanbul-lib-instrument.

[davidediak]

any luck on finding a solution, without downgrading?

[unematiii]

@rklos Indeed, it looks like coverage is also broken for Vue 3 single file components. I managed to narrow it down to this change: #662. Interestingly, using JSON.parse(JSON.stringify(coverageData.inputSourceMap)) instead of spread operator fixes the coverage report.

[oettingerj]

This issue is also affecting Svelte files- locking istanbul-lib-instrument at 5.2.0 fixes coverage there too.

[bcoe]

@marekdedic are there any thoughts on this?

@unematiii, @Shereef, any chance we could figure out a breaking test? It seems really weird that ... causes failures and JSON.parse(JSON.stringify(coverageData.inputSourceMap)) works.

[Shereef]

@marekdedic are there any thoughts on this?

@unematiii, @Shereef, any chance we could figure out a breaking test? It seems really weird that ... causes failures and JSON.parse(JSON.stringify(coverageData.inputSourceMap)) works.

I am not sure what you mean. Please give more details

[marekdedic]

Well, this is certainly strange...
Thanks @Shereef for the repro, let's try to use it to create a test for this.

I have no idea why using the JSON trick would fix this, we could switch to that, but it is generally slower... I'll look into this, however, I can't promise any timeline. Maintainers, feel free to revert the change if you feel this is pressing.

Sorry for the inconvenience.

[marekdedic]

Hi @Shereef,
I tried your demo repository and if I install istanbul-lib-instrument at version =5.2.0 I am still getting the issue you describe. Could you please confirm that this is the case for you? If so, that would mean that's a different issue from the one caused by #662 ... @rklos @unematiii Could you please make a minimal repro?

[rklos]

@marekdedic here you are: https://github.com/rklos/istanbul-lib-instrument-repro
I my case the key to reproduce this issue is to keep @babel/core and @babel/generator on version 7.17.9 or below.
Maybe the problem is more complex and scattered among few packages.

In this example changing the istanbul-lib-instrument version between 5.2.0 and 5.2.1 is also messing up the instrumented files list (5.2.1 instruments all of the files defined in collectCoverageFrom in jest config, but the 5.2.0 lists only tested files).

[marekdedic]

@marekdedic here you are: https://github.com/rklos/istanbul-lib-instrument-repro I my case the key to reproduce this issue is to keep @babel/core and @babel/generator on version 7.17.9 or below. Maybe the problem is more complex and scattered among few packages.

Oh, that'll make for a fun debugging... Thanks for the repro though

[Shereef]

I tried 5.2.0
Shereef/graphql-jest-coverage-attribute@main...testing-`[email protected]`

> [email protected] test:cov
> jest --coverage

 PASS  src/app.resolver.spec.ts
  AppResolver
    root
      ✓ should return "Hello World!" (7 ms)

-----------------|---------|----------|---------|---------|-------------------
File             | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-----------------|---------|----------|---------|---------|-------------------
All files        |   85.71 |      100 |      50 |     100 |                   
 app.resolver.ts |   85.71 |      100 |      50 |     100 |                   
-----------------|---------|----------|---------|---------|-------------------
Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        3.199 s, estimated 10 s
Ran all test suites.

Hi @Shereef, I tried your demo repository and if I install istanbul-lib-instrument at version =5.2.0 I am still getting the issue you describe. Could you please confirm that this is the case for you? If so, that would mean that's a different issue from the one caused by #662 ... @rklos @unematiii Could you please make a minimal repro?

[unematiii]

@Shereef Just a thought. Are you sure the callback inside Query decorator is actually being called during test run (as this is what is reportedly uncovered)?

I was playing around with st

@Query(() => {
    throw new Error('I was called');
})

and the test passed successfully. I also found this.

[marekdedic]

Ok, so to summarize:

• This issue as originally raised by @Shereef is present in version 5.2.0, therefore has nothing to do with Fixed an error when inputSourceMap is not a plain object #662 and is either caused by some other error in instanbul or by an error in the repro
• There is another unrelated issue as reported by @unematiii, that one is caused by a regression introduced in Fixed an error when inputSourceMap is not a plain object #662 and will be fixed by Fixed incorrect coverage for Vue files #711

@bcoe as you are the maintainer involved in all of this, could you please take a look at #711 and then decide what to do with this mess? :D

[Shereef]

@Shereef Just a thought. Are you sure the callback inside Query decorator is actually being called during test run (as this is what is reportedly uncovered)?

I was playing around with st

@Query(() => {
    throw new Error('I was called');
})

and the test passed successfully. I also found this.

I agree that I could test it like that but it seems like overkill
I would imagine a decorator like that shouldn't decrease the coverage.
To me it seems like saying this function returns a string you don't create a type alias and reference that type alias in tests to test it.

If I my logic is wrong then I apologize for wasting all of your time.