diff --git a/src/browser/cdp/selectivity/css-selectivity.ts b/src/browser/cdp/selectivity/css-selectivity.ts index d85abdadb..f18786e55 100644 --- a/src/browser/cdp/selectivity/css-selectivity.ts +++ b/src/browser/cdp/selectivity/css-selectivity.ts @@ -213,11 +213,9 @@ export class CSSSelectivity { }; } - return setCachedSelectivityFile( - CacheType.CssSessionCache, - this._wdSessionId, - JSON.stringify(sessionCache), - ).catch(err => { + return setCachedSelectivityFile(CacheType.CssSessionCache, this._wdSessionId, JSON.stringify(sessionCache), { + overwrite: true, + }).catch(err => { debugSelectivity(`Couldn't save session cache for session '%s': %O`, this._wdSessionId, err); }); } diff --git a/src/browser/cdp/selectivity/fs-cache.ts b/src/browser/cdp/selectivity/fs-cache.ts index 1aa4d6abf..481bda1b1 100644 --- a/src/browser/cdp/selectivity/fs-cache.ts +++ b/src/browser/cdp/selectivity/fs-cache.ts @@ -86,6 +86,7 @@ export const setCachedSelectivityFile = async ( cacheType: CacheTypeValue, key: string, utf8Contents: string, + { overwrite = false }: { overwrite?: boolean } = {}, ): Promise => { if (!key) { throw new Error("Attepted to write cache with empty key"); @@ -96,7 +97,7 @@ export const setCachedSelectivityFile = async ( const flagFilePath = cacheFilePath + SELECTIVITY_CACHE_READY_SUFFIX; // Cache was already written - if (await wasModifiedAfterProcessStart(flagFilePath)) { + if (!overwrite && (await wasModifiedAfterProcessStart(flagFilePath))) { return; } @@ -106,7 +107,12 @@ export const setCachedSelectivityFile = async ( .lock(flagFilePath, { stale: 5000, update: 1000, - retries: { minTimeout: 50, maxTimeout: 50, retries: 1 }, + retries: { + factor: 2, + minTimeout: 50, + maxTimeout: 200, + retries: overwrite ? 30 : 1, + }, realpath: false, }) .catch(() => null); @@ -117,7 +123,7 @@ export const setCachedSelectivityFile = async ( } // Cache was written while trying to get lock - if (await wasModifiedAfterProcessStart(flagFilePath)) { + if (!overwrite && (await wasModifiedAfterProcessStart(flagFilePath))) { await releaseLock(); return; } diff --git a/src/browser/cdp/selectivity/index.ts b/src/browser/cdp/selectivity/index.ts index 7ae4c045e..92115abd9 100644 --- a/src/browser/cdp/selectivity/index.ts +++ b/src/browser/cdp/selectivity/index.ts @@ -258,24 +258,16 @@ export const startSelectivity = async (browser: ExistingBrowser): Promise - mapDependencyRelativePath({ scope: "browser", relativePath }) - : null; - - const mapTestplaneDepsRelativePath = mapDependencyRelativePath - ? (relativePath: string): string | boolean | void => - mapDependencyRelativePath({ scope: "testplane", relativePath }) - : null; - const testDependencyWriter = getTestDependenciesWriter(testDependenciesPath, compression); const browserDeps = transformSourceDependencies( { css: cssDependencies, js: jsDependencies, png: null }, - mapBrowserDepsRelativePath, + mapDependencyRelativePath, + "browser", ); const testplaneDeps = transformSourceDependencies( { css: null, js: getCollectedTestplaneJsDependencies(), png: getCollectedTestplanePngDependencies() }, - mapTestplaneDepsRelativePath, + mapDependencyRelativePath, + "testplane", ); process.send?.({ diff --git a/src/browser/cdp/selectivity/utils.ts b/src/browser/cdp/selectivity/utils.ts index 80e268545..8c419adab 100644 --- a/src/browser/cdp/selectivity/utils.ts +++ b/src/browser/cdp/selectivity/utils.ts @@ -17,6 +17,11 @@ import type { import { WEBPACK_PROTOCOL } from "./constants"; import { readJsonWithCompression } from "./json-utils"; import type { Test } from "../../../types"; +import { + SelectivityDependencyReason, + SelectivityDependencyScope, + SelectivityMapDependencyRelativePathFn, +} from "../../../config/types"; /** * Tries to fetch text by url from node.js, then falls back to "fetch" from browser, if node.js fetch fails @@ -224,7 +229,8 @@ export const transformSourceDependencies = ( js: jsDependencies, png: pngDependencies, }: { css: Set | null; js: Set | null; png: Set | null }, - mapDependencyPathFn?: null | ((relativePath: string) => string | boolean | void), + mapDependencyPathFn: null | SelectivityMapDependencyRelativePathFn, + scope: SelectivityDependencyScope, ): NormalizedDependencies => { const nodeModulesLabel = "node_modules/"; const cssSet: Set = new Set(); @@ -232,7 +238,11 @@ export const transformSourceDependencies = ( const modulesSet: Set = new Set(); const pngSet: Set = new Set(); - const classifyDependency = (dependency: string, typedResultSet: Set): void => { + const classifyDependency = ( + dependency: string, + typedResultSet: Set, + reason: SelectivityDependencyReason, + ): void => { dependency = decodeURIComponent(softFileURLToPath(dependency)); const protocol = getProtocol(dependency); @@ -243,7 +253,13 @@ export const transformSourceDependencies = ( } const initialDependencyRelativePath = path.posix.relative(path.posix.resolve(), path.posix.resolve(dependency)); - const mapDependencyPathResult = mapDependencyPathFn ? mapDependencyPathFn(initialDependencyRelativePath) : true; + const mapDependencyPathResult = mapDependencyPathFn + ? mapDependencyPathFn({ + scope, + reason, + relativePath: initialDependencyRelativePath, + }) + : true; if (!mapDependencyPathResult) { return; @@ -285,19 +301,25 @@ export const transformSourceDependencies = ( if (cssDependencies) { for (const cssDependency of cssDependencies.values()) { - classifyDependency(cssDependency, cssSet); + // Testplane can't have css dependencies + classifyDependency(cssDependency, cssSet, "browser-css-import"); } } if (jsDependencies) { for (const jsDependency of jsDependencies.values()) { - classifyDependency(jsDependency, jsSet); + classifyDependency( + jsDependency, + jsSet, + scope === "browser" ? "browser-js-coverage" : "testplane-js-import", + ); } } if (pngDependencies) { for (const pngDependency of pngDependencies.values()) { - classifyDependency(pngDependency, pngSet); + // Browser png dependencies are not tracked + classifyDependency(pngDependency, pngSet, "testplane-assert-view-reference"); } } diff --git a/src/config/types.ts b/src/config/types.ts index c041c971e..8dfb5c014 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -306,6 +306,13 @@ export type StateOpts = { keepFile?: boolean; }; +export type SelectivityDependencyScope = "browser" | "testplane"; +export type SelectivityDependencyReason = + | "browser-css-import" + | "browser-js-coverage" + | "testplane-js-import" + | "testplane-assert-view-reference"; + /** * @param {Object} dependency - Object with dependency scope and posix relative path * @param {"browser"|"testplane"|string} dependency.scope - Dependency scope @@ -313,7 +320,8 @@ export type StateOpts = { * @returns mapped POSIX relative path, "true", if should be untouched, or "falsy", if should be ignored */ export type SelectivityMapDependencyRelativePathFn = (dependency: { - scope: "browser" | "testplane" | (string & NonNullable); + scope: SelectivityDependencyScope; + reason: SelectivityDependencyReason; relativePath: string; }) => string | boolean | void; diff --git a/test/src/browser/cdp/selectivity/fs-cache.ts b/test/src/browser/cdp/selectivity/fs-cache.ts index bba56c7a4..f6280480f 100644 --- a/test/src/browser/cdp/selectivity/fs-cache.ts +++ b/test/src/browser/cdp/selectivity/fs-cache.ts @@ -276,7 +276,7 @@ describe("CDP/Selectivity/FsCache", () => { assert.calledWith(lockfileStub.lock, `/tmp/${SELECTIVITY_CACHE_DIRECTIRY}/thash-of-${key}-ready`, { stale: 5000, update: 1000, - retries: { minTimeout: 50, maxTimeout: 50, retries: 1 }, + retries: { factor: 2, minTimeout: 50, maxTimeout: 200, retries: 1 }, realpath: false, }); assert.calledWith(fsStub.writeFile, `/tmp/${SELECTIVITY_CACHE_DIRECTIRY}/thash-of-${key}`, content, { @@ -424,5 +424,117 @@ describe("CDP/Selectivity/FsCache", () => { assert.calledWith(fsStub.writeFile, `/tmp/${SELECTIVITY_CACHE_DIRECTIRY}/cs${key}-ready`, ""); assert.notCalled(getMD5Stub); }); + + describe("overwrite option", () => { + it("should write cache even if it already exists when overwrite is true", async () => { + const key = "test-key"; + const cacheType = CacheType.TestFile; + const content = "new content"; + const futureTime = Date.now() + 10000; + const releaseLockStub = sandbox.stub().resolves(); + + fsStub.stat.resolves({ mtimeMs: futureTime }); + lockfileStub.lock.resolves(releaseLockStub); + + await setCachedSelectivityFile(cacheType, key, content, { overwrite: true }); + + assert.calledOnce(fsStub.ensureDir); + assert.calledOnce(lockfileStub.lock); + assert.calledWith(fsStub.writeFile, `/tmp/${SELECTIVITY_CACHE_DIRECTIRY}/thash-of-${key}`, content, { + encoding: "utf8", + }); + assert.calledWith(fsStub.writeFile, `/tmp/${SELECTIVITY_CACHE_DIRECTIRY}/thash-of-${key}-ready`, ""); + assert.calledOnce(releaseLockStub); + }); + + it("should not write cache if it already exists when overwrite is false", async () => { + const key = "test-key"; + const cacheType = CacheType.TestFile; + const content = "new content"; + const futureTime = Date.now() + 10000; + + fsStub.stat.resolves({ mtimeMs: futureTime }); + + await setCachedSelectivityFile(cacheType, key, content, { overwrite: false }); + + assert.notCalled(fsStub.ensureDir); + assert.notCalled(lockfileStub.lock); + assert.notCalled(fsStub.writeFile); + }); + + it("should use 10 lock retries when overwrite is true", async () => { + const key = "test-key"; + const cacheType = CacheType.TestFile; + const content = "test content"; + const pastTime = 0; + const releaseLockStub = sandbox.stub().resolves(); + + fsStub.stat.resolves({ mtimeMs: pastTime }); + lockfileStub.lock.resolves(releaseLockStub); + + await setCachedSelectivityFile(cacheType, key, content, { overwrite: true }); + + assert.calledWith(lockfileStub.lock, `/tmp/${SELECTIVITY_CACHE_DIRECTIRY}/thash-of-${key}-ready`, { + stale: 5000, + update: 1000, + retries: { factor: 2, minTimeout: 50, maxTimeout: 200, retries: 30 }, + realpath: false, + }); + }); + + it("should use 1 lock retry when overwrite is false", async () => { + const key = "test-key"; + const cacheType = CacheType.TestFile; + const content = "test content"; + const pastTime = 0; + const releaseLockStub = sandbox.stub().resolves(); + + fsStub.stat.resolves({ mtimeMs: pastTime }); + lockfileStub.lock.resolves(releaseLockStub); + + await setCachedSelectivityFile(cacheType, key, content, { overwrite: false }); + + assert.calledWith(lockfileStub.lock, `/tmp/${SELECTIVITY_CACHE_DIRECTIRY}/thash-of-${key}-ready`, { + stale: 5000, + update: 1000, + retries: { factor: 2, minTimeout: 50, maxTimeout: 200, retries: 1 }, + realpath: false, + }); + }); + + it("should write cache even if it was created while acquiring lock when overwrite is true", async () => { + const key = "test-key"; + const cacheType = CacheType.Asset; + const content = "new content"; + const pastTime = 0; + const futureTime = Date.now() + 10000; + const releaseLockStub = sandbox.stub().resolves(); + + fsStub.stat.onCall(0).resolves({ mtimeMs: pastTime }); // Flag not ready + fsStub.stat.onCall(1).resolves({ mtimeMs: futureTime }); // Cache file exists after lock + lockfileStub.lock.resolves(releaseLockStub); + + await setCachedSelectivityFile(cacheType, key, content, { overwrite: true }); + + assert.calledWith(fsStub.writeFile, `/tmp/${SELECTIVITY_CACHE_DIRECTIRY}/ahash-of-${key}`, content, { + encoding: "utf8", + }); + assert.calledWith(fsStub.writeFile, `/tmp/${SELECTIVITY_CACHE_DIRECTIRY}/ahash-of-${key}-ready`, ""); + assert.calledOnce(releaseLockStub); + }); + + it("should default overwrite to false when options object is not provided", async () => { + const key = "test-key"; + const cacheType = CacheType.TestFile; + const content = "test content"; + const futureTime = Date.now() + 10000; + + fsStub.stat.resolves({ mtimeMs: futureTime }); + + await setCachedSelectivityFile(cacheType, key, content); + + assert.notCalled(fsStub.writeFile); + }); + }); }); }); diff --git a/test/src/browser/cdp/selectivity/index.ts b/test/src/browser/cdp/selectivity/index.ts index 4e5f12e9a..09345ba7d 100644 --- a/test/src/browser/cdp/selectivity/index.ts +++ b/test/src/browser/cdp/selectivity/index.ts @@ -364,11 +364,18 @@ describe("CDP/Selectivity", () => { assert.calledWith(cssSelectivityMock.stop, false); assert.calledWith(jsSelectivityMock.stop, false); - assert.calledWith(transformSourceDependenciesStub, { - css: new Set(["src/styles.css"]), - js: new Set(["src/app.js"]), - png: null, - }); + assert.calledWith( + transformSourceDependenciesStub, + { css: new Set(["src/styles.css"]), js: new Set(["src/app.js"]), png: null }, + null, + "browser", + ); + assert.calledWith( + transformSourceDependenciesStub, + { css: null, js: sinon.match.any, png: sinon.match.any }, + null, + "testplane", + ); assert.calledWith(getTestDependenciesWriterStub, "/test/dependencies"); assert.calledWith(testDependenciesWriterMock.saveFor, mockTest, { css: ["src/styles.css"], @@ -378,8 +385,8 @@ describe("CDP/Selectivity", () => { }); it("should not save when no dependencies are found", async () => { - cssSelectivityMock.stop.resolves([]); - jsSelectivityMock.stop.resolves([]); + cssSelectivityMock.stop.resolves(new Set()); + jsSelectivityMock.stop.resolves(new Set()); await stopFn(mockTest, false); @@ -415,11 +422,16 @@ describe("CDP/Selectivity", () => { }); it("should save dependencies when only CSS dependencies exist", async () => { - jsSelectivityMock.stop.resolves([]); + jsSelectivityMock.stop.resolves(new Set()); await stopFn(mockTest, false); - assert.calledWith(transformSourceDependenciesStub, { css: new Set(["src/styles.css"]), js: [], png: null }); + assert.calledWith( + transformSourceDependenciesStub, + { css: new Set(["src/styles.css"]), js: new Set(), png: null }, + null, + "browser", + ); assert.calledOnce(testDependenciesWriterMock.saveFor); }); @@ -428,9 +440,48 @@ describe("CDP/Selectivity", () => { await stopFn(mockTest, false); - assert.calledWith(transformSourceDependenciesStub, { css: null, js: new Set(["src/app.js"]), png: null }); + assert.calledWith( + transformSourceDependenciesStub, + { css: null, js: new Set(["src/app.js"]), png: null }, + null, + "browser", + ); assert.calledOnce(testDependenciesWriterMock.saveFor); }); + + it("should call transformSourceDependencies with 'browser' scope for browser deps", async () => { + await stopFn(mockTest, false); + + const browserCall = transformSourceDependenciesStub + .getCalls() + .find((call: sinon.SinonSpyCall) => call.args[2] === "browser"); + + assert.ok(browserCall, "expected a call with scope 'browser'"); + assert.equal(browserCall!.args[2], "browser"); + }); + + it("should call transformSourceDependencies with 'testplane' scope for testplane deps", async () => { + await stopFn(mockTest, false); + + const testplaneCall = transformSourceDependenciesStub + .getCalls() + .find((call: sinon.SinonSpyCall) => call.args[2] === "testplane"); + + assert.ok(testplaneCall, "expected a call with scope 'testplane'"); + assert.equal(testplaneCall!.args[2], "testplane"); + }); + + it("should pass mapDependencyRelativePath directly to transformSourceDependencies", async () => { + const mapFn = sinon.stub().returns(true); + (browserMock.config.selectivity as any).mapDependencyRelativePath = mapFn; + + // startSelectivity captures mapDependencyRelativePath via closure, so we must start a new session + const customStopFn = await startSelectivity(browserMock as unknown as ExistingBrowser); + await customStopFn(mockTest as unknown as import("src/types").Test, false); + + assert.calledWith(transformSourceDependenciesStub, sinon.match.any, mapFn, "browser"); + assert.calledWith(transformSourceDependenciesStub, sinon.match.any, mapFn, "testplane"); + }); }); describe("updateSelectivityHashes", () => { diff --git a/test/src/browser/cdp/selectivity/utils.ts b/test/src/browser/cdp/selectivity/utils.ts index 4ac388d9e..bc0e6cb13 100644 --- a/test/src/browser/cdp/selectivity/utils.ts +++ b/test/src/browser/cdp/selectivity/utils.ts @@ -490,7 +490,7 @@ describe("CDP/Selectivity/Utils", () => { fsStub.existsSync.returns(true); - const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }); + const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }, null, "browser"); assert.deepEqual(result.css, ["src/styles.css"]); assert.deepEqual(result.js, ["src/app.js"]); @@ -503,7 +503,7 @@ describe("CDP/Selectivity/Utils", () => { fsStub.existsSync.returns(true); - const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }); + const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }, null, "browser"); assert.deepEqual(result.modules, ["node_modules/@scope/package"]); }); @@ -515,7 +515,7 @@ describe("CDP/Selectivity/Utils", () => { fsStub.existsSync.returns(false); assert.throws(() => { - utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }); + utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }, null, "browser"); }, /Selectivity: Couldn't find/); }); @@ -527,14 +527,20 @@ describe("CDP/Selectivity/Utils", () => { pathStub.posix.relative.returns("src/file with spaces.css"); fsStub.existsSync.returns(true); - const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }); + const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }, null, "browser"); assert.calledWith(softFileURLToPathStub, "src/file%20with%20spaces.css"); assert.deepEqual(result.css, ["src/file with spaces.css"]); }); it("should map dependencies using passed function", () => { - const mapFn = (relativePath: string): string | void => { + const mapFn = ({ + relativePath, + }: { + scope: string; + reason: string; + relativePath: string; + }): string | void => { if (relativePath === "ignore") { return; } @@ -549,7 +555,7 @@ describe("CDP/Selectivity/Utils", () => { fsStub.existsSync.returns(true); - const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }, mapFn); + const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }, mapFn, "browser"); assert.deepEqual(result.js, ["../bar"]); }); @@ -559,7 +565,7 @@ describe("CDP/Selectivity/Utils", () => { fsStub.existsSync.returns(true); - const result = utils.transformSourceDependencies({ css: null, js: null, png: pngDeps }); + const result = utils.transformSourceDependencies({ css: null, js: null, png: pngDeps }, null, "testplane"); assert.deepEqual(result.png, ["screenshots/ref1.png", "screenshots/ref2.png"]); }); @@ -572,7 +578,7 @@ describe("CDP/Selectivity/Utils", () => { fsStub.existsSync.returns(true); - const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }, mapFn); + const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }, mapFn, "browser"); assert.calledOnce(mapFn); assert.deepEqual(result.js, ["src/app.js"]); @@ -586,11 +592,71 @@ describe("CDP/Selectivity/Utils", () => { fsStub.existsSync.returns(true); - const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }, mapFn); + const result = utils.transformSourceDependencies({ css: cssDeps, js: jsDeps, png: null }, mapFn, "browser"); assert.deepEqual(result.css, []); assert.deepEqual(result.js, []); }); + + it("should pass scope to mapDependencyPathFn for each dependency", () => { + const mapFn = sinon.stub().returns(true); + const jsDeps = new Set(["src/app.js"]); + + fsStub.existsSync.returns(true); + + utils.transformSourceDependencies({ css: null, js: jsDeps, png: null }, mapFn, "testplane"); + + assert.calledOnce(mapFn); + assert.calledWith(mapFn, sinon.match({ scope: "testplane" })); + }); + + it("should pass 'browser-css-import' reason for CSS dependencies", () => { + const mapFn = sinon.stub().returns(true); + const cssDeps = new Set(["src/styles.css"]); + + fsStub.existsSync.returns(true); + + utils.transformSourceDependencies({ css: cssDeps, js: null, png: null }, mapFn, "browser"); + + assert.calledOnce(mapFn); + assert.calledWith(mapFn, sinon.match({ reason: "browser-css-import" })); + }); + + it("should pass 'browser-js-coverage' reason for JS dependencies with browser scope", () => { + const mapFn = sinon.stub().returns(true); + const jsDeps = new Set(["src/app.js"]); + + fsStub.existsSync.returns(true); + + utils.transformSourceDependencies({ css: null, js: jsDeps, png: null }, mapFn, "browser"); + + assert.calledOnce(mapFn); + assert.calledWith(mapFn, sinon.match({ reason: "browser-js-coverage" })); + }); + + it("should pass 'testplane-js-import' reason for JS dependencies with testplane scope", () => { + const mapFn = sinon.stub().returns(true); + const jsDeps = new Set(["src/app.js"]); + + fsStub.existsSync.returns(true); + + utils.transformSourceDependencies({ css: null, js: jsDeps, png: null }, mapFn, "testplane"); + + assert.calledOnce(mapFn); + assert.calledWith(mapFn, sinon.match({ reason: "testplane-js-import" })); + }); + + it("should pass 'testplane-assert-view-reference' reason for PNG dependencies", () => { + const mapFn = sinon.stub().returns(true); + const pngDeps = new Set(["screenshots/ref.png"]); + + fsStub.existsSync.returns(true); + + utils.transformSourceDependencies({ css: null, js: null, png: pngDeps }, mapFn, "testplane"); + + assert.calledOnce(mapFn); + assert.calledWith(mapFn, sinon.match({ reason: "testplane-assert-view-reference" })); + }); }); describe("mergeSourceDependencies", () => {