diff --git a/user.js/tests/browser-shim.test.js b/user.js/tests/browser-shim.test.js new file mode 100644 index 0000000..80d5c35 --- /dev/null +++ b/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}'); +}); diff --git a/user.js/tests/helpers/browser-shim.js b/user.js/tests/helpers/browser-shim.js index c601032..0e63c89 100644 --- a/user.js/tests/helpers/browser-shim.js +++ b/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 };