Refactors tests to abstract implementation details (yarnpkg#6711)

arcanis · web-flow · commit 6b3f3e70a66c · 2025-03-05T10:03:47.000+01:00
## What's the problem this PR addresses? Some of our tests rely on implementation details which can make refactoring more difficult. ## How did you fix it? This diff attempts to remove checks for things that don't directly match the test description, by using standard APIs as much as possible. For example, if a test wants to read a file from a dependency, it should do so by calling `require.resolve` to find out the location of this file rather than by hardcoding it. ## Checklist    - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing).   - [x] I have set the packages that need to be released for my changes to be effective.   - [x] I will check that all automated PR checks pass before the PR gets reviewed.
diff --git a/.gitignore b/.gitignore
@@ -30,6 +30,7 @@ package.tgz
 
 # Those files are meant to be local-only
 /packages/*/bundles
+/packages/yarnpkg-pnp/sources/hook.raw.js
 
 # Those folders are meant to contain the prepack build artifacts; we don't commit them
 /packages/*/lib/*
diff --git a/.yarn/versions/744c99cf.yml b/.yarn/versions/744c99cf.yml
@@ -0,0 +1,2 @@
+declined:
+  - "@yarnpkg/pnp"
diff --git a/packages/acceptance-tests/pkg-tests-core/sources/utils/makeTemporaryEnv.ts b/packages/acceptance-tests/pkg-tests-core/sources/utils/makeTemporaryEnv.ts
@@ -37,6 +37,7 @@ const mte = generatePkgDriver({
         [`HOME`]: nativeHomePath,
         [`USERPROFILE`]: nativeHomePath,
         [`PATH`]: `${nativePath}/bin${delimiter}${process.env.PATH}`,
+        [`RUST_BACKTRACE`]: `1`,
         [`YARN_IS_TEST_ENV`]: `true`,
         [`YARN_GLOBAL_FOLDER`]: `${nativePath}/.yarn/global`,
         [`YARN_NPM_REGISTRY_SERVER`]: registryUrl,
diff --git a/packages/acceptance-tests/pkg-tests-core/sources/utils/tests.ts b/packages/acceptance-tests/pkg-tests-core/sources/utils/tests.ts
@@ -51,6 +51,7 @@ export enum RequestType {
   Login = `login`,
   PackageInfo = `packageInfo`,
   PackageTarball = `packageTarball`,
+  PackageVersion = `packageVersion`,
   Whoami = `whoami`,
   Repository = `repository`,
   Publish = `publish`,
@@ -66,6 +67,12 @@ export type Request = {
   type: RequestType.PackageInfo;
   scope?: string;
   localName: string;
+} | {
+  registry?: string;
+  type: RequestType.PackageVersion;
+  scope?: string;
+  localName: string;
+  version: string;
 } | {
   registry?: string;
   type: RequestType.PackageTarball;
@@ -409,6 +416,37 @@ export const startPackageServer = ({type}: {type: keyof typeof packageServerUrls
       response.end(data);
     },
 
+    async [RequestType.PackageVersion](parsedRequest, request, response) {
+      if (parsedRequest.type !== RequestType.PackageVersion)
+        throw new Error(`Assertion failed: Invalid request type`);
+
+      const {scope, localName, version} = parsedRequest;
+      const name = scope ? `${scope}/${localName}` : localName;
+
+      const packageEntry = await getPackageEntry(name);
+      if (!packageEntry) {
+        processError(response, 404, `Package not found: ${name}`);
+        return;
+      }
+
+      const packageVersionEntry = packageEntry.get(version);
+      invariant(packageVersionEntry, `This can only exist`);
+
+      const data = JSON.stringify({
+        [version as string]: Object.assign({}, packageVersionEntry!.packageJson, {
+          dist: {
+            shasum: await getPackageArchiveHash(name, version),
+            tarball: (localName === `unconventional-tarball` || localName === `private-unconventional-tarball`)
+              ? (await getPackageHttpArchivePath(name, version)).replace(`/-/`, `/tralala/`)
+              : await getPackageHttpArchivePath(name, version),
+          },
+        }),
+      });
+
+      response.writeHead(200, {[`Content-Type`]: `application/json`});
+      response.end(data);
+    },
+
     async [RequestType.PackageTarball](parsedRequest, request, response) {
       if (parsedRequest.type !== RequestType.PackageTarball)
         throw new Error(`Assertion failed: Invalid request type`);
@@ -628,6 +666,16 @@ export const startPackageServer = ({type}: {type: keyof typeof packageServerUrls
           scope,
           localName,
         };
+      } else if ((match = url.match(/^\/(?:(@[^/]+)\/)?([^@/][^/]*)\/([0-9]+\.[0-9]+\.[0-9]+(?:-[^/]+)?)$/))) {
+        const [, scope, localName, version] = match;
+
+        return {
+          ...registry,
+          type: RequestType.PackageVersion,
+          scope,
+          localName,
+          version,
+        };
       } else if ((match = url.match(/^\/(?:(@[^/]+)\/)?([^@/][^/]*)\/(-|tralala)\/\2-(.*)\.tgz(\?.*)?$/))) {
         const [, scope, localName, split, version] = match;
 
diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/features/berryLockfile.test.ts b/packages/acceptance-tests/pkg-tests-specs/sources/features/berryLockfile.test.ts
@@ -0,0 +1,32 @@
+import {ppath, xfs} from '@yarnpkg/fslib';
+
+describe(`Features`, () => {
+  describe(`Berry Lockfile`, () => {
+    test(`it should correctly import the lockfile from a Berry install`, makeTemporaryEnv({
+      dependencies: {[`no-deps`]: `^1.0.0`},
+    }, async ({path, run, source}) => {
+      await xfs.writeFilePromise(ppath.join(path, `yarn.lock`), [
+        `# This file is generated by running "yarn install" inside your project.\n`,
+        `# Manual changes might be lost - proceed with caution!\n`,
+        `\n`,
+        `__metadata:\n`,
+        `  version: 8\n`,
+        `  cacheKey: 0c0\n`,
+        `\n`,
+        `"no-deps@npm:^1.0.0":\n`,
+        `  version: 1.0.0\n`,
+        `  resolution: "no-deps@npm:1.0.0"\n`,
+        `  checksum: 0c0/af041f19ffad13c40b220f0319b95436c0e886ddb630988d78e35eb3b4e3d64131f1e7e9efa5818a7f3307718e322220bda7676fcc202f1841e587aa079b7205\n`,
+        `  languageName: node\n`,
+        `  linkType: hard\n`,
+      ].join(``));
+
+      await run(`install`);
+
+      await expect(source(`require('no-deps')`)).resolves.toMatchObject({
+        name: `no-deps`,
+        version: `1.0.0`,
+      });
+    }));
+  });
+});
diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/features/resolutions.test.js b/packages/acceptance-tests/pkg-tests-specs/sources/features/resolutions.test.js
@@ -1,4 +1,4 @@
-import {xfs} from '@yarnpkg/fslib';
+import {ppath, xfs} from '@yarnpkg/fslib';
 
 const {
   fs: {writeFile, writeJson},
@@ -109,6 +109,56 @@ describe(`Features`, () => {
       ),
     );
 
+    test(
+      `it should support overriding a packages with another, but only if its parent package has a specific version`,
+      makeTemporaryEnv(
+        {
+          dependencies: {
+            [`one-fixed-dep`]: `1.0.0`,
+          },
+          resolutions: {
+            [`one-fixed-dep@1.0.0/no-deps`]: `1.0.1`,
+          },
+        },
+        async ({path, run, source}) => {
+          await run(`install`);
+
+          await expect(source(`require('one-fixed-dep')`)).resolves.toMatchObject({
+            name: `one-fixed-dep`,
+            version: `1.0.0`,
+            dependencies: {
+              [`no-deps`]: {
+                name: `no-deps`,
+                version: `1.0.1`,
+              },
+            },
+          });
+
+          await xfs.writeJsonPromise(ppath.join(path, `package.json`), {
+            dependencies: {
+              [`one-fixed-dep`]: `2.0.0`,
+            },
+            resolutions: {
+              [`one-fixed-dep@1.0.0/no-deps`]: `1.0.1`,
+            },
+          });
+
+          await run(`install`);
+
+          await expect(source(`require('one-fixed-dep')`)).resolves.toMatchObject({
+            name: `one-fixed-dep`,
+            version: `2.0.0`,
+            dependencies: {
+              [`no-deps`]: {
+                name: `no-deps`,
+                version: `2.0.0`,
+              },
+            },
+          });
+        },
+      ),
+    );
+
     test(
       `it should support overriding packages with portals`,
       makeTemporaryEnv(
@@ -141,7 +191,7 @@ describe(`Features`, () => {
     );
 
     test(
-      `it should error when legacy glob syntax is used`,
+      `it should warn when legacy glob syntax is used`,
       makeTemporaryEnv(
         {
           resolutions: {
diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/lock.test.ts b/packages/acceptance-tests/pkg-tests-specs/sources/lock.test.ts
@@ -31,31 +31,58 @@ describe(`Lock tests`, () => {
     `it shouldn't loose track of the resolutions when upgrading the lockfile version`,
     makeTemporaryEnv(
       {
-        dependencies: {[`no-deps`]: `patch:no-deps@^1.0.0#`},
+        dependencies: {[`one-range-dep`]: `1.0.0`},
       },
       async ({path, run, source}) => {
         await setPackageWhitelist(new Map([[`no-deps`, new Set([`1.0.0`])]]), async () => {
           await run(`install`);
         });
 
-        const lockfilePath = ppath.join(path, Filename.lockfile);
-        const lockfile = await xfs.readFilePromise(lockfilePath, `utf8`);
-        const expectedLockfile = lockfile.replace(/(__metadata:[\t\r\n ]*version: )([0-9]+)/, ($0, $1, $2) => `${$1}${parseInt($2, 10) + 1}`);
-
-        // Sanity check to be sure that the test does something
-        expect(expectedLockfile).not.toEqual(lockfile);
+        await expect(source(`require('one-range-dep')`)).resolves.toMatchObject({
+          name: `one-range-dep`,
+          version: `1.0.0`,
+          dependencies: {
+            [`no-deps`]: {
+              name: `no-deps`,
+              version: `1.0.0`,
+            },
+          },
+        });
 
         await setPackageWhitelist(new Map([[`no-deps`, new Set([`1.0.0`, `1.1.0`])]]), async () => {
           await run(`install`, {
             lockfileVersionOverride: LOCKFILE_VERSION + 1,
           });
         });
 
-        await expect(xfs.readFilePromise(lockfilePath, `utf8`)).resolves.toEqual(expectedLockfile);
+        await expect(source(`require('one-range-dep')`)).resolves.toMatchObject({
+          name: `one-range-dep`,
+          version: `1.0.0`,
+          dependencies: {
+            [`no-deps`]: {
+              name: `no-deps`,
+              version: `1.0.0`,
+            },
+          },
+        });
 
-        await expect(source(`require('no-deps')`)).resolves.toMatchObject({
-          name: `no-deps`,
+        await xfs.rmPromise(ppath.join(path, Filename.lockfile));
+
+        await setPackageWhitelist(new Map([[`no-deps`, new Set([`1.0.0`, `1.1.0`])]]), async () => {
+          await run(`install`, {
+            lockfileVersionOverride: LOCKFILE_VERSION + 2,
+          });
+        });
+
+        await expect(source(`require('one-range-dep')`)).resolves.toMatchObject({
+          name: `one-range-dep`,
           version: `1.0.0`,
+          dependencies: {
+            [`no-deps`]: {
+              name: `no-deps`,
+              version: `1.1.0`,
+            },
+          },
         });
       },
     ),
diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/pnp.test.js b/packages/acceptance-tests/pkg-tests-specs/sources/pnp.test.js
@@ -1631,20 +1631,18 @@ describe(`Plug'n'Play`, () => {
         },
       },
       async ({path, run, source}) => {
+        await writeFile(`${path}/dep/index.js`, `module.exports = require('./package.json');`);
+
         await writeJson(npath.toPortablePath(`${path}/dep/package.json`), {
           name: `dep`,
           version: `1.0.0`,
           os: [`!${process.platform}`],
           scripts: {
-            postinstall: `echo 'Shall not be run'`,
+            postinstall: `echo 'module.exports = 42;' > index.js`,
           },
         });
-        await writeFile(`${path}/dep/index.js`, `module.exports = require('./package.json');`);
-
-        const stdout = (await run(`install`)).stdout;
 
-        expect(stdout).not.toContain(`Shall not be run`);
-        expect(stdout).toMatch(new RegExp(`dep@file:./dep.*The ${process.platform}-${process.arch}(-[a-z]+)? architecture is incompatible with this package, build skipped.`));
+        await run(`install`);
 
         await expect(source(`require('dep')`)).resolves.toMatchObject({
           name: `dep`,
@@ -1956,22 +1954,17 @@ describe(`Plug'n'Play`, () => {
         },
       },
       async ({path, run, source}) => {
-        await expect(run(`install`)).resolves.toMatchObject({
-          code: 0,
-          stdout: expect.stringContaining(`YN0007`),
-        });
-
-        await expect(run(`install`)).resolves.toMatchObject({
-          code: 0,
-          stdout: expect.not.stringContaining(`YN0007`),
-        });
+        await run(`install`);
 
         await xfs.removePromise(ppath.join(path, `.yarn/unplugged`));
 
-        await expect(run(`install`)).resolves.toMatchObject({
-          code: 0,
-          stdout: expect.stringContaining(`YN0007`),
-        });
+        await run(`install`);
+
+        await expect(source(`require('no-deps-scripted/log')`)).resolves.toEqual([
+          `preinstall`,
+          `install`,
+          `postinstall`,
+        ]);
       },
     ),
   );
diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/pnpapi.test.ts b/packages/acceptance-tests/pkg-tests-specs/sources/pnpapi.test.ts
@@ -84,7 +84,7 @@ describe(`Plug'n'Play API`, () => {
           await run(`install`);
 
           await expect(
-            source(`typeof require('pnpapi').getPackageInformation({name: null, reference: null}).packageLocation`),
+            source(`typeof require('pnpapi').getPackageInformation(require('pnpapi').topLevel).packageLocation`),
           ).resolves.toEqual(
             `string`,
           );
@@ -102,10 +102,10 @@ describe(`Plug'n'Play API`, () => {
           await run(`install`);
 
           await expect(
-            source(`[...require('pnpapi').getPackageInformation({name: null, reference: null}).packageDependencies]`),
+            source(`[...require('pnpapi').getPackageInformation(require('pnpapi').topLevel).packageDependencies].sort((a, b) => a[0].localeCompare(b[0]))`),
           ).resolves.toEqual([
             [`no-deps`, `npm:1.0.0`],
-            [`root-workspace`, `workspace:.`],
+            [`root-workspace`, expect.stringContaining(`workspace:`)],
           ]);
         }),
       );
@@ -165,8 +165,9 @@ describe(`Plug'n'Play API`, () => {
 
           await expect(
             source(`{
+              const path = require('path');
               const pnp = require('pnpapi');
-              const deps = pnp.getPackageInformation({name: 'foo', reference: 'workspace:packages/foo'}).packageDependencies;
+              const deps = pnp.getPackageInformation(pnp.findPackageLocator(path.resolve('packages/foo/package.json'))).packageDependencies;
               return [...pnp.getPackageInformation(pnp.getLocator('bar', deps.get('bar'))).packagePeers];
             }`),
           ).resolves.toEqual([
@@ -208,8 +209,9 @@ describe(`Plug'n'Play API`, () => {
 
           await expect(
             source(`{
+              const path = require('path');
               const pnp = require('pnpapi');
-              return [...pnp.getPackageInformation({name: 'bar', reference: 'workspace:packages/bar'}).packagePeers || []];
+              return [...pnp.getPackageInformation(pnp.findPackageLocator(path.resolve('packages/bar'))).packagePeers || []];
             }`),
           ).resolves.toEqual([]);
         }),
@@ -338,7 +340,7 @@ describe(`Plug'n'Play API`, () => {
           await run(`install`);
 
           const reference = await source(
-            `require('pnpapi').getPackageInformation({name: null, reference: null}).packageDependencies.get('no-deps')`,
+            `require('pnpapi').getPackageInformation(require('pnpapi').topLevel).packageDependencies.get('no-deps')`,
           );
 
           await expect(
@@ -359,7 +361,7 @@ describe(`Plug'n'Play API`, () => {
           await run(`install`);
 
           const reference = await source(
-            `require('pnpapi').getPackageInformation({name: null, reference: null}).packageDependencies.get('self')`,
+            `require('pnpapi').getPackageInformation(require('pnpapi').topLevel).packageDependencies.get('self')`,
           );
 
           await expect(
diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/protocols/__snapshots__/git.test.ts.snap b/packages/acceptance-tests/pkg-tests-specs/sources/protocols/__snapshots__/git.test.ts.snap
diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/protocols/git.test.ts b/packages/acceptance-tests/pkg-tests-specs/sources/protocols/git.test.ts
diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/script.test.ts b/packages/acceptance-tests/pkg-tests-specs/sources/script.test.ts
diff --git a/packages/yarnpkg-pnp/rollup.config.js b/packages/yarnpkg-pnp/rollup.config.js
