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

Workspace coverage wrong when a file transforms differently in projects #3251

Closed
6 tasks done
AriPerkkio opened this issue Apr 25, 2023 · 2 comments · Fixed by #4309
Closed
6 tasks done

Workspace coverage wrong when a file transforms differently in projects #3251

AriPerkkio opened this issue Apr 25, 2023 · 2 comments · Fixed by #4309
Labels
feat: coverage Issues and PRs related to the coverage feature

Comments

@AriPerkkio
Copy link
Member

Describe the bug

Writing down the findings from #3226 (comment) with reproduction case. This affects both @vitest/coverage-c8 and @vitest/coverage-istanbul.

If a single file can transform differently in different workspace projects, the final coverage reported by @vitest/coverage-<x> packages will be incorrect. For example:

├── vitest.workspace.ts // Declares workspaces ['client', 'server']
├── vitest.config.ts // Enables coverage and sets the options
|
├── client
|  └── test
|     └── user-list.test.js // Imports user-list.js with web/client-side specific transforms
|
├── server
|  └── test
|     └── user-list.test.js // Imports user-list.js with SSR transforms
|
└── shared
   └── user-list.js // Can output both client and server side code depending on environment

Here both client and server import shared/user-list.js with different code transforms. These produce two different code outputs and source maps.

When generating coverage report the @vitest/coverage-c8 picks the first source map it finds from project's vite-node cache and uses it for both workspace projects. Instead it should be aware of which workspace project the current V8 coverage report belongs to, and use that project's vite-node cache.

I haven't yet debugged the @vitest/coverage-istanbul case but it's likely picking the first source map it encounters in istanbul-lib-source-maps and re-uses that one for both file coverages. Instead it should separate each workspace projects' coverageMap completely, and merge them at the end into one.

Reproduction

Source
export default function run(a = 1, b = 2) {
  //! WORKSPACE_1_REMOVE_START

  // In workspace 1 we remove two functions
  function removedFunctionOne() {
    return 1
  }
  function removedFunctionThree() {
    return 1
  }

  //! WORKSPACE_1_REMOVE_END

  const unusedFunction1 = () => 1
  const unusedFunction2 = () => a ? b : null

  // Coverage of this branch breaks when combined coverage of workspaces is not handled correctly
  if (a === 1000 && b === 1) {
    // This should be uncovered
    return 1001
  }

  //! WORKSPACE_3_REMOVE_START

  // In workspace 3 we remove three branches
  if (a === 100 && b === 1)
    return 101

  if (a === 100 && b === 2)
    return 102

  if (a === 100 && b === 3)
    return 103

  //! WORKSPACE_3_REMOVE_END

  function unusedFunction3() {
    return 1
  }

  // Coverage of this branch breaks when combined coverage of workspaces is not handled correctly
  if (a === 1000 && b === 2) {
    // This should be uncovered
    return 1002
  }

  function unusedFunction4() {
    return 1
  }

  return 'OK'
}
Transformed for workspace 1
function run(a = 1, b = 2) {
  
  const unusedFunction1 = () => 1;
  const unusedFunction2 = () => a ? b : null;
  if (a === 1e3 && b === 1) {
    return 1001;
  }
  //! WORKSPACE_3_REMOVE_START
  if (a === 100 && b === 1)
    return 101;
  if (a === 100 && b === 2)
    return 102;
  if (a === 100 && b === 3)
    return 103;
  //! WORKSPACE_3_REMOVE_END
  function unusedFunction3() {
    return 1;
  }
  if (a === 1e3 && b === 2) {
    return 1002;
  }
  function unusedFunction4() {
    return 1;
  }
  return "OK";
}
Transformed for workspace 3
function run(a = 1, b = 2) {
  //! WORKSPACE_1_REMOVE_START
  function removedFunctionOne() {
    return 1;
  }
  function removedFunctionThree() {
    return 1;
  }
  //! WORKSPACE_1_REMOVE_END
  const unusedFunction1 = () => 1;
  const unusedFunction2 = () => a ? b : null;
  if (a === 1e3 && b === 1) {
    return 1001;
  }
  
  function unusedFunction3() {
    return 1;
  }
  if (a === 1e3 && b === 2) {
    return 1002;
  }
  function unusedFunction4() {
    return 1;
  }
  return "OK";
}

Coverage reports

From left to right: merged broken coverage, workspace 1 coverage, workspace 3 coverage:

The separate workspace specific reports look OK but the merged one is clearly broken.

Istanbul:

C8:

System Info

Vitest `main` + any

Used Package Manager

pnpm

Validations

@AriPerkkio
Copy link
Member Author

AriPerkkio commented Oct 20, 2023

@sheremet-va, is WorkspaceProject.getName() unique enough to identify which project's fetchCache should be picked? I'm thinking about passing projectName: project.getName() in WorkerContext and then pass it as argument to coverage providers when reporting coverage.

Coverage providers will then use projectName and transformMode when deciding which fetchCache to use.

@sheremet-va
Copy link
Member

is WorkspaceProject.getName() unique enough to identify which project's fetchCache should be picked?

Yes, they must be unique. The error will be thrown if they are not before tests are running.

AriPerkkio added a commit to AriPerkkio/vitest that referenced this issue Oct 20, 2023
AriPerkkio added a commit to AriPerkkio/vitest that referenced this issue Oct 20, 2023
AriPerkkio added a commit to AriPerkkio/vitest that referenced this issue Oct 20, 2023
AriPerkkio added a commit to AriPerkkio/vitest that referenced this issue Oct 21, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2023
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants