Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code coverage: cannot cover type of express request #5161

Closed
6 tasks done
wataruian opened this issue Feb 9, 2024 · 6 comments · Fixed by #5206
Closed
6 tasks done

Code coverage: cannot cover type of express request #5161

wataruian opened this issue Feb 9, 2024 · 6 comments · Fixed by #5206
Labels
enhancement New feature or request feat: coverage Issues and PRs related to the coverage feature

Comments

@wataruian
Copy link

Describe the bug

I have a simple nestjs controller that I am testing that accepts express - request as its param.

I achieved almost 100% code coverage except:

Screenshot 2024-02-09 at 5 07 10 PM

I cannot cover the type of Request from code coverage. I am using vitest-mock-express to mock the request object passed to the controller. I also tried node-mocks-http and had the same result.

vitest-mock-express:

import { getMockReq } from 'vitest-mock-express';
const request = getMockReq();

node-mocks-http:

import { createRequest } from 'node-mocks-http';
const request = createRequest();

The following test cases should cover all of the code:

describe('/', () => {
      it('should succeed', async () => {
        const response = await controller.index();
        expect(response).toStrictEqual({ status: 'ok' });
      });
});

describe('/csrf', () => {
      it('should succeed', async () => {
        const { res } = getMockRes();
        request.res = res;
        const response = await controller.csrf(request as Request);
        expect(response).toStrictEqual({ success: true });
      });

      it('should fail', async () => {
        const response = await controller.csrf(request as Request);
        expect(response).toStrictEqual({ success: false });
      });
});

Coverage result:
Screenshot 2024-02-09 at 5 15 29 PM

I have a repo for reproduction at: https://github.com/wataruian/vitest-nestjs-repro

Reproduction

https://github.com/wataruian/vitest-nestjs-repro

System Info

System:
    OS: macOS 13.5.2
    CPU: (10) arm64 Apple M2 Pro
    Memory: 84.00 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.15.0 - ~/.asdf/installs/nodejs/18.15.0/bin/node
    Yarn: 1.22.21 - ~/.asdf/installs/nodejs/18.15.0/bin/yarn
    npm: 9.5.0 - ~/.asdf/plugins/nodejs/shims/npm
    pnpm: 8.11.0 - ~/.asdf/installs/nodejs/18.15.0/bin/pnpm
    Watchman: 2023.09.25.00 - /opt/homebrew/bin/watchman
  Browsers:
    Brave Browser: 121.1.62.156
    Safari: 16.6
  npmPackages:
    @vitest/coverage-v8: ^1.1.3 => 1.2.2
    vitest: ^1.1.3 => 1.2.2

Used Package Manager

npm

Validations

@AriPerkkio AriPerkkio added feat: coverage Issues and PRs related to the coverage feature and removed pending triage labels Feb 10, 2024
@AriPerkkio
Copy link
Member

When Typescript transpiles the decorators it adds generated code that is included in source maps. There's a typeof Request check which is only partially covered, https://evanw.github.io/source-map-visualization/#MjEzNgBmdW5j....

image

Ideally Typescript should not include this in source maps. Then code coverage tools would not consider it as your source code, and they would exclude it from the report automatically.

ts-jest used to have hacky solution for this where they add ignore hint above the transpiled decorator: kulshekhar/ts-jest#488. Maybe Vitest could try to do something similar. There's already similar approach used for Vite's generated code:

/**
* Remove generated code from the source maps:
* - Vite's export helpers: e.g. `Object.defineProperty(__vite_ssr_exports__, "sum", { enumerable: true, configurable: true, get(){ return sum }});`
*/
function removeViteHelpersFromSourceMaps(source: string | undefined, map: EncodedSourceMap) {
if (!source || !source.match(VITE_EXPORTS_LINE_PATTERN))
return map
const sourceWithoutHelpers = new MagicString(source)
sourceWithoutHelpers.replaceAll(VITE_EXPORTS_LINE_PATTERN, '\n')
const mapWithoutHelpers = sourceWithoutHelpers.generateMap({
hires: 'boundary',
})
// A merged source map where the first one excludes helpers
const combinedMap = remapping(
[{ ...mapWithoutHelpers, version: 3 }, map],
() => null,
)
return combinedMap as EncodedSourceMap
}

@wataruian
Copy link
Author

Are there any plans to implement this hack or a permanent solution to this problem?

For now, I am ignoring decorators via /* v8 ignore next */

Thanks!

@AriPerkkio
Copy link
Member

It actually looks like the ts_decorate part is coming from SWC: https://github.com/search?q=repo%3Aswc-project%2Fswc%20_ts_decorate&type=code

Could you try if disabling SWC helps?

@wataruian
Copy link
Author

wataruian commented Feb 11, 2024

Yes, disabling SWC fixes the problem from my minimal reproduction repo.

But on a larger nestjs repository, removing swc.vite() from plugins breaks many things.

One example of such error is:

TypeError: Cannot read properties of undefined (reading 'get')
 ❯ Object.shutdown tests/vitest/common.ts:219:47
    217|   },
    218|   async shutdown(app: INestApplication, moduleFixture: TestingModule) {
    219|     const pubSub: RedisPubSub = moduleFixture.get<RedisPubSub>('PUB_SUB');
       |                                               ^
    220|     await pubSub.close();
    221|
 ❯ tests/vitest/unit/app.spec.ts:29:18

On nestjs docs it is specified that swc.vite() plugin should be used when using vitest: https://docs.nestjs.com/recipes/swc

Also, it was stated here:
nestjs/nest#9228 (comment)

@AriPerkkio
Copy link
Member

Ok, disabling SWC is not an option as decorator metadata support of esbuild doesn't seem to work with NestJs.

I did some testing here AriPerkkio@42e1e1b and it seems to work on the minimal reproduction. I'll need to look into @vitest/coverage-istanbul later.

@wataruian
Copy link
Author

@AriPerkkio Awesome!

@AriPerkkio AriPerkkio added the enhancement New feature or request label Feb 14, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request feat: coverage Issues and PRs related to the coverage feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants