From 4b97cefb7e7a2310ad80373d7abad35de82b9748 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov <zharinov@users.noreply.github.com> Date: Mon, 30 Sep 2024 02:26:53 -0300 Subject: [PATCH] test: Improve errors for missing/unused HTTP mocks (#31678) --- lib/modules/datasource/npm/get.spec.ts | 3 + lib/modules/datasource/readme.md | 2 +- test/http-mock.ts | 216 +++++++++++++++++++++---- 3 files changed, 185 insertions(+), 36 deletions(-) diff --git a/lib/modules/datasource/npm/get.spec.ts b/lib/modules/datasource/npm/get.spec.ts index b8a3809bc4..7479a06063 100644 --- a/lib/modules/datasource/npm/get.spec.ts +++ b/lib/modules/datasource/npm/get.spec.ts @@ -366,6 +366,7 @@ describe('modules/datasource/npm/get', () => { "user-agent": "RenovateBot/0.0.0-semantic-release (https://github.com/renovatebot/renovate)", }, "method": "GET", + "status": 200, "url": "https://test.org/@neutrinojs%2Freact", }, ] @@ -512,6 +513,7 @@ describe('modules/datasource/npm/get', () => { "user-agent": "RenovateBot/0.0.0-semantic-release (https://github.com/renovatebot/renovate)", }, "method": "GET", + "status": 200, "url": "https://test.org/@neutrinojs%2Freact", }, ] @@ -552,6 +554,7 @@ describe('modules/datasource/npm/get', () => { "user-agent": "RenovateBot/0.0.0-semantic-release (https://github.com/renovatebot/renovate)", }, "method": "GET", + "status": 200, "url": "https://test.org/@neutrinojs%2Freact", }, ] diff --git a/lib/modules/datasource/readme.md b/lib/modules/datasource/readme.md index ba2e625aa7..39e22f0202 100644 --- a/lib/modules/datasource/readme.md +++ b/lib/modules/datasource/readme.md @@ -8,7 +8,7 @@ New datasources _must_ follow the class-based programming style. Use the `java-version` datasource as a reference. Add the datasource to the API in [`api.ts`](api.ts) so that the new datasource is usable. -If you find `Pending mocks!` errors in the Jest tests _and_ your mocked URLs are correct, make sure the datasource is correctly registered. +If you find `Unused HTTP mocks` errors in the Jest tests _and_ your mocked URLs are correct, make sure the datasource is correctly registered. ## getReleases diff --git a/test/http-mock.ts b/test/http-mock.ts index 4dabebd87a..009d892eb9 100644 --- a/test/http-mock.ts +++ b/test/http-mock.ts @@ -1,5 +1,7 @@ import type { Url } from 'node:url'; import { afterAll, afterEach, beforeAll } from '@jest/globals'; +import { codeBlock } from 'common-tags'; +import fs from 'fs-extra'; // eslint-disable-next-line no-restricted-imports import nock from 'nock'; import { makeGraphqlSnapshot } from './graphql-snapshot'; @@ -7,18 +9,24 @@ import { makeGraphqlSnapshot } from './graphql-snapshot'; // eslint-disable-next-line no-restricted-imports export type { Scope, ReplyHeaders, Body } from 'nock'; -interface RequestLogItem { +interface RequestLog { headers: Record<string, string>; method: string; url: string; + status: number; body?: any; graphql?: any; } +interface MissingRequestLog { + method: string; + url: string; +} + type BasePath = string | RegExp | Url; -let requestLog: RequestLogItem[] = []; -let missingLog: string[] = []; +let requestsDone: RequestLog[] = []; +let requestsMissing: MissingRequestLog[] = []; type TestRequest = { method: string; @@ -27,9 +35,9 @@ type TestRequest = { function onMissing(req: TestRequest, opts?: TestRequest): void { if (opts) { - missingLog.push(` ${opts.method} ${opts.href}`); + requestsMissing.push({ method: opts.method, url: opts.href }); } else { - missingLog.push(` ${req.method} ${req.href}`); + requestsMissing.push({ method: req.method, url: req.href }); } } @@ -37,31 +45,52 @@ export function allUsed(): boolean { return nock.isDone(); } +function getPending(): string[] { + return nock.pendingMocks().map((req) => `- ${req.replace(':443/', '/')}`); +} + /** * Clear nock state. Will be called in `afterEach` - * @argument throwOnPending Use `false` to simply clear mocks. + * + * @argument check Use `false` to clear mocks without checking for the missing/unused ones. + * Disabling such checks is discouraged. */ -export function clear(throwOnPending = true): void { +export function clear(check = true): void { const isDone = nock.isDone(); - const pending = nock.pendingMocks(); + const pending = getPending(); + nock.abortPendingRequests(); nock.cleanAll(); - const missing = missingLog; - requestLog = []; - missingLog = []; - if (missing.length && throwOnPending) { - throw new Error(`Missing mocks!\n * ${missing.join('\n * ')}`); + + const done = requestsDone; + requestsDone = []; + + const missing = requestsMissing; + requestsMissing = []; + + if (!check) { + return; } - if (!isDone && throwOnPending) { - throw new Error(`Pending mocks!\n * ${pending.join('\n * ')}`); + + if (missing.length) { + const err = new Error(missingHttpMockMessage(done, missing)); + massageHttpMockStacktrace(err); + throw err; + } + + if (!isDone) { + const err = new Error(unusedHttpMockMessage(done, pending)); + massageHttpMockStacktrace(err); + throw err; } } export function scope(basePath: BasePath, options?: nock.Options): nock.Scope { - return nock(basePath, options).on('request', (req) => { + return nock(basePath, options).on('replied', (req) => { const { headers, method } = req; const url = req.options?.href; - const result: RequestLogItem = { headers, method, url }; + const status = req.response?.statusCode; + const result: RequestLog = { headers, method, url, status }; const requestBody = req.requestBodyBuffers?.[0]?.toString(); if (requestBody && headers['content-type'] === 'application/json') { @@ -77,30 +106,147 @@ export function scope(basePath: BasePath, options?: nock.Options): nock.Scope { result.body = requestBody; } } - requestLog.push(result); + requestsDone.push(result); }); } -export function getTrace(): RequestLogItem[] /* istanbul ignore next */ { - const errorLines: string[] = []; - if (missingLog.length) { - errorLines.push('Missing mocks:'); - errorLines.push(...missingLog); +export function getTrace(): RequestLog[] { + return requestsDone; +} + +function massageHttpMockStacktrace(err: Error): void { + if (!err.stack) { + return; } - if (!nock.isDone()) { - errorLines.push('Unused mocks:'); - errorLines.push(...nock.pendingMocks().map((x) => ` ${x}`)); + + const state = expect.getState(); + if (!state.currentTestName || !state.testPath) { + return; + } + + const content = fs.readFileSync(state.testPath, { encoding: 'utf8' }); + + // Shrink the `testName` until we could locate it in the source file + let testName = state.currentTestName.replace(/^[^\s]*\s/, ''); + let idx = content.indexOf(testName); + while (testName.length) { + if (idx !== -1) { + break; + } + + const prevName = testName; + testName = testName.replace(/^[^\s]*\s/, ''); + if (prevName === testName) { + break; + } + + idx = content.indexOf(testName); + } + + if (idx === -1) { + return; + } + + const lines = content.slice(0, idx).split('\n'); + const lineNum = lines.length; + const linePos = lines[lines.length - 1].length + 1; + + const stackLine = ` at <test> (${state.testPath}:${lineNum}:${linePos})`; + err.stack = err.stack.replace(/\+\+\+.*$/s, stackLine); +} + +function missingHttpMockMessage( + done: RequestLog[], + missing: MissingRequestLog[], +): string { + const blocks: string[] = []; + + const title = codeBlock` + *** Missing HTTP mocks *** + `; + + const explanation = codeBlock` + --- + + Renovate testing strategy requires that every HTTP request + has a corresponding mock. + + This error occurs when some of the request aren't mocked. + + Let's suppose your code performs two HTTP calls: + + GET https://example.com/foo/bar/fail 404 <without body> + POST https://example.com/foo/bar/success 200 { "ok": true } + + The unit test should have this mock: + + httpMock.scope('https://example.com/foo/bar') + .get('/fail') + .reply(404) + .post('/success') + .reply(200, { ok: true }); + + Note: \`httpMock.scope(...)\` is the Renovate-specific construct. + The scope object itself is provided by the \`nock\` library. + + Details: https://github.com/nock/nock#usage + + +++ + `; + + blocks.push(title); + + blocks.push(codeBlock` + ${missing.map(({ method, url }) => `- ${method} ${url}`).join('\n')} + `); + + if (done.length) { + blocks.push(codeBlock` + Requests done: + + ${done.map(({ method, url, status }) => `- ${method} ${url} [${status}]`).join('\n')} + `); } - if (errorLines.length) { - throw new Error( - [ - 'Completed requests:', - ...requestLog.map(({ method, url }) => ` ${method} ${url}`), - ...errorLines, - ].join('\n'), - ); + + blocks.push(explanation); + + return blocks.join('\n\n'); +} + +function unusedHttpMockMessage(done: RequestLog[], pending: string[]): string { + const blocks: string[] = []; + + const title = codeBlock` + *** Unused HTTP mocks *** + `; + + const explanation = codeBlock` + --- + + Renovate testing strategy requires that every HTTP request + has a corresponding mock. + + This error occurs because some of the created mocks are unused. + + In most cases, you simply need to remove them. + + +++ + `; + + blocks.push(title); + blocks.push(pending.join('\n')); + + if (done.length) { + blocks.push(codeBlock` + Requests done: + + ${done.map(({ method, url, status }) => `- ${method} ${url} [${status}]`).join('\n')} + `); } - return requestLog; + + blocks.push(explanation); + + return blocks.join('\n\n'); } // init nock -- GitLab