From 9210f03d6745ff5bbee2327e747d65e894ffbb09 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 28 Jan 2026 10:31:51 +0100 Subject: [PATCH 1/2] fix(middleware-code-coverage): Lazy evaluate exclude patterns This prevents the initial evaluation of exclude patterns on middleware initialization, which is not necessary in case the middleware is not used. Calculating exclude patterns requires to read and parse all .library files of the project and its dependencies. --- .../lib/middleware.js | 19 +- packages/middleware-code-coverage/lib/util.js | 21 +- .../test/unit/lib/middleware.js | 309 +++++++++++++++++- .../test/unit/lib/util.js | 160 ++++----- 4 files changed, 402 insertions(+), 107 deletions(-) diff --git a/packages/middleware-code-coverage/lib/middleware.js b/packages/middleware-code-coverage/lib/middleware.js index 08e5fca0..6d7bb196 100644 --- a/packages/middleware-code-coverage/lib/middleware.js +++ b/packages/middleware-code-coverage/lib/middleware.js @@ -3,7 +3,9 @@ import { createInstrumentationConfig, shouldInstrumentResource, getLatestSourceMap, - readJsonFile} from "./util.js"; + readJsonFile, + getLibraryCoverageExcludePatterns +} from "./util.js"; import {createInstrumenter} from "istanbul-lib-instrument"; import reportCoverage from "./coverage-reporter.js"; import bodyParser from "body-parser"; @@ -104,9 +106,22 @@ export default async function({log, middlewareUtil, options={}, resources}) { ) ); + let excludePatterns; + router.use(async (req, res, next) => { + // Lazy initialize exclude patterns + if (excludePatterns === undefined) { + // Custom patterns take precedence over .library defined patterns (also when set to null) + if (generalConfig.excludePatterns !== undefined) { + excludePatterns = generalConfig.excludePatterns; + } else { + // Read patterns from .library files, this should only be done if needed and only once + excludePatterns = await getLibraryCoverageExcludePatterns(resources.all); + } + } + // Skip files which should not be instrumented - if (!shouldInstrumentResource(req, generalConfig)) { + if (!shouldInstrumentResource(req, excludePatterns)) { next(); return; } diff --git a/packages/middleware-code-coverage/lib/util.js b/packages/middleware-code-coverage/lib/util.js index 7edb85ee..cc193be3 100644 --- a/packages/middleware-code-coverage/lib/util.js +++ b/packages/middleware-code-coverage/lib/util.js @@ -7,18 +7,15 @@ import {readFile} from "node:fs/promises"; * * @public * @param {object} configuration instrumentation configuration - * @param {@ui5/fs/Resource[]} resources * @returns {Promise} configuration */ -export async function createInstrumentationConfig(configuration = {}, resources) { - const excludedPatterns = resources ? await excludePatterns(resources) : []; - +export async function createInstrumentationConfig(configuration = {}) { const {instrument, report, ...generalConfig} = configuration; return { // General configuration cwd: "./", - excludePatterns: excludedPatterns, + // General config overwrites ...generalConfig, @@ -55,17 +52,17 @@ export function getLatestSourceMap(instrumenter) { * * @public * @param {object} request - * @param {object} config + * @param {Array} excludePatterns Patterns to exclude file from instrumentation (RegExp or string) * @returns {boolean} */ -export function shouldInstrumentResource(request, config) { +export function shouldInstrumentResource(request, excludePatterns) { return ( request.path && request.path.endsWith(".js") && // Only .js file requests !isFalsyValue(request.query.instrument) && // instrument only flagged files, ignore "falsy" values - !(config && config.excludePatterns || []).some((pattern) => { + !(excludePatterns || []).some((pattern) => { if (pattern instanceof RegExp) { - // The ones comming from .library files are regular expressions + // The ones coming from .library files are regular expressions return pattern.test(request.path); } else { return request.path.includes(pattern); @@ -131,13 +128,13 @@ function isFalsyValue(value) { * Note: We might consider to move this utility into the @ui5/project * * @private - * @param {@ui5/fs/Resource[]} resources + * @param {@ui5/fs/AbstractReader} reader * @returns {Promise} exclude patterns */ -async function excludePatterns(resources) { +export async function getLibraryCoverageExcludePatterns(reader) { const aExcludes = []; // Read excludes from .library files - const aDotLibrary = await resources.byGlob(["/resources/**/.library"]); + const aDotLibrary = await reader.byGlob(["/resources/**/.library"]); for (const oDotLibrary of aDotLibrary) { const content = await oDotLibrary.getString(); const result = await xml2js.parseStringPromise(content); diff --git a/packages/middleware-code-coverage/test/unit/lib/middleware.js b/packages/middleware-code-coverage/test/unit/lib/middleware.js index def69a21..de9fa586 100644 --- a/packages/middleware-code-coverage/test/unit/lib/middleware.js +++ b/packages/middleware-code-coverage/test/unit/lib/middleware.js @@ -335,8 +335,12 @@ test("Instrument resources request for non instrumented resource", async (t) => resolve(); } }; - const next = () => { - t.pass("Should be called."); + const next = (error) => { + if (error) { + t.fail(error); + } else { + t.pass("Should be called without error."); + } t.is(shouldInstrumentResourceStub.callCount, 1); resolve(); }; @@ -382,8 +386,12 @@ test("Instrument resources request with no matching resources", async (t) => { resolve(); } }; - const next = () => { - t.pass("Should be called."); + const next = (error) => { + if (error) { + t.fail(error); + } else { + t.pass("Should be called without error."); + } t.is(log.verbose.callCount, 1); t.is(log.warn.callCount, 1); resolve(); @@ -398,3 +406,296 @@ test("Instrument resources request with no matching resources", async (t) => { }, res, next); }); }); + +test("Instrument resources request with custom excludePatterns from configuration", async (t) => { + const log = { + verbose: sinon.stub() + }; + const customResources = { + all: { + byGlob() { + return []; // No .library files + }, + async byPath() { + return { + async getString() { + return sampleJS; + } + }; + } + } + }; + const options = { + configuration: { + excludePatterns: [ + "/resources/lib1/Control1.js" + ] + } + }; + const {instrumenterMiddleware} = t.context; + const middleware = await instrumenterMiddleware({log, middlewareUtil, options, resources: customResources}); + + t.plan(2); + + await new Promise((resolve) => { + const res = { + end() { + t.fail("should not be called because resource is excluded."); + resolve(); + }, + type() { + t.fail("should not be called because resource is excluded."); + resolve(); + } + }; + const next = (error) => { + if (error) { + t.fail(String(error)); + } else { + t.pass("Should be called without error because resource is excluded via custom excludePatterns."); + } + t.is(log.verbose.callCount, 0, "verbose should not be called for excluded resources"); + resolve(); + }; + middleware({ + method: "GET", + url: "/resources/lib1/Control1.js", + path: "/resources/lib1/Control1.js", + query: { + instrument: "true" + } + }, res, next); + }); +}); + +test("Instrument resources request with custom excludePatterns overrides .library excludes", async (t) => { + const log = { + verbose: sinon.stub() + }; + const sDotLibrary = ` + + ui5.lib1 + + + + + +`; + + const customResources = { + all: { + byGlob() { + return [{ + getString() { + return sDotLibrary; + } + }]; + }, + async byPath() { + return { + async getString() { + return sampleJS; + } + }; + } + } + }; + const options = { + configuration: { + excludePatterns: [ + "/resources/lib1/Control1.js" + ] + } + }; + const {instrumenterMiddleware} = t.context; + const middleware = await instrumenterMiddleware({log, middlewareUtil, options, resources: customResources}); + + t.plan(2); + + await new Promise((resolve) => { + const res = { + end() { + t.fail("should not be called because resource is excluded by custom pattern."); + resolve(); + }, + type() { + t.fail("should not be called because resource is excluded by custom pattern."); + resolve(); + } + }; + const next = (error) => { + if (error) { + t.fail(error); + } else { + t.pass("Should be called without error - custom excludePatterns override .library excludes."); + } + t.is(log.verbose.callCount, 0, "verbose should not be called for excluded resources"); + resolve(); + }; + middleware({ + method: "GET", + url: "/resources/lib1/Control1.js", + path: "/resources/lib1/Control1.js", + query: { + instrument: "true" + } + }, res, next); + }); +}); + +test("Instrument multiple JS files in sequence", async (t) => { + const log = { + verbose: sinon.stub() + }; + const sampleJS2 = `sap.ui.define(["sap/ui/core/Control"], (Control) => Control.extend("ui5.sample.Control2", { + renderer: { + render(oRm, oControl) { + oRm.write("
Control2
"); + } + } + }));`; + + const customResources = { + all: { + byGlob() { + return []; + }, + async byPath(path) { + if (path === "/resources/lib1/Control1.js") { + return { + async getString() { + return sampleJS; + } + }; + } else if (path === "/resources/lib2/Control2.js") { + return { + async getString() { + return sampleJS2; + } + }; + } + return undefined; + } + } + }; + + const customMiddlewareUtil = { + getPathname(req) { + return req.path; + } + }; + + const {instrumenterMiddleware} = t.context; + const middleware = await instrumenterMiddleware({ + log, + middlewareUtil: customMiddlewareUtil, + resources: customResources + }); + + t.plan(7); + + // First request for Control1.js + await new Promise((resolve) => { + const res = { + end(resource) { + t.true(resource.includes("path=\"/resources/lib1/Control1.js\""), + "First instrumented resource is correct"); + t.true(resource.includes( + `${SOURCE_MAPPING_URL}=data:application/json;charset=utf-8;base64,` + ), "First instrumented resource contains source map"); + resolve(); + }, + type(type) { + t.is(type, ".js"); + } + }; + const next = () => { + t.fail("should not be called."); + resolve(); + }; + middleware({ + method: "GET", + url: "/resources/lib1/Control1.js", + path: "/resources/lib1/Control1.js", + query: { + instrument: "true" + } + }, res, next); + }); + + // Second request for Control2.js + await new Promise((resolve) => { + const res = { + end(resource) { + t.true(resource.includes("path=\"/resources/lib2/Control2.js\""), + "Second instrumented resource is correct"); + t.true(resource.includes( + `${SOURCE_MAPPING_URL}=data:application/json;charset=utf-8;base64,` + ), "Second instrumented resource contains source map"); + resolve(); + }, + type(type) { + t.is(type, ".js"); + } + }; + const next = () => { + t.fail("should not be called."); + resolve(); + }; + middleware({ + method: "GET", + url: "/resources/lib2/Control2.js", + path: "/resources/lib2/Control2.js", + query: { + instrument: "true" + } + }, res, next); + }); + + // Verify verbose was called for both requests + t.is(log.verbose.callCount, 6, "verbose should be called 3 times per instrumented resource"); +}); + +test("Instrument resources request with excludePatterns set to null", async (t) => { + const log = { + verbose: sinon.stub() + }; + const {instrumenterMiddleware} = t.context; + const options = { + configuration: { + excludePatterns: null + } + }; + const middleware = await instrumenterMiddleware({log, middlewareUtil, options, resources}); + + t.plan(4); + + await new Promise((resolve) => { + const res = { + end(resource) { + t.true(resource.includes("path=\"/resources/lib1/Control1.js\""), + "Instrumented resource is correct"); + t.true(resource.includes( + `${SOURCE_MAPPING_URL}=data:application/json;charset=utf-8;base64,` + ), "Instrumented resource contains source map"); + t.is(log.verbose.callCount, 3, "verbose should be called normally"); + resolve(); + }, + type(type) { + t.is(type, ".js"); + } + }; + const next = () => { + t.fail("should not be called."); + resolve(); + }; + middleware({ + method: "GET", + url: "/resources/lib1/Control1.js", + path: "/resources/lib1/Control1.js", + query: { + 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 667e9f76..088eff59 100644 --- a/packages/middleware-code-coverage/test/unit/lib/util.js +++ b/packages/middleware-code-coverage/test/unit/lib/util.js @@ -2,6 +2,7 @@ import test from "ava"; import { createInstrumentationConfig, getLatestSourceMap, + getLibraryCoverageExcludePatterns, readJsonFile, shouldInstrumentResource } from "../../../lib/util.js"; @@ -21,7 +22,6 @@ function getMockedRequest(path="", query={}) { test("createInstrumentationConfig: default config", async (t) => { const expectedConfig = { cwd: "./", - excludePatterns: [], instrument: { coverageGlobalScope: "window.top", coverageGlobalScopeFunc: false, @@ -59,7 +59,6 @@ test("createInstrumentationConfig: default config", async (t) => { test("createInstrumentationConfig: custom config", async (t) => { const expectedConfig = { cwd: "./myworkingdirectory", - excludePatterns: [], instrument: { coverageGlobalScope: "this", coverageGlobalScopeFunc: true, @@ -125,14 +124,12 @@ test("createInstrumentationConfig: custom config", async (t) => { t.deepEqual(config, expectedConfig); }); -test("createInstrumentationConfig: .library excludes", async (t) => { - const expectedConfig = { - excludePatterns: [ - /\/resources\/((([^/]+[/])*my-file))(-dbg)?.js$/, - /\/resources\/((ui5\/customlib\/utils\/([^/]+[/])*[^/]*))(-dbg)?.js$/, - /\/resources\/((ui5\/customlib\/Control1))(-dbg)?.js$/, - ], - }; +test("getLibraryCoverageExcludePatterns: .library excludes", async (t) => { + const expectedPatterns = [ + /\/resources\/((([^/]+[/])*my-file))(-dbg)?.js$/, + /\/resources\/((ui5\/customlib\/utils\/([^/]+[/])*[^/]*))(-dbg)?.js$/, + /\/resources\/((ui5\/customlib\/Control1))(-dbg)?.js$/, + ]; const sDotLibrary = ` @@ -147,7 +144,7 @@ test("createInstrumentationConfig: .library excludes", async (t) => { `; - const resources = { + const reader = { byGlob() { return [{ getString() { @@ -156,15 +153,11 @@ test("createInstrumentationConfig: .library excludes", async (t) => { }]; } }; - const config = await createInstrumentationConfig({}, resources); - t.deepEqual(config.excludePatterns, expectedConfig.excludePatterns); + const patterns = await getLibraryCoverageExcludePatterns(reader); + t.deepEqual(patterns, expectedPatterns); }); -test("createInstrumentationConfig: .library but without jscoverage", async (t) => { - const expectedConfig = { - excludePatterns: [], - }; - +test("getLibraryCoverageExcludePatterns: .library without jscoverage", async (t) => { const sDotLibrary = ` ui5.customlib @@ -172,7 +165,7 @@ test("createInstrumentationConfig: .library but without jscoverage", async (t) = `; - const resources = { + const reader = { byGlob() { return [{ getString() { @@ -181,15 +174,11 @@ test("createInstrumentationConfig: .library but without jscoverage", async (t) = }]; } }; - const config = await createInstrumentationConfig({}, resources); - t.deepEqual(config.excludePatterns, expectedConfig.excludePatterns); + const patterns = await getLibraryCoverageExcludePatterns(reader); + t.deepEqual(patterns, []); }); -test("createInstrumentationConfig: .library but without excludes", async (t) => { - const expectedConfig = { - excludePatterns: [], - }; - +test("getLibraryCoverageExcludePatterns: .library without excludes", async (t) => { const sDotLibrary = ` ui5.customlib @@ -199,7 +188,7 @@ test("createInstrumentationConfig: .library but without excludes", async (t) => `; - const resources = { + const reader = { byGlob() { return [{ getString() { @@ -208,60 +197,61 @@ test("createInstrumentationConfig: .library but without excludes", async (t) => }]; } }; - const config = await createInstrumentationConfig({}, resources); - t.deepEqual(config.excludePatterns, expectedConfig.excludePatterns); + const patterns = await getLibraryCoverageExcludePatterns(reader); + t.deepEqual(patterns, []); }); -test("createInstrumentationConfig: custom excludes", async (t) => { - const excludes = [ - /\/resources\/((ui5\/customlib\/src\/([^/]+[/])*[^/]*))(-dbg)?.js$/, - /\/resources\/((ui5\/customlib\/test))?.js$/, - ]; - - const expectedConfig = { - excludePatterns: excludes +test("getLibraryCoverageExcludePatterns: no .library files", async (t) => { + const reader = { + byGlob() { + return []; + } }; - - const config = await createInstrumentationConfig({ - excludePatterns: excludes - }); - t.deepEqual(config.excludePatterns, expectedConfig.excludePatterns); + const patterns = await getLibraryCoverageExcludePatterns(reader); + t.deepEqual(patterns, []); }); -test("createInstrumentationConfig: custom excludes override .library excludes", async (t) => { - const excludes = [ - /\/resources\/((ui5\/customlib\/src\/([^/]+[/])*[^/]*))(-dbg)?.js$/, - /\/resources\/((ui5\/customlib\/test))?.js$/, - ]; - - const expectedConfig = { - excludePatterns: excludes - }; +test("getLibraryCoverageExcludePatterns: multiple .library files", async (t) => { + const sDotLibrary1 = ` + + ui5.lib1 + + + + + +`; - const sDotLibrary = ` + const sDotLibrary2 = ` - ui5.customlib + ui5.lib2 - + `; - const resources = { + const reader = { byGlob() { - return [{ - getString() { - return sDotLibrary; + return [ + { + getString() { + return sDotLibrary1; + } + }, + { + getString() { + return sDotLibrary2; + } } - }]; + ]; } }; - - const config = await createInstrumentationConfig({ - excludePatterns: excludes - }, resources); - t.deepEqual(config.excludePatterns, expectedConfig.excludePatterns); + const patterns = await getLibraryCoverageExcludePatterns(reader); + t.is(patterns.length, 2); + t.true(patterns[0].test("/resources/ui5/lib1/Control1.js")); + t.true(patterns[1].test("/resources/ui5/lib2/Control2.js")); }); test("readJsonFile", async (t) => { @@ -317,51 +307,43 @@ test("shouldInstrumentResource: Flag resource as non instrumented", (t) => { }); test("shouldInstrumentResource: Resource flagged as instrumented, no excludes", (t) => { - const toBeInstrumented = shouldInstrumentResource(getMockedRequest("Test.js", {instrument: "true"})); + const toBeInstrumented = shouldInstrumentResource(getMockedRequest("Test.js", {instrument: "true"}), []); t.true(toBeInstrumented); }); test("shouldInstrumentResource: Resource flagged as instrumented, but defined matching regex exclude", (t) => { const request = getMockedRequest("/resources/ui5/customlib/test/MyTest.js", {instrument: "true"}); - const config = { - excludePatterns: [ - /\/resources\/((ui5\/customlib\/test\/([^/]+[/])*[^/]*))(-dbg)?.js$/ - ] - }; - const toBeInstrumented = shouldInstrumentResource(request, config); + const excludePatterns = [ + /\/resources\/((ui5\/customlib\/test\/([^/]+[/])*[^/]*))(-dbg)?.js$/ + ]; + const toBeInstrumented = shouldInstrumentResource(request, excludePatterns); t.false(toBeInstrumented); }); test("shouldInstrumentResource: Resource flagged as instrumented, but defined matching pattern exclude", (t) => { const request = getMockedRequest("/resources/ui5/customlib/test/MyTest.js", {instrument: "true"}); - const config = { - excludePatterns: [ - "/resources/ui5/customlib/test/MyTest.js" - ] - }; - const toBeInstrumented = shouldInstrumentResource(request, config); + const excludePatterns = [ + new RegExp("/resources/ui5/customlib/test/MyTest\\.js") + ]; + const toBeInstrumented = shouldInstrumentResource(request, excludePatterns); t.false(toBeInstrumented); }); test("shouldInstrumentResource: Resource flagged as instrumented, with no matching regex exclude", (t) => { const request = getMockedRequest("/resources/ui5/customlib/src/Control1.js", {instrument: "true"}); - const config = { - excludePatterns: [ - /\/resources\/((ui5\/customlib\/test\/([^/]+[/])*[^/]*))(-dbg)?.js$/ - ] - }; - const toBeInstrumented = shouldInstrumentResource(request, config); + const excludePatterns = [ + /\/resources\/((ui5\/customlib\/test\/([^/]+[/])*[^/]*))(-dbg)?.js$/ + ]; + const toBeInstrumented = shouldInstrumentResource(request, excludePatterns); t.true(toBeInstrumented); }); test("shouldInstrumentResource: Resource flagged as instrumented, with no matching pattern exclude", (t) => { const request = getMockedRequest("/resources/ui5/customlib/src/Control.js", {instrument: "true"}); - const config = { - excludePatterns: [ - "/resources/ui5/customlib/test/MyTest.js" - ] - }; - const toBeInstrumented = shouldInstrumentResource(request, config); + const excludePatterns = [ + new RegExp("/resources/ui5/customlib/test/MyTest\\.js") + ]; + const toBeInstrumented = shouldInstrumentResource(request, excludePatterns); t.true(toBeInstrumented); }); From 1251dcaf51ccf5d0ed85d23b37e0b16701812777 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 28 Jan 2026 13:13:25 +0100 Subject: [PATCH 2/2] refactor: Remove obsolete argument --- packages/middleware-code-coverage/lib/middleware.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/middleware-code-coverage/lib/middleware.js b/packages/middleware-code-coverage/lib/middleware.js index 6d7bb196..ae5607df 100644 --- a/packages/middleware-code-coverage/lib/middleware.js +++ b/packages/middleware-code-coverage/lib/middleware.js @@ -35,10 +35,7 @@ import {promisify} from "node:util"; * @returns {Function} Middleware function to use */ export default async function({log, middlewareUtil, options={}, resources}) { - const config = await createInstrumentationConfig( - options.configuration, - resources.all - ); + const config = await createInstrumentationConfig(options.configuration); const { report: reporterConfig, instrument: instrumenterConfig,