chore(libnpmexec): refactor tests to use mock registry

DEPENDENCIES.md

@@ -33,6 +33,7 @@ graph LR;
   libnpmexec-->npm-package-arg;
   libnpmexec-->npmcli-arborist["@npmcli/arborist"];
   libnpmexec-->npmcli-eslint-config["@npmcli/eslint-config"];
+  libnpmexec-->npmcli-mock-registry["@npmcli/mock-registry"];
   libnpmexec-->npmcli-run-script["@npmcli/run-script"];
   libnpmexec-->npmcli-template-oss["@npmcli/template-oss"];
   libnpmexec-->npmlog;
@@ -352,11 +353,14 @@ graph LR;
   libnpmexec-->bin-links;
   libnpmexec-->chalk;
   libnpmexec-->ci-info;
+  libnpmexec-->just-extend;
+  libnpmexec-->just-safe-set;
   libnpmexec-->minify-registry-metadata;
   libnpmexec-->mkdirp;
   libnpmexec-->npm-package-arg;
   libnpmexec-->npmcli-arborist["@npmcli/arborist"];
   libnpmexec-->npmcli-eslint-config["@npmcli/eslint-config"];
+  libnpmexec-->npmcli-mock-registry["@npmcli/mock-registry"];
   libnpmexec-->npmcli-run-script["@npmcli/run-script"];
   libnpmexec-->npmcli-template-oss["@npmcli/template-oss"];
   libnpmexec-->npmlog;
@@ -772,8 +776,8 @@ Each group depends on packages lower down the chain, nothing depends on
 packages higher up the chain.
 
  - npm
- - @npmcli/smoke-tests, libnpmpublish
- - @npmcli/mock-registry, libnpmdiff, libnpmexec, libnpmfund, libnpmpack
+ - @npmcli/smoke-tests, libnpmexec, libnpmpublish
+ - @npmcli/mock-registry, libnpmdiff, libnpmfund, libnpmpack
  - @npmcli/arborist
  - @npmcli/metavuln-calculator
  - pacote, libnpmaccess, libnpmhook, libnpmorg, libnpmsearch, libnpmteam, npm-profile

mock-registry/lib/index.js

@@ -10,6 +10,7 @@ class MockRegistry {
   #authorization
   #basic
   #debug
+  #strict
 
   constructor (opts) {
     if (!opts.registry) {
@@ -19,20 +20,48 @@ class MockRegistry {
     this.#authorization = opts.authorization
     this.#basic = opts.basic
     this.#debug = opts.debug
+    this.#strict = opts.strict
     // Required for this.package
     this.#tap = opts.tap
+    if (this.#tap) {
+      this.startNock()
+    }
   }
 
-  static tnock (t, host, opts, { debug = false } = {}) {
-    if (debug) {
-      Nock.emitter.on('no match', req => console.error('NO MATCH', req.options))
+  static tnock (t, host, opts, { debug = false, strict = false } = {}) {
+    const noMatch = (req) => {
+      if (strict) {
+        // There are network requests that get caught regardless of error code.
+        // Turning on strict mode requires that those requests get explicitly
+        // mocked with a 404, 500, etc.
+        // XXX: this is opt-in currently because it breaks some existing CLI
+        // tests. We should work towards making this the default for all tests.
+        t.fail(`Unmatched request: ${JSON.stringify(req.options, null, 2)}`)
+      }
+      if (debug) {
+        console.error('NO MATCH', t.name, req.options)
+      }
     }
+
+    Nock.emitter.on('no match', noMatch)
     Nock.disableNetConnect()
     const server = Nock(host, opts)
+
+    if (strict) {
+      // this requires that mocks not be shared between sub tests but it helps
+      // find mistakes quicker instead of waiting for the entire test to end
+      t.afterEach((t) => {
+        t.strictSame(server.pendingMocks(), [], 'no pending mocks after each')
+        t.strictSame(server.activeMocks(), [], 'no active mocks after each')
+      })
+    }
+
     t.teardown(() => {
       Nock.enableNetConnect()
       server.done()
+      Nock.emitter.off('no match', noMatch)
     })
+
     return server
   }
 
@@ -41,31 +70,38 @@ class MockRegistry {
   }
 
   get nock () {
-    if (!this.#nock) {
-      if (!this.#tap) {
-        throw new Error('cannot mock packages without a tap fixture')
-      }
-      const reqheaders = {}
-      if (this.#authorization) {
-        reqheaders.authorization = `Bearer ${this.#authorization}`
-      }
-      if (this.#basic) {
-        reqheaders.authorization = `Basic ${this.#basic}`
-      }
-      this.#nock = MockRegistry.tnock(
-        this.#tap,
-        this.#registry,
-        { reqheaders },
-        { debug: this.#debug }
-      )
-    }
     return this.#nock
   }
 
   set nock (nock) {
     this.#nock = nock
   }
 
+  startNock () {
+    if (this.nock) {
+      return
+    }
+
+    if (!this.#tap) {
+      throw new Error('cannot mock packages without a tap fixture')
+    }
+
+    const reqheaders = {}
+    if (this.#authorization) {
+      reqheaders.authorization = `Bearer ${this.#authorization}`
+    }
+    if (this.#basic) {
+      reqheaders.authorization = `Basic ${this.#basic}`
+    }
+
+    this.nock = MockRegistry.tnock(
+      this.#tap,
+      this.#registry,
+      { reqheaders },
+      { debug: this.#debug, strict: this.#strict }
+    )
+  }
+
   search ({ responseCode = 200, results = [], error }) {
     // the flags, score, and searchScore parts of the response are never used
     // by npm, only package is used
@@ -296,13 +332,14 @@ class MockRegistry {
       manifest.users = users
     }
     for (const packument of packuments) {
+      const unscoped = name.includes('/') ? name.split('/')[1] : name
       manifest.versions[packument.version] = {
         _id: `${name}@${packument.version}`,
         name,
         description: 'test package mock manifest',
         dependencies: {},
         dist: {
-          tarball: `${this.#registry}/${name}/-/${name}-${packument.version}.tgz`,
+          tarball: `${this.#registry}/${name}/-/${unscoped}-${packument.version}.tgz`,
         },
         maintainers: [],
         ...packument,

package-lock.json

@@ -15118,8 +15118,11 @@
       },
       "devDependencies": {
         "@npmcli/eslint-config": "^4.0.0",
+        "@npmcli/mock-registry": "^1.0.0",
         "@npmcli/template-oss": "4.10.0",
         "bin-links": "^4.0.1",
+        "just-extend": "^6.1.1",
+        "just-safe-set": "^4.1.1",
         "minify-registry-metadata": "^2.2.0",
         "mkdirp": "^1.0.4",
         "tap": "^16.0.1"

smoke-tests/test/fixtures/setup.js

@@ -84,6 +84,7 @@ module.exports = async (t, { testdir = {}, debug } = {}) => {
     tap: t,
     registry: 'http://smoke-test-registry.club/',
     debug,
+    strict: true,
   })
   const httpProxyRegistry = `http://localhost:${PORT}`
   const proxy = httpProxy.createProxyServer({})
@@ -92,12 +93,8 @@ module.exports = async (t, { testdir = {}, debug } = {}) => {
   t.teardown(() => server.close())
 
   // update notifier should never be written
-  t.afterEach(async (t) => {
+  t.afterEach((t) => {
     t.equal(existsSync(join(paths.cache, '_update-notifier-last-checked')), false)
-    // this requires that mocks not be shared between sub tests but it helps
-    // find mistakes quicker instead of waiting for the entire test to end
-    t.strictSame(registry.nock.pendingMocks(), [], 'no pending mocks after each')
-    t.strictSame(registry.nock.activeMocks(), [], 'no active mocks after each')
   })
 
   const debugLog = debug || CI ? (...a) => console.error(...a) : () => {}

workspaces/libnpmexec/package.json

@@ -51,8 +51,11 @@
   },
   "devDependencies": {
     "@npmcli/eslint-config": "^4.0.0",
+    "@npmcli/mock-registry": "^1.0.0",
     "@npmcli/template-oss": "4.10.0",
     "bin-links": "^4.0.1",
+    "just-extend": "^6.1.1",
+    "just-safe-set": "^4.1.1",
     "minify-registry-metadata": "^2.2.0",
     "mkdirp": "^1.0.4",
     "tap": "^16.0.1"