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

When using coverage.all typescript files with no executable code are colored red in the coverage report #3605

Closed
6 tasks done
HristoKolev opened this issue Jun 17, 2023 · 16 comments · Fixed by #5328
Closed
6 tasks done
Labels
feat: coverage Issues and PRs related to the coverage feature p2-to-be-discussed Enhancement under consideration (priority)

Comments

@HristoKolev
Copy link

Describe the bug

I have a typescript file that only contains typescript interfaces. When I run vitest with coverage I get it reported as red (uncovered) even when the interfaces are referenced by other files that are covered by tests.

image

The way jest handles this situation is to not count these file as neither covered or uncovered but just empty and colored in gray:

image

Reproduction

Here is the github repo from the images:

https://github.com/HristoKolev/vite-workshop/tree/6549514d7940d642a51aebcfd049466c3cc25e7b

to run it:

cd vite-app

npm i

npm run test

System Info

System:
    OS: Windows 10 10.0.22621
    CPU: (32) x64 AMD Ryzen 9 7950X 16-Core Processor
    Memory: 14.66 GB / 31.16 GB
  Binaries:
    Node: 18.12.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD
    npm: 9.2.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22621.1778.0), Chromium (114.0.1823.51)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    @vitejs/plugin-react: ^4.0.0 => 4.0.0
    @vitest/coverage-v8: ^0.32.2 => 0.32.2
    vite: ^4.3.2 => 4.3.9
    vitest: ^0.32.0 => 0.32.2

Used Package Manager

npm

Validations

@AriPerkkio
Copy link
Member

even when the interfaces are referenced by other files that are covered by tests

Which file is covering it? When I add import "./utils/server-data-model" to App.tsx or even App.test.tsx, the file is marked as fully covered.

The way jest handles this situation

Is that with Jest's v8 provider or babel? Jest's default babel provider is similar as Vitest's coverage.provider: 'istanbul'.

@HristoKolev
Copy link
Author

This import exists in EditPetModal.tsx which is being imported by EditPetModal.test.tsx:

import { Pet } from '../utils/server-data-model';

if I change it to:

import '../utils/server-data-model';

the file is reported as covered.


Yes - the jest setup was using the default provider.

I changed my vitest setup's provider to istanbul and now it doesn't even report files that only have types in them. I guess that's good enough. I'll try using that for now. Thank you.

@Kolobamanacas
Copy link

I don't wish to open another issue for which seems to be the same issue, so here the thing. I'm trying to incorporate @vitest/coverage-v8 for coverage in a project and it seems like it behaves not the way I expect with files consist of only types.

Here is repro:

  1. Clone this mini repo: https://github.com/Kolobamanacas/vitest-type-only-files-coverage
  2. Install dependencies: yarn.
  3. Run tests: yarn test.

Expected behavior:
I have a file src/types.ts which only consists of two exported types:

export type ResponseType = string

export type ResponseCode = number

I expect it to not be tested for coverage or 100% covered.

Actual behavior: Vitest reports it as not covered with tests:

2023-07-07_10-56-11

Is it expected behavior?

@AriPerkkio
Copy link
Member

@Kolobamanacas either remove coverage.all or add the types.ts to coverage.exclude.

This file is picked due to coverage.all. The v8-to-istanbul doesn't do AST analysis for files so it has no idea what the file contains. It simply marks whole file as uncovered. As this file is picked up due to coverage.all, it is never even run.

@HristoKolev
Copy link
Author

@Kolobamanacas try and import type the types of that file and see if makes any difference.

@Kolobamanacas
Copy link

@HristoKolev Just checked, unfortunately it didn't help.

@AriPerkkio So it's a feature then. ) Ok, thank you. I'll stick to excluding all types files in config.

JoshuaKGoldberg added a commit to JoshuaKGoldberg/create-typescript-app that referenced this issue Aug 23, 2023
## PR Checklist

- [x] Addresses an existing open issue: fixes #692
- [x] That issue was marked as [`status: accepting
prs`](https://github.com/JoshuaKGoldberg/template-typescript-node-package/issues?q=is%3Aopen+is%3Aissue+label%3A%22status%3A+accepting+prs%22)
- [x] Steps in
[CONTRIBUTING.md](https://github.com/JoshuaKGoldberg/template-typescript-node-package/blob/main/.github/CONTRIBUTING.md)
were taken

## Overview

Skips using istanbul, in favor of v8. I'm hoping using the same coverage
report format will help Codecov figure things out. Note that this now
triggers vitest-dev/vitest#3605 issue of v8
counting types as untested.
@JoshuaKGoldberg
Copy link
Contributor

feature

Ehh, this really doesn't feel like a feature IMO. If the file has no runtime code (other than CJS module.exports transpilation when not running in ESM) then it doesn't make sense to count it as uncovered lines. There's nothing being covered/uncovered! Especially if the file is only ever imported with import type, and since this is not an issue in Jest (kulshekhar/ts-jest#378 / jestjs/jest#10812).

Example file that just contains an interface and no runtime code: https://app.codecov.io/gh/JoshuaKGoldberg/create-typescript-app/blob/ef2e3483d851c8df2d4fdb2f7847d921a79f29b6/src%2Ftypes.ts

Request: can we please consider this a bug rather than a feature?

@AriPerkkio
Copy link
Member

AriPerkkio commented Oct 2, 2023

and since this is not an issue in Jest

When using V8 coverage in Jest, is it really able to detect whether file contains only types? By default Jest uses Istanbul instrumented code coverage which is identical with Vitest's provider: 'istanbul'. When testing Jest, make sure to switch to coverageProvider: 'v8'.

@AriPerkkio AriPerkkio added feat: coverage Issues and PRs related to the coverage feature and removed pending triage labels Oct 2, 2023
@AriPerkkio
Copy link
Member

Tested with "jest": "^29.4.0" and it's identical with Vitest: --coverageProvider babel does not include files containing just types, but --coverageProvider v8 does. Vitest equivalent configurations are --coverage.provider="v8"|"istanbul".

Similar issues:

Root cause is described here: #3605 (comment)

@JoshuaKGoldberg
Copy link
Contributor

Sorry for the delay in responding - but yes, +1 to the last comment. I have a standalone repro at https://github.com/JoshuaKGoldberg/repros/tree/v8-types-coverage-jest-vs-vitest.

@AriPerkkio
Copy link
Member

AriPerkkio commented Oct 19, 2023

@JoshuaKGoldberg your reproduction case is missing coverage.all equivalent flag - it's not covering untested files at all which this issue is all about. Jest uses collectCoverageFrom to handle coverage of files that are not covered by tests. Add that and you'll see identical behaviour as in Vitest.

If you want Vitest to behave similarly in your reproduction, set coverage.all as false.

@JoshuaKGoldberg
Copy link
Contributor

D'oh! Thanks - updated the branch. Sorry for the noise.

@sheremet-va sheremet-va moved this to P2 - 2 in Team Board Feb 12, 2024
@sheremet-va sheremet-va added the p2-to-be-discussed Enhancement under consideration (priority) label Feb 15, 2024
@philly-vanilly
Copy link

philly-vanilly commented Feb 21, 2024

@JoshuaKGoldberg your reproduction case is missing coverage.all equivalent flag - it's not covering untested files at all which this issue is all about. Jest uses collectCoverageFrom to handle coverage of files that are not covered by tests. Add that and you'll see identical behaviour as in Vitest.

If you want Vitest to behave similarly in your reproduction, set coverage.all as false.

@AriPerkkio If you disable all, then uncovered pure interface files are ignored - but so are uncovered component files too which do not have a .spec file at all. Please reopen this issue! Typescript interfaces should very obviously not be covered by coverage as they are compiled away at runtime. This is the behaviour that any developer would expect, so this should be considered a bug.

@AriPerkkio
Copy link
Member

Please reopen this issue! Typescript interfaces should very obviously not be covered by coverage as they are compiled away at runtime. This is the behaviour that any developer would expect, so this should be considered a bug.

This issue is open. You can use @vitest/coverage-istanbul to avoid Typescript interfaces showing up uncovered - this was one of the reason why @vitest/coverage-istanbul was built in the first place: #1252

As mentioned before here, in Vitest, Jest and c8 there is no AST analysis for V8 coverage. It has no idea what the files contain.

I've been thinking about experimenting with introducing AST analysis to @vitest/coverage-v8. It would solve this and many other issues. Though it would decrease performance a bit but hopefully not too much. It's not easy task and requires time.

@philly-vanilly
Copy link

@AriPerkkio Sorry, my bad, I got confused by the big purple closed above my comment which was just a referenced other issue. It's good to see you are working on this, thank you! In general, Vitest has been a blessing for our project.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 19, 2024
@AriPerkkio
Copy link
Member

In #5457 there are more improvements related to Typescript typings and code coverage. The v8 provider can now exclude non-runtime code from covered files as well.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: coverage Issues and PRs related to the coverage feature p2-to-be-discussed Enhancement under consideration (priority)
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants