From 03547f9e76eecfeff4ea6b6f77ae0a5110f39f86 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 17 Aug 2026 14:31:13 +0200 Subject: [PATCH] fix(middleware-code-coverage): Drop Express coupling Add an integration test that mounts the middleware on a plain connect() app (no ui5 serve, no Express) to prove it depends only on the sanctioned Node.js http surface, and rewrite the middleware to satisfy it: - res.json(...) / res.err(...) -> res.writeHead + res.end(JSON.stringify(...)) - res.type(".js") -> res.setHeader("Content-Type", "text/javascript") - req.path / req.query -> new URL(req.url, "http://localhost") req.body is retained (populated by body-parser). --- package-lock.json | 113 ++++++++++++ .../ava-integration.config.js | 2 +- .../lib/middleware.js | 18 +- packages/middleware-code-coverage/lib/util.js | 14 +- .../middleware-code-coverage/package.json | 1 + .../test/integration/connect.js | 153 +++++++++++++++++ .../test/unit/lib/middleware.js | 162 ++++++++---------- .../test/unit/lib/util.js | 17 +- 8 files changed, 367 insertions(+), 113 deletions(-) create mode 100644 packages/middleware-code-coverage/test/integration/connect.js diff --git a/package-lock.json b/package-lock.json index 554ea0b8..0574af7c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10914,6 +10914,39 @@ "node": ">=10.18.0 <11 || >=12.14.0 <13 || >=14" } }, + "node_modules/connect": { + "version": "3.7.0", + "resolved": "https://registry.npmjs.org/connect/-/connect-3.7.0.tgz", + "integrity": "sha512-ZqRXc+tZukToSNmh5C2iWMSoV3X1YUcPbqEM4DkEG5tNQXrQUZCNVGGv3IuicnkMtPfGf3Xtp8WCXs295iQ1pQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "debug": "2.6.9", + "finalhandler": "1.1.2", + "parseurl": "~1.3.3", + "utils-merge": "1.0.1" + }, + "engines": { + "node": ">= 0.10.0" + } + }, + "node_modules/connect/node_modules/debug": { + "version": "2.6.9", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", + "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", + "dev": true, + "license": "MIT", + "dependencies": { + "ms": "2.0.0" + } + }, + "node_modules/connect/node_modules/ms": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", + "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==", + "dev": true, + "license": "MIT" + }, "node_modules/consola": { "version": "3.4.2", "resolved": "https://registry.npmjs.org/consola/-/consola-3.4.2.tgz", @@ -12163,6 +12196,75 @@ "node": ">=8" } }, + "node_modules/finalhandler": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.1.2.tgz", + "integrity": "sha512-aAWcW57uxVNrQZqFXjITpW3sIUQmHGG3qSb9mUah9MgMC4NeWhNOlNjXEYq3HjRAvL6arUviZGGJsBg6z0zsWA==", + "dev": true, + "license": "MIT", + "dependencies": { + "debug": "2.6.9", + "encodeurl": "~1.0.2", + "escape-html": "~1.0.3", + "on-finished": "~2.3.0", + "parseurl": "~1.3.3", + "statuses": "~1.5.0", + "unpipe": "~1.0.0" + }, + "engines": { + "node": ">= 0.8" + } + }, + "node_modules/finalhandler/node_modules/debug": { + "version": "2.6.9", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", + "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", + "dev": true, + "license": "MIT", + "dependencies": { + "ms": "2.0.0" + } + }, + "node_modules/finalhandler/node_modules/encodeurl": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/encodeurl/-/encodeurl-1.0.2.tgz", + "integrity": "sha512-TPJXq8JqFaVYm2CWmPvnP2Iyo4ZSM7/QKcSmuMLDObfpH5fi7RUGmd/rTDf+rut/saiDiQEeVTNgAmJEdAOx0w==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 0.8" + } + }, + "node_modules/finalhandler/node_modules/ms": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", + "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==", + "dev": true, + "license": "MIT" + }, + "node_modules/finalhandler/node_modules/on-finished": { + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.3.0.tgz", + "integrity": "sha512-ikqdkGAAyf/X/gPhXGvfgAytDZtDbr+bkNUJ0N9h5MI/dmdgCs3l6hoHrcUv41sRKew3jIwrp4qQDXiK99Utww==", + "dev": true, + "license": "MIT", + "dependencies": { + "ee-first": "1.1.1" + }, + "engines": { + "node": ">= 0.8" + } + }, + "node_modules/finalhandler/node_modules/statuses": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/statuses/-/statuses-1.5.0.tgz", + "integrity": "sha512-OpZ3zP+jT1PI7I8nemJX4AKmAX070ZkYPVWV/AaKTJl+tXCTGyVdC1a4SL8RUQYEwk/f34ZX8UTykN68FwrqAA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 0.6" + } + }, "node_modules/find-cache-dir": { "version": "3.3.2", "resolved": "https://registry.npmjs.org/find-cache-dir/-/find-cache-dir-3.3.2.tgz", @@ -16563,6 +16665,16 @@ "dev": true, "license": "MIT" }, + "node_modules/utils-merge": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/utils-merge/-/utils-merge-1.0.1.tgz", + "integrity": "sha512-pMZTvIkT1d+TFGvDOqodOclx0QWkkgi6Tdoa8gC8ffGAAqz9pzPTZWAybbsHHoED/ztMtkv/VoYTYyShUn81hA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 0.4.0" + } + }, "node_modules/validate-npm-package-name": { "version": "7.0.2", "resolved": "https://registry.npmjs.org/validate-npm-package-name/-/validate-npm-package-name-7.0.2.tgz", @@ -16842,6 +16954,7 @@ "@eslint/js": "^9.39.2", "@istanbuljs/esm-loader-hook": "^0.3.0", "ava": "^6.4.1", + "connect": "^3.7.0", "eslint": "^9.39.5", "eslint-config-google": "^0.14.0", "eslint-plugin-ava": "^15.1.0", diff --git a/packages/middleware-code-coverage/ava-integration.config.js b/packages/middleware-code-coverage/ava-integration.config.js index 42d30a66..d2f1e700 100644 --- a/packages/middleware-code-coverage/ava-integration.config.js +++ b/packages/middleware-code-coverage/ava-integration.config.js @@ -1,5 +1,5 @@ export default { - files: ["test/integration/boot.js"], + files: ["test/integration/boot.js", "test/integration/connect.js"], watchMode: { ignoreChanges: [ "tmp/**" diff --git a/packages/middleware-code-coverage/lib/middleware.js b/packages/middleware-code-coverage/lib/middleware.js index a40fc924..131ed8c8 100644 --- a/packages/middleware-code-coverage/lib/middleware.js +++ b/packages/middleware-code-coverage/lib/middleware.js @@ -75,9 +75,12 @@ export default async function({log, middlewareUtil, options={}, resources}) { ); if (reportData) { - res.json(reportData); + const body = JSON.stringify(reportData); + res.writeHead(200, {"Content-Type": "application/json"}); + res.end(body); } else { - res.err("No report data provided"); + res.writeHead(400, {"Content-Type": "application/json"}); + res.end(JSON.stringify({error: "No report data provided"})); } } ); @@ -86,9 +89,9 @@ export default async function({log, middlewareUtil, options={}, resources}) { * Endpoint to check for middleware existence */ router.get("/.ui5/coverage/ping", async (req, res) => { - res.json({ - version: middlewareVersion - }); + const body = JSON.stringify({version: middlewareVersion}); + res.writeHead(200, {"Content-Type": "application/json"}); + res.end(body); }); /** @@ -123,9 +126,8 @@ export default async function({log, middlewareUtil, options={}, resources}) { return; } - log.verbose(`handling ${req.path}...`); - const pathname = middlewareUtil.getPathname(req); + log.verbose(`handling ${pathname}...`); const matchedResource = await resources.all.byPath(pathname); if (!matchedResource) { @@ -147,7 +149,7 @@ export default async function({log, middlewareUtil, options={}, resources}) { } // send out instrumented source + source map - res.type(".js"); + res.setHeader("Content-Type", "text/javascript"); res.end(instrumentedSource); }); diff --git a/packages/middleware-code-coverage/lib/util.js b/packages/middleware-code-coverage/lib/util.js index cc193be3..f041a63d 100644 --- a/packages/middleware-code-coverage/lib/util.js +++ b/packages/middleware-code-coverage/lib/util.js @@ -56,16 +56,18 @@ export function getLatestSourceMap(instrumenter) { * @returns {boolean} */ export function shouldInstrumentResource(request, excludePatterns) { + if (!request.url) { + return false; + } + const {pathname, searchParams} = new URL(request.url, "http://localhost"); return ( - request.path && - request.path.endsWith(".js") && // Only .js file requests - !isFalsyValue(request.query.instrument) && // instrument only flagged files, ignore "falsy" values + pathname.endsWith(".js") && + !isFalsyValue(searchParams.get("instrument")) && !(excludePatterns || []).some((pattern) => { if (pattern instanceof RegExp) { - // The ones coming from .library files are regular expressions - return pattern.test(request.path); + return pattern.test(pathname); } else { - return request.path.includes(pattern); + return pathname.includes(pattern); } }) ); diff --git a/packages/middleware-code-coverage/package.json b/packages/middleware-code-coverage/package.json index 149616fa..6b307a81 100644 --- a/packages/middleware-code-coverage/package.json +++ b/packages/middleware-code-coverage/package.json @@ -46,6 +46,7 @@ "@eslint/js": "^9.39.2", "@istanbuljs/esm-loader-hook": "^0.3.0", "ava": "^6.4.1", + "connect": "^3.7.0", "eslint": "^9.39.5", "eslint-config-google": "^0.14.0", "eslint-plugin-ava": "^15.1.0", diff --git a/packages/middleware-code-coverage/test/integration/connect.js b/packages/middleware-code-coverage/test/integration/connect.js new file mode 100644 index 00000000..5e1f6e78 --- /dev/null +++ b/packages/middleware-code-coverage/test/integration/connect.js @@ -0,0 +1,153 @@ +import test from "ava"; +import connect from "connect"; +import request from "supertest"; +import middleware from "../../lib/middleware.js"; + +const sampleJS = `sap.ui.define([ +"sap/ui/core/mvc/Controller", +"sap/m/MessageToast" +], (Controller, MessageToast) => Controller.extend("ui5.sample.controller.App", { + +onInit: () => { }, + +onButtonPress() { + MessageToast.show(this.getMessage()); +}, + +getMessage() { + return this.getView().getModel("i18n").getProperty("message"); +}, + +formatMessage(message) { + return message.toUpperCase(); +} +}));`; + +const resources = { + all: { + byGlob() { + return []; + }, + async byPath() { + return { + async getString() { + return sampleJS; + } + }; + } + } +}; + +const middlewareUtil = { + getPathname(req) { + return new URL(req.url, "http://localhost").pathname; + } +}; + +const log = { + verbose() {}, + warn() {}, + error() {} +}; + +async function createApp(options = {}) { + const mw = await middleware({log, middlewareUtil, options, resources}); + const app = connect(); + app.use(mw); + return request(app); +} + +test.beforeEach(async (t) => { + t.context.app = await createApp(); +}); + +const coverageMap = { + "/resources/Control1.js": { + path: "/resources/Control1.js", + statementMap: {}, + fnMap: {}, + branchMap: {}, + s: {}, + f: {}, + b: {} + } +}; + +// Case 1: Ping endpoint — exercises the JSON response path of the ping handler +test("Ping endpoint returns 200 JSON with version", async (t) => { + const res = await t.context.app + .get("/.ui5/coverage/ping") + .expect(200); + + t.is(res.headers["content-type"].split(";")[0], "application/json"); + t.truthy(res.body.version); +}); + +// Case 2: Send coverage report — exercises body-parser + the JSON response path +test("POST report with coverage map returns coverageMap and availableReports", async (t) => { + const res = await t.context.app + .post("/.ui5/coverage/report") + .set("Content-Type", "application/json") + .send(coverageMap) + .expect(200); + + t.is(res.headers["content-type"].split(";")[0], "application/json"); + t.true(Array.isArray(res.body.coverageMap)); + t.true(Array.isArray(res.body.availableReports)); + t.true(res.body.availableReports.some((report) => report.report === "html")); +}); + +// Case 3: Empty body — reportCoverage always returns a (possibly empty) report, +// so the request succeeds with an empty coverage map. The "no report data" 400 +// branch is only reachable when reportCoverage returns falsy, which body-parser +// (always providing at least `{}`) prevents at the HTTP level; that branch is +// covered by the unit tests instead. +test("POST report with empty body returns 200 with an empty coverage map", async (t) => { + const res = await t.context.app + .post("/.ui5/coverage/report") + .set("Content-Type", "application/json") + .send({}) + .expect(200); + + t.true(Array.isArray(res.body.coverageMap)); + t.is(res.body.coverageMap.length, 0); +}); + +// Case 4: Generated report is served via serve-static (framework-agnostic) +test("Generated report is served after posting coverage data", async (t) => { + const reportApp = await createApp(); + await reportApp + .post("/.ui5/coverage/report") + .set("Content-Type", "application/json") + .send(coverageMap) + .expect(200); + + const res = await reportApp + .get("/.ui5/coverage/report/html/index.html") + .expect(200); + + t.true(res.text.includes("Code coverage report")); +}); + +// Case 5: Instrument a .js resource — exercises req.url parsing in +// shouldInstrumentResource and the Content-Type header set on the response +test("GET with ?instrument=true returns instrumented JS with sourceMappingURL", async (t) => { + const res = await t.context.app + .get("/resources/lib1/Control1.js?instrument=true") + .expect(200); + + const contentType = res.headers["content-type"]; + t.is(contentType, "text/javascript"); + t.true(res.text.includes("path=\"/resources/lib1/Control1.js\"")); + t.true(res.text.includes("sourceMappingURL=data:application/json")); +}); + +// Case 6: Non-instrumented resource falls through to connect's default 404, +// proving the middleware calls next() instead of swallowing unrelated requests +test("Non-instrumented resource falls through to 404", async (t) => { + await t.context.app + .get("/resources/lib1/Control1.js") + .expect(404); + + t.pass(); +}); diff --git a/packages/middleware-code-coverage/test/unit/lib/middleware.js b/packages/middleware-code-coverage/test/unit/lib/middleware.js index 4e01fea1..1d297160 100644 --- a/packages/middleware-code-coverage/test/unit/lib/middleware.js +++ b/packages/middleware-code-coverage/test/unit/lib/middleware.js @@ -61,17 +61,23 @@ test("Ping request", async (t) => { const {instrumenterMiddleware, readJsonFile} = t.context; const middleware = await instrumenterMiddleware({resources}); - t.plan(6); + t.plan(7); t.is(readJsonFile.callCount, 1, "package.json should be read once during middleware initialization"); t.deepEqual(readJsonFile.getCall(0).args, [new URL("../../../package.json", import.meta.url)]); await new Promise((resolve) => { + let statusCode; const res = { - json: function(body) { - t.is(Object.keys(body).length, 1); - t.is(Object.keys(body)[0], "version"); - t.is(body.version, "0.0.0-test", "The version is returned"); + writeHead(code) { + statusCode = code; + }, + end(body) { + const parsed = JSON.parse(body); + t.is(statusCode, 200); + t.is(Object.keys(parsed).length, 1); + t.is(Object.keys(parsed)[0], "version"); + t.is(parsed.version, "0.0.0-test", "The version is returned"); t.is(readJsonFile.callCount, 1, "package.json should not be read again per request"); resolve(); } @@ -96,22 +102,24 @@ test("Coverage report request", async (t) => { }); const middleware = await instrumenterMiddleware({log, resources}); - t.plan(7); + t.plan(8); await new Promise((resolve) => { + let statusCode; const res = { - json(body) { + writeHead(code) { + statusCode = code; + }, + end(body) { + const parsed = JSON.parse(body); + t.is(statusCode, 200); t.is(reportCoverageStub.callCount, 1); t.is(reportCoverageStub.getCall(0).args.length, 4); t.is(reportCoverageStub.getCall(0).args[0], coverageData); t.is(reportCoverageStub.getCall(0).args[1].cwd, "./"); t.is(reportCoverageStub.getCall(0).args[2], resources); t.is(reportCoverageStub.getCall(0).args[3], log); - t.is(body, expectedCoverageReport); - resolve(); - }, - err() { - t.fail("should not be called."); + t.deepEqual(parsed, expectedCoverageReport); resolve(); } }; @@ -144,14 +152,14 @@ test("Coverage report request: no report data", async (t) => { t.plan(2); await new Promise((resolve) => { + let statusCode; const res = { - json() { - t.fail("should not be called."); - resolve(); + writeHead(code) { + statusCode = code; }, - err(message) { + end(body) { + t.is(statusCode, 400); t.is(reportCoverageStub.callCount, 1); - t.is(message, "No report data provided"); resolve(); } }; @@ -181,14 +189,14 @@ test("Coverage report request: no body", async (t) => { t.plan(2); await new Promise((resolve) => { + let statusCode; const res = { - json() { - t.fail("should not be called."); - resolve(); + writeHead(code) { + statusCode = code; }, - err(message) { + end(body) { + t.is(statusCode, 400); t.is(reportCoverageStub.callCount, 1); - t.is(message, "No report data provided"); resolve(); } }; @@ -235,7 +243,7 @@ test("Instrument resources request with source map", async (t) => { const {instrumenterMiddleware} = t.context; const middleware = await instrumenterMiddleware({log, middlewareUtil, resources}); - t.plan(4); + t.plan(5); await new Promise((resolve) => { const res = { @@ -248,8 +256,9 @@ test("Instrument resources request with source map", async (t) => { t.is(log.verbose.callCount, 3); resolve(); }, - type(type) { - t.is(type, ".js"); + setHeader(name, value) { + t.is(name, "Content-Type"); + t.is(value, "text/javascript"); } }; const next = () => { @@ -258,11 +267,7 @@ test("Instrument resources request with source map", async (t) => { }; middleware({ method: "GET", - url: "/resources/lib1/Control1.js", - path: "/resources/lib1/Control1.js", - query: { - instrument: "true" - } + url: "/resources/lib1/Control1.js?instrument=true" }, res, next); }); }); @@ -275,7 +280,7 @@ test("Instrument resources request with source map: manual enablement", async (t const options = {configuration: {instrument: {produceSourceMap: true}}}; const middleware = await instrumenterMiddleware({log, middlewareUtil, options, resources}); - t.plan(4); + t.plan(5); await new Promise((resolve) => { const res = { @@ -288,8 +293,9 @@ test("Instrument resources request with source map: manual enablement", async (t t.is(log.verbose.callCount, 3); resolve(); }, - type(type) { - t.is(type, ".js"); + setHeader(name, value) { + t.is(name, "Content-Type"); + t.is(value, "text/javascript"); } }; const next = () => { @@ -298,11 +304,7 @@ test("Instrument resources request with source map: manual enablement", async (t }; middleware({ method: "GET", - url: "/resources/lib1/Control1.js", - path: "/resources/lib1/Control1.js", - query: { - instrument: "true" - } + url: "/resources/lib1/Control1.js?instrument=true" }, res, next); }); }); @@ -316,7 +318,7 @@ test("Instrument resources request without source map", async (t) => { const options = {configuration: {instrument: {produceSourceMap: false}}}; const middleware = await instrumenterMiddleware({log, middlewareUtil, options, resources}); - t.plan(4); + t.plan(5); await new Promise((resolve) => { const res = { @@ -329,8 +331,9 @@ test("Instrument resources request without source map", async (t) => { t.is(log.verbose.callCount, 2); resolve(); }, - type(type) { - t.is(type, ".js"); + setHeader(name, value) { + t.is(name, "Content-Type"); + t.is(value, "text/javascript"); } }; const next = () => { @@ -339,11 +342,7 @@ test("Instrument resources request without source map", async (t) => { }; middleware({ method: "GET", - url: "/resources/lib1/Control1.js", - path: "/resources/lib1/Control1.js", - query: { - instrument: "true" - } + url: "/resources/lib1/Control1.js?instrument=true" }, res, next); }); }); @@ -366,7 +365,7 @@ test("Instrument resources request for non instrumented resource", async (t) => t.fail("should not be called."); resolve(); }, - type() { + setHeader() { t.fail("should not be called."); resolve(); } @@ -382,11 +381,7 @@ test("Instrument resources request for non instrumented resource", async (t) => }; middleware({ method: "GET", - url: "/resources/lib1/Control1.js", - path: "/resources/lib1/Control1.js", - query: { - instrument: "true" - } + url: "/resources/lib1/Control1.js?instrument=true" }, res, next); }); }); @@ -417,7 +412,7 @@ test("Instrument resources request with no matching resources", async (t) => { t.fail("should not be called."); resolve(); }, - type() { + setHeader() { t.fail("should not be called."); resolve(); } @@ -434,11 +429,7 @@ test("Instrument resources request with no matching resources", async (t) => { }; middleware({ method: "GET", - url: "/resources/lib1/Control1.js", - path: "/resources/lib1/Control1.js", - query: { - instrument: "true" - } + url: "/resources/lib1/Control1.js?instrument=true" }, res, next); }); }); @@ -479,7 +470,7 @@ test("Instrument resources request with custom excludePatterns from configuratio t.fail("should not be called because resource is excluded."); resolve(); }, - type() { + setHeader() { t.fail("should not be called because resource is excluded."); resolve(); } @@ -495,11 +486,7 @@ test("Instrument resources request with custom excludePatterns from configuratio }; middleware({ method: "GET", - url: "/resources/lib1/Control1.js", - path: "/resources/lib1/Control1.js", - query: { - instrument: "true" - } + url: "/resources/lib1/Control1.js?instrument=true" }, res, next); }); }); @@ -554,7 +541,7 @@ test("Instrument resources request with custom excludePatterns overrides .librar t.fail("should not be called because resource is excluded by custom pattern."); resolve(); }, - type() { + setHeader() { t.fail("should not be called because resource is excluded by custom pattern."); resolve(); } @@ -570,11 +557,7 @@ test("Instrument resources request with custom excludePatterns overrides .librar }; middleware({ method: "GET", - url: "/resources/lib1/Control1.js", - path: "/resources/lib1/Control1.js", - query: { - instrument: "true" - } + url: "/resources/lib1/Control1.js?instrument=true" }, res, next); }); }); @@ -617,7 +600,7 @@ test("Instrument multiple JS files in sequence", async (t) => { const customMiddlewareUtil = { getPathname(req) { - return req.path; + return new URL(req.url, "http://localhost").pathname; } }; @@ -628,7 +611,7 @@ test("Instrument multiple JS files in sequence", async (t) => { resources: customResources }); - t.plan(7); + t.plan(9); // First request for Control1.js await new Promise((resolve) => { @@ -641,8 +624,9 @@ test("Instrument multiple JS files in sequence", async (t) => { ), "First instrumented resource contains source map"); resolve(); }, - type(type) { - t.is(type, ".js"); + setHeader(name, value) { + t.is(name, "Content-Type"); + t.is(value, "text/javascript"); } }; const next = () => { @@ -651,11 +635,7 @@ test("Instrument multiple JS files in sequence", async (t) => { }; middleware({ method: "GET", - url: "/resources/lib1/Control1.js", - path: "/resources/lib1/Control1.js", - query: { - instrument: "true" - } + url: "/resources/lib1/Control1.js?instrument=true" }, res, next); }); @@ -670,8 +650,9 @@ test("Instrument multiple JS files in sequence", async (t) => { ), "Second instrumented resource contains source map"); resolve(); }, - type(type) { - t.is(type, ".js"); + setHeader(name, value) { + t.is(name, "Content-Type"); + t.is(value, "text/javascript"); } }; const next = () => { @@ -680,11 +661,7 @@ test("Instrument multiple JS files in sequence", async (t) => { }; middleware({ method: "GET", - url: "/resources/lib2/Control2.js", - path: "/resources/lib2/Control2.js", - query: { - instrument: "true" - } + url: "/resources/lib2/Control2.js?instrument=true" }, res, next); }); @@ -704,7 +681,7 @@ test("Instrument resources request with excludePatterns set to null", async (t) }; const middleware = await instrumenterMiddleware({log, middlewareUtil, options, resources}); - t.plan(4); + t.plan(5); await new Promise((resolve) => { const res = { @@ -717,8 +694,9 @@ test("Instrument resources request with excludePatterns set to null", async (t) t.is(log.verbose.callCount, 3, "verbose should be called normally"); resolve(); }, - type(type) { - t.is(type, ".js"); + setHeader(name, value) { + t.is(name, "Content-Type"); + t.is(value, "text/javascript"); } }; const next = () => { @@ -727,11 +705,7 @@ test("Instrument resources request with excludePatterns set to null", async (t) }; middleware({ method: "GET", - url: "/resources/lib1/Control1.js", - path: "/resources/lib1/Control1.js", - query: { - instrument: "true" - } + url: "/resources/lib1/Control1.js?instrument=true" }, res, next); }); }); diff --git a/packages/middleware-code-coverage/test/unit/lib/util.js b/packages/middleware-code-coverage/test/unit/lib/util.js index 088eff59..72d0360d 100644 --- a/packages/middleware-code-coverage/test/unit/lib/util.js +++ b/packages/middleware-code-coverage/test/unit/lib/util.js @@ -13,10 +13,15 @@ import { const SOURCE_MAPPING_URL = "//" + "# sourceMappingURL"; function getMockedRequest(path="", query={}) { - return { - path, - query - }; + const url = new URL(path, "http://localhost"); + for (const [key, value] of Object.entries(query)) { + if (typeof value === "string") { + url.searchParams.set(key, value); + } + // JS-falsy non-string values (false, 0, null, undefined) are omitted; + // searchParams.get() returns null for absent keys, which isFalsyValue covers. + } + return {url: url.pathname + url.search}; } test("createInstrumentationConfig: default config", async (t) => { @@ -289,6 +294,10 @@ test("shouldInstrumentResource: No JS file", (t) => { t.false(toBeInstrumented); }); +test("shouldInstrumentResource: Request without URL", (t) => { + t.false(shouldInstrumentResource({})); +}); + test("shouldInstrumentResource: Non flagged resources", (t) => { const toBeInstrumented = shouldInstrumentResource(getMockedRequest("Test.js")); t.false(toBeInstrumented);