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

Coverage with Changed #5237

Closed
4 tasks done
bad4iz opened this issue Feb 19, 2024 · 6 comments · Fixed by #5314
Closed
4 tasks done

Coverage with Changed #5237

bad4iz opened this issue Feb 19, 2024 · 6 comments · Fixed by #5314
Labels
enhancement New feature or request feat: coverage Issues and PRs related to the coverage feature p2-to-be-discussed Enhancement under consideration (priority)

Comments

@bad4iz
Copy link
Contributor

bad4iz commented Feb 19, 2024

Clear and concise description of the problem

--coverage + --changed don't work well together. Coverage doesn't do anything special when --changed is passed.
There are files for which there are no tests.

I created an example where you can see it.
https://gitlab.com/bad4iz/file-signature-in-react/-/merge_requests/92

$ git diff --name-only origin/develop HEAD
.gitlab-ci.yml
src/component/SelectCert.Copy.tsx
src/component/utils/toBase64.ts
$ /builds/bad4iz/file-signature-in-react/node_modules/.bin/vitest --coverage.enabled --changed=origin/develop
 RUN  v1.2.2 /builds/bad4iz/file-signature-in-react
      Coverage enabled with v8
stderr | plugin_loaded_error (/builds/bad4iz/file-signature-in-react/node_modules/crypto-pro-cadesplugin/dist/lib/cadesplugin_api.js:527:17)
Плагин недоступен
 ✓ src/component/utils/signFile.spec.js  (3 tests) 6ms
 ✓ src/component/utils/toBase64.spec.ts  (1 test) 7ms
 Test Files  2 passed (2)
      Tests  4 passed (4)
   Start at  10:51:05
   Duration  1.14s (transform 33ms, setup 0ms, collect 65ms, tests 13ms, environment 422ms, prepare 162ms)
 % Coverage report from v8
--------------|---------|----------|---------|---------|-------------------
File          | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
--------------|---------|----------|---------|---------|-------------------
All files     |   73.25 |      100 |      60 |   73.25 |                   
 b64toBlob.ts |   34.28 |      100 |       0 |   34.28 | 13-35             
 signFile.ts  |     100 |      100 |     100 |     100 |                   
 toBase64.ts  |     100 |      100 |   66.66 |     100 |                   
--------------|---------|----------|---------|---------|-------------------
Done in 1.76s.
  • What files would you expect to be included in coverage report in this case?
  • src/component/SelectCert.Copy.tsx

and just the second problem. It's strange how the coverage works with --changes + coverage
vitest --coverage.enabled --changed=origin/develop --coverage.all=true
https://gitlab.com/bad4iz/file-signature-in-react/-/jobs/6198442714

-------------------|---------|----------|---------|---------|-------------------
File               | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-------------------|---------|----------|---------|---------|-------------------
All files          |   11.43 |    31.57 |   16.66 |   11.43 |                   
 src               |       0 |        0 |       0 |       0 |                   
//........             
  index.ts         |       0 |        0 |       0 |       0 | 1-2               
 ...omponent/utils |   55.75 |       75 |   42.85 |   55.75 |                   
  b64toBlob.ts     |   34.28 |      100 |       0 |   34.28 | 13-35             
  checkQuotes.ts   |       0 |        0 |       0 |       0 | 1-9               
  extract.ts       |       0 |        0 |       0 |       0 | 1-18              
  signFile.ts      |     100 |      100 |     100 |     100 |                   
  toBase64.ts      |     100 |      100 |   66.66 |     100 |                   
 ...nt/utils/hooks |       0 |        0 |       0 |       0 |                   
  index.js         |       0 |        0 |       0 |       0 | 1-2               
  ...oCertsList.ts |       0 |        0 |       0 |       0 | 1-27              
  ...ertificate.ts |       0 |        0 |       0 |       0 | 1-29              
-------------------|---------|----------|---------|---------|-------------------

and
vitest --coverage.enabled --coverage.all=true
https://gitlab.com/bad4iz/file-signature-in-react/-/jobs/6198442715

% Coverage report from v8
-------------------|---------|----------|---------|---------|-------------------
File               | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-------------------|---------|----------|---------|---------|-------------------
All files          |   30.12 |    63.33 |   44.44 |   30.12 |                   
 src               |       0 |        0 |       0 |       0 |                   
//.....            
 ...omponent/utils |   97.34 |    86.66 |   85.71 |   97.34 |                   
  b64toBlob.ts     |     100 |      100 |     100 |     100 |                   
  checkQuotes.ts   |     100 |      100 |     100 |     100 |                   
  extract.ts       |   83.33 |    33.33 |     100 |   83.33 | 11-13             
  signFile.ts      |     100 |      100 |     100 |     100 |                   
  toBase64.ts      |     100 |      100 |   66.66 |     100 |                   
 ...nt/utils/hooks |   96.55 |    85.71 |   66.66 |   96.55 |                   
  index.js         |       0 |        0 |       0 |       0 | 1-2               
  ...oCertsList.ts |     100 |      100 |     100 |     100 |                   
  ...ertificate.ts |     100 |      100 |     100 |     100 |                   
-------------------|---------|----------|---------|---------|------------------

I think this is an important.
feature to write tests at all. And I think it's not worth running all the tests mr or pr to make sure that you didn't forget to test some file.

what is the point of measuring the coverage of the entire project? to increase it. and this is not present now in --coverage + --changed

Suggested solution

  1. --coverage + --changes show the coverage of files that do not have tests..
  2. --coverage + --changes shows true information about coverage

Alternative

No response

Additional context

No response

Validations

@hi-ogawa hi-ogawa added enhancement New feature or request feat: coverage Issues and PRs related to the coverage feature labels Feb 20, 2024
@trane2025
Copy link

We are waiting for this too

@AriPerkkio
Copy link
Member

I'm planning to focus on this next. Starting probably this or next week.

Before starting we'll need a smaller minimal reproduction setup with clear goals of what should be seen on coverage report. The linked project is too big for this.

If you want to help and see this feature implemented faster, please help setting up a minimal reproduction that's dedicated for this.

@AriPerkkio
Copy link
Member

There's now a minimal reproduction setup at https://github.com/AriPerkkio/reproductions, branch vitest-changed-coverage. Make changes to source files and run pnpm run coverage --changed=HEAD to see the issue.

There's also branch jest-changed-coverage which has Jest setup for comparison. However even Jest seems to have issues with --coverage + --changedSince:

  • Run pnpm test and see how src/food.ts has 100% statement coverage.
  • Make change to test/animals.test.ts:
    test("Dog eats apples", () => {
    -  new Dog().eat("APPLES");
    +  new Dog().eat("NONE");
    });
  • Run pnpm run test --changedSince=HEAD
  • See how src/food.ts is not in coverage report ❌
  • Run pnpm test to run all tests and see how src/food.ts has now 55% statement coverage.

In Vitest we should be able to pick up all dependencies that the changed file has and be able to add those as uncovered. So we should not show only the coverage of the changed files, but also the coverage of their dependencies.

@AriPerkkio
Copy link
Member

AriPerkkio commented Feb 24, 2024

Also --coverage --changed may run different test files compared to just --changed. If there are two test files that cover a single source file, and only one of these test files is changed, we'll need to run the unchanged test file as well in order to get correct coverage of the source file. Without --coverage flag the --changed would not run the unchanged test file though.

├── src
|  └── source-file.ts
└── test
   ├── first.test.ts
   └── second.test.ts
// src/source-file.ts
export function first() {
  return true;
}
export function second() {
  return true;
}

// test/first.test.ts
import { first } from '../src/source-file';
test("first", () => {
  first();
});

// test/second.test.ts
import { second } from '../src/source-file';
test("second", () => {
  second();
});

Now make change to first.test.ts only:

  • vitest --changed=HEAD should run first.test.ts only.
  • vitest --changed=HEAD --coverage should run first.test.ts and second.test.ts in order to show correct coverage of src/source-file.ts.

@AriPerkkio AriPerkkio added the p2-to-be-discussed Enhancement under consideration (priority) label Feb 25, 2024
@bad4iz
Copy link
Contributor Author

bad4iz commented Feb 27, 2024

I wonder if a file without tests will be included in the coverage?

@AriPerkkio
Copy link
Member

Yes, changed (or newly created) files without tests should show up with 0% coverage. As long as they match the coverage.include/exclude/all settings.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 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 p2-to-be-discussed Enhancement under consideration (priority)
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants