test(build): make the fake fetch method-aware and header-readable

user.js/tests/browser-shim.test.js

@@ -0,0 +1,84 @@
+'use strict';
+
+// The test harness itself, pinned.
+//
+// This file exists because a gap in the harness -- not in the code under test
+// -- is what let a critical bug ship. lib/admincom-common.js's fetchWithRetry
+// reads response.headers.get('retry-after'), and the fake response used to
+// expose only forEach(), so any 503 fixture threw TypeError before reaching
+// the retry logic. The retry path was not merely untested; it was inexpressible.
+// A harness whose limits are invisible produces confident, empty coverage, so
+// the capabilities other tests depend on are asserted here directly.
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const path = require('node:path');
+const { loadScript, makeFakeResponse } = require('./helpers/browser-shim');
+
+const SCRIPT_PATH = path.join(__dirname, '..', 'peeringdb-deskpro-tools.user.js');
+
+test('fake response headers support get(), not just forEach()', () => {
+  const res = makeFakeResponse(503, {}, { 'Retry-After': '3' });
+
+  assert.equal(res.status, 503);
+  assert.equal(res.ok, false);
+  assert.equal(res.headers.get('Retry-After'), '3');
+  assert.equal(res.headers.get('retry-after'), '3', 'header lookup must be case-insensitive');
+  assert.equal(res.headers.get('absent'), null, 'a missing header reads as null, like the real API');
+  assert.equal(res.headers.has('retry-after'), true);
+});
+
+test('a 2xx status is ok, a 4xx/5xx is not', () => {
+  assert.equal(makeFakeResponse(200, {}).ok, true);
+  assert.equal(makeFakeResponse(204, {}).ok, true);
+  assert.equal(makeFakeResponse(404, {}).ok, false);
+  assert.equal(makeFakeResponse(503, {}).ok, false);
+});
+
+test('fetchMap accepts a response descriptor alongside a bare body', async () => {
+  const url = 'https://www.peeringdb.com/api/net/1';
+  const { window } = loadScript(SCRIPT_PATH, {
+    hooksKey: '__pdbDpTestHooks__',
+    pathname: '/app/ticket',
+    fetchMap: {
+      [url]: { __response: true, status: 503, body: { detail: 'busy' }, headers: { 'Retry-After': '1' } },
+      'https://www.peeringdb.com/api/net/2': { data: [{ id: 2 }] },
+    },
+  });
+
+  const failing = await window.fetch(url);
+  assert.equal(failing.status, 503);
+  assert.equal(failing.headers.get('retry-after'), '1');
+  assert.deepEqual(await failing.json(), { detail: 'busy' });
+
+  // The bare-body form must keep working unchanged -- every pre-existing test
+  // depends on it.
+  const ok = await window.fetch('https://www.peeringdb.com/api/net/2');
+  assert.equal(ok.status, 200);
+  assert.equal(ok.ok, true);
+  assert.deepEqual(await ok.json(), { data: [{ id: 2 }] });
+
+  // An unmapped URL still 404s rather than throwing.
+  const missing = await window.fetch('https://www.peeringdb.com/api/net/999');
+  assert.equal(missing.status, 404);
+});
+
+test('every request is recorded with its method, so retries are observable', async () => {
+  const url = 'https://www.peeringdb.com/api/netixlan/7';
+  const { window, fetchCalls } = loadScript(SCRIPT_PATH, {
+    hooksKey: '__pdbDpTestHooks__',
+    pathname: '/app/ticket',
+    fetchMap: { [url]: { data: [] } },
+  });
+
+  assert.equal(fetchCalls.length, 0, 'no requests before anything runs');
+
+  await window.fetch(url);
+  await window.fetch(url, { method: 'delete' });
+  await window.fetch(url, { method: 'PUT', body: '{"speed":1000}' });
+
+  assert.equal(fetchCalls.length, 3);
+  assert.deepEqual(fetchCalls.map((c) => c.method), ['GET', 'DELETE', 'PUT'], 'method is captured and upper-cased');
+  assert.equal(fetchCalls[0].url, url);
+  assert.equal(fetchCalls[2].body, '{"speed":1000}');
+});

user.js/tests/helpers/browser-shim.js

@@ -165,6 +165,11 @@ function loadScript(scriptPath, opts) {
     localStorage = makeFakeStorage(),
   } = opts;
 
+  // Every fake-fetch request lands here in order. Returned from loadScript so a
+  // test can assert request count and method -- the only observable difference
+  // between one request and a retried one.
+  const fetchCalls = [];
+
   const source = fs.readFileSync(scriptPath, 'utf-8');
 
   const fakeDocument = {
@@ -207,7 +212,7 @@ function loadScript(scriptPath, opts) {
     URL,
     URLSearchParams,
     AbortController,
-    fetch: makeFakeFetch(fetchMap),
+    fetch: makeFakeFetch(fetchMap, fetchCalls),
     MutationObserver: class {
       observe() {}
       disconnect() {}
@@ -231,7 +236,30 @@ function loadScript(scriptPath, opts) {
     throw new Error(`${path.basename(scriptPath)} did not expose window.${hooksKey} -- was window.__PDB_TEST__ wired up?`);
   }
 
-  return { window: sandbox, document: fakeDocument, hooks };
+  return { window: sandbox, document: fakeDocument, hooks, fetchCalls };
+}
+
+/**
+ * Builds a fake response object with headers real enough for production code
+ * to read. The previous version exposed only forEach(), so any code calling
+ * response.headers.get() -- which lib/admincom-common.js's fetchWithRetry does
+ * for Retry-After -- threw TypeError before its logic could be reached. That
+ * is why the retry/backoff path had no coverage: not oversight, but a harness
+ * that could not express the case.
+ */
+function makeFakeResponse(status, body, headers = {}) {
+  const lowered = new Map(Object.entries(headers).map(([k, v]) => [k.toLowerCase(), String(v)]));
+  return {
+    ok: status >= 200 && status < 300,
+    status,
+    headers: {
+      get: (name) => (lowered.has(String(name).toLowerCase()) ? lowered.get(String(name).toLowerCase()) : null),
+      has: (name) => lowered.has(String(name).toLowerCase()),
+      forEach: (cb) => lowered.forEach((v, k) => cb(v, k)),
+    },
+    json: async () => body,
+    text: async () => JSON.stringify(body),
+  };
 }
 
 /**
@@ -240,24 +268,43 @@ function loadScript(scriptPath, opts) {
  * Purpose: Let API-calling functions run for real (no manual mocking of the
  * function itself) while keeping tests fully offline/deterministic -- no
  * accidental live traffic, no network flakiness.
+ *
+ * A fetchMap value is normally the JSON body to return with a 200. To model a
+ * failure or a header-bearing response, use the descriptor form instead:
+ *
+ *   { __response: true, status: 503, body: {}, headers: { 'Retry-After': '1' } }
+ *
+ * Every call is appended to `calls` as { url, method, body, headers }, so a
+ * test can assert *how many* requests a function issued and with which method
+ * -- the only way to observe retry behavior, since a retried request is
+ * indistinguishable from a single one by its return value alone.
+ *
+ * @param {object} fetchMap - URL -> JSON body, or URL -> response descriptor.
+ * @param {object[]} calls - Array the fake appends each request to, in order.
  */
-function makeFakeFetch(fetchMap) {
-  return async (url) => {
+function makeFakeFetch(fetchMap, calls) {
+  return async (url, init = {}) => {
     const key = String(url);
+    calls.push({
+      url: key,
+      method: String(init.method || 'GET').toUpperCase(),
+      body: init.body,
+      headers: init.headers,
+    });
+
     if (!Object.prototype.hasOwnProperty.call(fetchMap, key)) {
-      return {
-        ok: false,
-        status: 404,
-        headers: { forEach() {} },
-        json: async () => ({}),
-      };
+      return makeFakeResponse(404, {});
+    }
+
+    const entry = fetchMap[key];
+    if (entry && typeof entry === 'object' && entry.__response === true) {
+      return makeFakeResponse(
+        typeof entry.status === 'number' ? entry.status : 200,
+        'body' in entry ? entry.body : {},
+        entry.headers || {},
+      );
     }
-    return {
-      ok: true,
-      status: 200,
-      headers: { forEach() {} },
-      json: async () => fetchMap[key],
-    };
+    return makeFakeResponse(200, entry);
   };
 }
 
@@ -282,4 +329,4 @@ function makeFakeStorage() {
   };
 }
 
-module.exports = { loadScript, el, FakeElement, FakeTextNode, FakeDocumentFragment, makeFakeStorage };
+module.exports = { loadScript, el, FakeElement, FakeTextNode, FakeDocumentFragment, makeFakeStorage, makeFakeResponse };