test: type the install option helpers so a stale key cannot pass (#13951)

A test's options object is the entire input to the subject under test, so a
key nothing reads does not fail: it quietly runs the default instead of the
case the test is named for. Both install helpers accepted anything — the
headless one took `opts?: any`, and the install one takes `opts?: T & {...}`,
where an unknown key is absorbed into the inferred `T`. That is how the
`registries` -> `registriesByScope` rename left seven suites pointed at the
wrong registry with a green type-check, and only CI to say so.

The headless helper now takes a `Partial<HeadlessOptions>`. Everything that
surfaced was real:

- Seven fields `HeadlessOptions` requires and reads — `configByUri`,
  `globalVirtualStoreDir`, `pruneStore`, `sideEffectsCacheRead`/`Write`,
  `userAgent`, `virtualStoreDirMaxLength` — were never set, so every headless
  test ran with them `undefined`. They now carry the values the install
  behaved as.
- `verifyStoreIntegrity` is a package-store option, not a headless one. The
  three tests that "disable" it were setting a key nobody read; it is now
  forwarded to the store.
- `hoistPattern: '*'` and `publicHoistPattern: '*'` are `string[]`. A
  one-character string iterates like a one-element array, which is the only
  reason they worked.
- `development`, `optional`, `production` predate `include`, which the same
  calls already set, and `sideEffectsCache` predates the read/write split.

Constraining the install helper's `T` is a bigger job — 162 errors, mostly
unrelated to options keys — so it keeps its generic and gains a narrow guard
instead: the three renamed keys are declared with a literal type that names
the replacement. It caught one more live case on the way in.

.changeset/typed-test-option-helpers.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/testing.temp-store": patch
+---
+
+`createTempStore`'s `storeOptions` are typed as a partial, which is how they are used: they are spread over the store's own defaults, so a caller that overrides one knob no longer has to restate the other five.

pnpm11/installing/deps-installer/test/lockfile.ts

@@ -466,7 +466,7 @@ test('scoped module from different registry', async () => {
     '@zkochan': `http://localhost:${REGISTRY_MOCK_PORT}`,
     '@foo': `http://localhost:${REGISTRY_MOCK_PORT}`,
   }
-  await addDependenciesToPackage({}, ['@zkochan/foo', '@foo/has-dep-from-same-scope', 'is-positive'], testDefaults({ registries }, { registries }))
+  await addDependenciesToPackage({}, ['@zkochan/foo', '@foo/has-dep-from-same-scope', 'is-positive'], testDefaults({ registriesByScope: registries }, { registriesByScope: registries }))
 
   project.has('@zkochan/foo')
 

pnpm11/installing/deps-installer/test/utils/testDefaults.ts

@@ -19,6 +19,21 @@ export function testDefaults<T> (
     minimumReleaseAge?: number
     minimumReleaseAgeStrict?: boolean
     minimumReleaseAgeExclude?: string[]
+    /**
+     * Renamed to `registriesByScope`, and kept here so an options object that
+     * still carries the old key fails to compile: `T` is inferred from the
+     * argument, so an unknown key is otherwise absorbed into it and the test
+     * silently exercises the default registry instead.
+     *
+     * Typed as the replacement's name rather than `never` so the compiler
+     * prints the fix — `not assignable to type '… & "renamed: use
+     * registriesByScope"'`.
+     */
+    registries?: 'renamed: use registriesByScope'
+    /** Renamed to `registriesByPrefix`. See `registries` above. */
+    namedRegistries?: 'renamed: use registriesByPrefix'
+    /** Renamed to `registryOptionsByUrl`. See `registries` above. */
+    registryOptions?: 'renamed: use registryOptionsByUrl'
   },
   resolveOpts?: any, // eslint-disable-line
   fetchOpts?: any, // eslint-disable-line

pnpm11/installing/deps-restorer/test/index.ts

@@ -301,15 +301,12 @@ test('installing only optional deps', async () => {
   const prefix = f.prepare('simple')
 
   await headlessInstall(await testDefaults({
-    development: false,
     include: {
       dependencies: false,
       devDependencies: false,
       optionalDependencies: true,
     },
     lockfileDir: prefix,
-    optional: true,
-    production: false,
   }))
 
   const project = assertProject(prefix)
@@ -536,7 +533,7 @@ test('installing using passed in lockfile files', async () => {
 
   await headlessInstall(await testDefaults({
     lockfileDir: prefix,
-    wantedLockfile,
+    wantedLockfile: wantedLockfile ?? undefined,
   }))
 
   const project = assertProject(prefix)
@@ -570,7 +567,7 @@ test('installing with hoistPattern=*', async () => {
   const prefix = prepareFixtureWithIntegrity('simple-shamefully-flatten')
   const reporter = jest.fn()
 
-  await headlessInstall(await testDefaults({ lockfileDir: prefix, reporter, hoistPattern: '*' }))
+  await headlessInstall(await testDefaults({ lockfileDir: prefix, reporter, hoistPattern: ['*'] }))
 
   const project = assertProject(prefix)
   expect(project.requireModule('is-positive')).toBeTruthy()
@@ -629,7 +626,7 @@ test('installing with publicHoistPattern=*', async () => {
   const prefix = prepareFixtureWithIntegrity('simple-shamefully-flatten')
   const reporter = jest.fn()
 
-  await headlessInstall(await testDefaults({ lockfileDir: prefix, reporter, publicHoistPattern: '*' }))
+  await headlessInstall(await testDefaults({ lockfileDir: prefix, reporter, publicHoistPattern: ['*'] }))
 
   const project = assertProject(prefix)
   expect(project.requireModule('is-positive')).toBeTruthy()
@@ -694,7 +691,7 @@ test('installing with publicHoistPattern=* in a project with external lockfile',
   await headlessInstall(await testDefaults({
     lockfileDir,
     projects: [prefix],
-    publicHoistPattern: '*',
+    publicHoistPattern: ['*'],
   }))
 
   const project = assertProject(lockfileDir)
@@ -703,7 +700,7 @@ test('installing with publicHoistPattern=* in a project with external lockfile',
 
 const ENGINE_DIR = `${process.platform}-${process.arch}-node-${process.version.split('.')[0]}`
 
-test.each([['isolated'], ['hoisted']])('using side effects cache with nodeLinker=%s', async (nodeLinker) => {
+test.each([['isolated'], ['hoisted']] as const)('using side effects cache with nodeLinker=%s', async (nodeLinker) => {
   let prefix = prepareFixtureWithIntegrity('side-effects')
 
   // Right now, hardlink does not work with side effects, so we specify copy as the packageImportMethod
@@ -764,7 +761,7 @@ test.skip('using side effects cache and hoistPattern=*', async () => {
   // Right now, hardlink does not work with side effects, so we specify copy as the packageImportMethod
   // We disable verifyStoreIntegrity because we are going to change the cache
   const opts = await testDefaults({
-    hoistPattern: '*',
+    hoistPattern: ['*'],
     lockfileDir,
     sideEffectsCacheRead: true,
     sideEffectsCacheWrite: true,

pnpm11/installing/deps-restorer/test/utils/testDefaults.ts

@@ -6,25 +6,44 @@ import { safeReadPackageJsonFromDir } from '@pnpm/pkg-manifest.reader'
 import { getStorePath } from '@pnpm/store.path'
 import { REGISTRY_MOCK_PORT } from '@pnpm/testing.registry-mock'
 import { createTempStore } from '@pnpm/testing.temp-store'
+import type { DepPath, ProjectRootDir } from '@pnpm/types'
 import { temporaryDirectory } from 'tempy'
 
 const registry = `http://localhost:${REGISTRY_MOCK_PORT}/`
 
+/**
+ * The options a test may override, on top of what this helper fills in.
+ *
+ * Typed rather than `any` so a key the headless install does not read is a
+ * compile error here: an options object is the whole input to the subject
+ * under test, and a misspelled or renamed key in one silently exercises the
+ * default instead of the case the test is named for.
+ */
+export type TestHeadlessOptions = Partial<HeadlessOptions> & {
+  /** Project directories, expanded into `HeadlessOptions.projects`. */
+  projects?: string[]
+  /**
+   * Forwarded to the package store, which is what reads it — the headless
+   * install itself has no such option.
+   */
+  verifyStoreIntegrity?: boolean
+}
+
 export async function testDefaults (
-  opts?: any, // eslint-disable-line
-  resolveOpts?: any, // eslint-disable-line
-  fetchOpts?: any, // eslint-disable-line
-  storeOpts?: any, // eslint-disable-line
+  opts?: TestHeadlessOptions,
+  resolveOpts?: Record<string, unknown>,
+  fetchOpts?: Record<string, unknown>,
+  storeOpts?: Record<string, unknown>
 ): Promise<HeadlessOptions> {
   const tmp = temporaryDirectory()
   let storeDir = opts?.storeDir ?? path.join(tmp, 'store')
   const lockfileDir = opts?.lockfileDir ?? process.cwd()
   const { include, pendingBuilds, projects } = await readProjectsContext(
-    opts.projects
-      ? opts.projects.map((rootDir: string) => ({ rootDir }))
+    opts?.projects
+      ? opts.projects.map((rootDir) => ({ rootDir: rootDir as ProjectRootDir }))
       : [
         {
-          rootDir: lockfileDir,
+          rootDir: lockfileDir as ProjectRootDir,
         },
       ],
     { lockfileDir }
@@ -41,7 +60,13 @@ export async function testDefaults (
         ...resolveOpts,
         ...fetchOpts,
       },
-      storeOptions: storeOpts,
+      storeOptions: {
+        // The package store reads this, not the headless install.
+        ...(opts?.verifyStoreIntegrity != null
+          ? { verifyStoreIntegrity: opts.verifyStoreIntegrity }
+          : {}),
+        ...storeOpts,
+      },
     }
   )
   return {
@@ -60,20 +85,25 @@ export async function testDefaults (
       version: '1.0.0',
     },
     pendingBuilds,
-    selectedProjectDirs: opts.selectedProjectDirs ?? projects.map((project) => project.rootDir),
+    selectedProjectDirs: opts?.selectedProjectDirs ?? projects.map((project) => project.rootDir),
     allProjects: Object.fromEntries(
       await Promise.all(projects.map(async (project) => [project.rootDir, { ...project, manifest: await safeReadPackageJsonFromDir(project.rootDir) }]))
     ),
-    authConfig: {},
     registriesByScope: {
       default: registry,
     },
-    sideEffectsCache: true,
-    skipped: new Set<string>(),
+    skipped: new Set<DepPath>(),
     storeController,
     storeDir,
+    configByUri: {},
+    globalVirtualStoreDir: path.join(storeDir, 'links'),
+    ignoreScripts: false,
+    pruneStore: false,
+    sideEffectsCacheRead: false,
+    sideEffectsCacheWrite: false,
+    userAgent: 'pnpm/0.0.0 npm/? node/0.0.0 test test',
+    virtualStoreDirMaxLength: process.platform === 'win32' ? 60 : 120,
     unsafePerm: true,
-    verifyStoreIntegrity: true,
     ...opts,
   }
 }

pnpm11/testing/temp-store/src/index.ts

@@ -20,7 +20,7 @@ export function createTempStore (opts?: {
   fastUnpack?: boolean
   storeDir?: string
   clientOptions?: Partial<ClientOptions>
-  storeOptions?: CreatePackageStoreOptions
+  storeOptions?: Partial<CreatePackageStoreOptions>
 }): CreateTempStoreResult {
   const configByUri: ClientOptions['configByUri'] = {}
   const cacheDir = path.resolve('cache')