diff --git a/packages/middleware-code-coverage/lib/middleware.js b/packages/middleware-code-coverage/lib/middleware.js index 08e5fca0..ae5607df 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"; @@ -33,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, @@ -104,9 +103,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); });