Resolve PnP Virtuals when collecting test coverage

[noahnu]

🚀 Feature Proposal

Yarn Berry introduces the concept of a virtual package, whereby a source file may appear to Jest under a name different than the "real path". This happens when installing a workspace in a monorepo as a peer dependency of another workspace. As a result, when collecting coverage across the entire monorepo, if .yarn/__virtual__ is added to collectCoverageFrom, we will get more than one file with coverage reported, even though they point to the same underlying file in the filesystem.

I propose we resolve the virtuals to the real path, to allow coverage merging.

Motivation

I have a monorepo with workspaces monodeploy, @monodeploy/cli, @monodeploy/changelog.

monodeploy depends on @monodeploy/cli as a dependency, and @monodeploy/cli depends on @monodeploy/changelog as a peer dependency. This results in require.resolve('@monodeploy/changelog') pointing to a virtual path, e.g. .yarn/__virtual__/@monodeploy-changelog-virtual-699b9c0a11/1/packages/changelog/src.

I have tests that run in each workspace. The tests colocated in @monodeploy/changelog will generate coverage for <rootDir>/packages/changelog/**/*.ts. The tests located in monodeploy which indirectly depend on @monodeploy/changelog will generate coverage for .yarn/__virtual__/@monodeploy-changelog-virtual.... I want these 2 coverage files to be merged, and listed under the real path.

Example

Ideally this would just work, no need for additional setup. Only requirement would be to instruct jest to collect coverage from the .yarn virtuals filepath.

Pitch

Why does this feature belong in the Jest core platform?

This is required in jest because it cannot be added anywhere else. Yarn is not aware of how it will be used, so it can't make an exception for jest's coverage.

Additional Comments

Here's an example of resolving a virtual path as it's used by the typescript yarn patch: https://github.com/yarnpkg/berry/blob/e53036a37f5763d2568a1f7465c9a7562e091129/.yarn/sdks/typescript/lib/tsserver.js#L40.

const pnpApi = require(`pnpapi`)

const mySourceFile = '<rootDir>/.yarn/__virtual__/.../my/file.ts'
pnpApi.resolveVirtual(mySourceFile)

---

I managed to get accurate reporting by adding this to FileCoverage in istanbul-lib-coverage/lib/file-coverage.js so perhaps this is out of scope of jest and should be moved to a feature request in istanbul? Or is there somewhere in jest where we call istanbul that we can pass a different name? (I'm using ts-jest for reference).

if (process.versions.pnp && this.data.path && this.data.path.includes('/__virtual__/')) {
        this.data.path = require('pnpapi').resolveVirtual(this.data.path);
}

Although the reporting was accurate, the report itself still showed the virtual path. I think perhaps we can transform the paths before passing it off into istanbul? So we can keep this in one place in jest. Perhaps the CoverageReporter file?

[noahnu]

Got this working with a Yarn patch on istanbul-lib-coverage, though as I mention in the issue, I think this should be hoisted into jest somehow:

diff --git a/lib/coverage-map.js b/lib/coverage-map.js
index 0a1ebd0a90f958cabf94481fc6ea2dff206bfeb2..36c26a7c423c6db22d2cf1a433d6775a7c0346ce 100644
--- a/lib/coverage-map.js
+++ b/lib/coverage-map.js
@@ -41,6 +41,16 @@ class CoverageMap {
         } else {
             this.data = loadMap(obj);
         }
+
+        if (process.versions.pnp && this.data) {
+            const pnpapi = require('pnpapi');
+            this.data = Object.fromEntries(Object.entries(this.data).map(([k, data]) => {
+                if (k.includes('/__virtual__/')) {
+                    return [pnpapi.resolveVirtual(k), data];
+                }
+                return [k, data];
+            }));
+        }
     }

     /**
diff --git a/lib/file-coverage.js b/lib/file-coverage.js
index ed056a6f3bd7eaa7bd38b2778c029be5b422648f..2b7d43454f0e3e83a479237fdff60cc48f0601a1 100644
--- a/lib/file-coverage.js
+++ b/lib/file-coverage.js
@@ -76,6 +76,10 @@ class FileCoverage {
             throw new Error('Invalid argument to coverage constructor');
         }
         assertValidObject(this.data);
+
+        if (process.versions.pnp && this.data.path && this.data.path.includes('/__virtual__/')) {
+            this.data.path = require('pnpapi').resolveVirtual(this.data.path);
+        }
     }

     /**

[github-actions[bot]]

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

[noahnu]

I'd definitely like this directly addressed, though my patch has been working quite well. I still don't believe istanbul is the right place to do this.

[github-actions[bot]]

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

[noahnu]

Feature request is still relevant I believe.

[github-actions[bot]]

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

[noahnu]

Still relevant