Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/browser/cdp/selectivity/hash-reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,24 @@ export class HashReader {

/** @returns changed deps or null, if nothing changed */
async getTestChangedDeps(testDeps: NormalizedDependencies): Promise<NormalizedDependencies | null> {
const depFileTypes: Array<keyof NormalizedDependencies> = ["css", "js", "modules"] as const;
const depFileTypes: Array<keyof NormalizedDependencies> = ["css", "js", "modules", "png"] as const;
const fileContents = await this._getHashFileContents();

let result: NormalizedDependencies | null = null;

const checkForDepFileType = async (depFileType: keyof NormalizedDependencies): Promise<void> => {
// Old selectivity dependency files did not have "png" property
if (!testDeps[depFileType]) {
return;
}

for (const filePath of testDeps[depFileType]) {
const isChanged = this._fileStateCache.get(filePath);

if (isChanged === false) {
continue;
} else if (isChanged === true) {
result ||= { css: [], js: [], modules: [] };
result ||= { css: [], js: [], modules: [], png: [] };
result[depFileType].push(filePath);
continue;
}
Expand All @@ -65,7 +70,7 @@ export class HashReader {
}

if (cachedFileHash !== calculatedFileHash) {
result ||= { css: [], js: [], modules: [] };
result ||= { css: [], js: [], modules: [], png: [] };
result[depFileType].push(filePath);
this._fileStateCache.set(filePath, true);
} else {
Expand Down
7 changes: 4 additions & 3 deletions src/browser/cdp/selectivity/hash-writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ export class HashWriter {
}

addTestDependencyHashes(dependencies: NormalizedDependencies): void {
dependencies.css.forEach(dependency => this._addFileDependency(dependency));
dependencies.js.forEach(dependency => this._addFileDependency(dependency));
dependencies.modules.forEach(dependency => this._addModuleDependency(dependency));
dependencies.css?.forEach(dependency => this._addFileDependency(dependency));
dependencies.js?.forEach(dependency => this._addFileDependency(dependency));
dependencies.png?.forEach(dependency => this._addFileDependency(dependency));
dependencies.modules?.forEach(dependency => this._addModuleDependency(dependency));
}

async save(readExisting?: boolean): Promise<void> {
Expand Down
10 changes: 6 additions & 4 deletions src/browser/cdp/selectivity/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { Test, TestDepsContext, TestDepsData } from "../../../types";
import { getSelectivityTestsPath, mergeSourceDependencies, transformSourceDependencies } from "./utils";
import { getHashWriter } from "./hash-writer";
import { Compression } from "./types";
import { getCollectedTestplaneDependencies } from "./testplane-selectivity";
import { getCollectedTestplaneJsDependencies, getCollectedTestplanePngDependencies } from "./testplane-selectivity";
import { getHashReader } from "./hash-reader";
import type { Config } from "../../../config";
import { MasterEvents } from "../../../events";
Expand Down Expand Up @@ -257,10 +257,12 @@ export const startSelectivity = async (browser: ExistingBrowser): Promise<StopSe
: null;

const testDependencyWriter = getTestDependenciesWriter(testDependenciesPath, compression);
const browserDeps = transformSourceDependencies(cssDependencies, jsDependencies, mapBrowserDepsRelativePath);
const browserDeps = transformSourceDependencies(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

давай просто объединим типы зависимостей в один аргумент deps. Сейчас набор аргументов вида null, jsDeps, null выглядит нечитаено. Ну и ты еще должен запоминать в какой последовательности нужно передать эти зависимости.

{ css: cssDependencies, js: jsDependencies, png: null },
mapBrowserDepsRelativePath,
);
const testplaneDeps = transformSourceDependencies(
null,
getCollectedTestplaneDependencies(),
{ css: null, js: getCollectedTestplaneJsDependencies(), png: getCollectedTestplanePngDependencies() },
mapTestplaneDepsRelativePath,
);

Expand Down
1 change: 1 addition & 0 deletions src/browser/cdp/selectivity/merge-dumps/merge-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ const mergeJsonTestContents = (
css: [],
js: [],
modules: [],
png: [],
};

for (const dependencyType in testContent[browserId][dependencyScope]) {
Expand Down
2 changes: 1 addition & 1 deletion src/browser/cdp/selectivity/test-dependencies-reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class TestDependenciesReader {

async getFor(test: Test): Promise<NormalizedDependencies> {
const testDeps = await readTestDependencies(this._selectivityTestsPath, test, this._compression);
let result: NormalizedDependencies = { css: [], js: [], modules: [] };
let result: NormalizedDependencies = { css: [], js: [], modules: [], png: [] };

for (const browserId of Object.keys(testDeps)) {
const depTypes = Object.keys(testDeps[browserId]);
Expand Down
2 changes: 1 addition & 1 deletion src/browser/cdp/selectivity/test-dependencies-writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { NormalizedDependencies, SelectivityCompressionType } from "./types
import { writeJsonWithCompression } from "./json-utils";

const areDepsSame = (browserDepsA?: NormalizedDependencies, browserDepsB?: NormalizedDependencies): boolean => {
const props: Array<keyof NormalizedDependencies> = ["js", "css", "modules"] as const;
const props: Array<keyof NormalizedDependencies> = ["js", "css", "modules", "png"] as const;

if (!browserDepsA || !browserDepsB) {
return false;
Expand Down
29 changes: 25 additions & 4 deletions src/browser/cdp/selectivity/testplane-selectivity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import { debugSelectivity } from "./debug";

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const TypedModule = UntypedModule as unknown as { _resolveFilename: (...args: any) => string | void };
const testDependenciesStorage = new AsyncLocalStorage<{ jsTestplaneDeps?: Set<string> }>();
const testDependenciesStorage = new AsyncLocalStorage<{
jsTestplaneDeps?: Set<string>;
pngTestplaneDeps?: Set<string>;
}>();
const testFileDependenciesRamCache = new Map<string, string[]>();
const testFileLocks: Record<string, Promise<void>> = {};

Expand Down Expand Up @@ -49,24 +52,42 @@ export const enableCollectingTestplaneDependencies = (): void => {
};
};

export const getCollectedTestplaneDependencies = (): Set<string> | null => {
export const getCollectedTestplaneJsDependencies = (): Set<string> | null => {
const store = testDependenciesStorage.getStore();

return store && store.jsTestplaneDeps ? store.jsTestplaneDeps : null;
};

export const getCollectedTestplanePngDependencies = (): Set<string> | null => {
const store = testDependenciesStorage.getStore();

return store && store.pngTestplaneDeps ? store.pngTestplaneDeps : null;
};

export const runWithTestplaneDependenciesCollecting = <T>(fn: () => Promise<T>): Promise<T> => {
enableCollectingTestplaneDependencies();

const store: { jsTestplaneDeps?: Set<string> } = { jsTestplaneDeps: new Set() };
const store: {
jsTestplaneDeps?: Set<string>;
pngTestplaneDeps?: Set<string>;
} = { jsTestplaneDeps: new Set(), pngTestplaneDeps: new Set() };

return testDependenciesStorage.run(store, fn).finally(() => {
// After "fn" completion, "store" is reachable in CDP ping interval callback, so it never GC-removed
// Thats why we do it manually. Removing "jsTestplaneDeps" is enough, and set remains unchanged, if used
// Thats why we do it manually. It is enough, and set remains unchanged, if used
delete store.jsTestplaneDeps;
delete store.pngTestplaneDeps;
});
};

export const addTestplaneSelectivityPngDependency = (pngPath: string): void => {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When called during test run, adds png dependency to test dependencies

Called on "assert-view"

const store = testDependenciesStorage.getStore();

if (store && store.pngTestplaneDeps) {
store.pngTestplaneDeps.add(pngPath);
}
};

export const readTestFileWithTestplaneDependenciesCollecting = async <T>(
file: string,
fn: () => Promise<T>,
Expand Down
2 changes: 2 additions & 0 deletions src/browser/cdp/selectivity/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ export interface NormalizedDependencies {
js: string[];
/** Module names from node_modules (e.g. "react", "@remix-run/router") */
modules: string[];
/** Reference paths */
png: string[];
}

export const Compression = {
Expand Down
61 changes: 52 additions & 9 deletions src/browser/cdp/selectivity/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,19 +213,24 @@ const warnUnsupportedProtocol = memoize((protocol: string, dependency: string):
});

/**
* @param cssDependencies set of css dependenciy URI's
* @param jsDependencies set of js dependenciy URI's
* @param dependencies.css set of css dependency URI's
* @param dependencies.js set of js dependency URI's
* @param dependencies.png set of png dependency URI's
* @returns sorted uniq arrays of relative paths
*/
export const transformSourceDependencies = (
cssDependencies: Set<string> | null,
jsDependencies: Set<string> | null,
{
css: cssDependencies,
js: jsDependencies,
png: pngDependencies,
}: { css: Set<string> | null; js: Set<string> | null; png: Set<string> | null },
mapDependencyPathFn?: null | ((relativePath: string) => string | void),
): NormalizedDependencies => {
const nodeModulesLabel = "node_modules/";
const cssSet: Set<string> = new Set();
const jsSet: Set<string> = new Set();
const modulesSet: Set<string> = new Set();
const pngSet: Set<string> = new Set();

const classifyDependency = (dependency: string, typedResultSet: Set<string>): void => {
dependency = decodeURIComponent(softFileURLToPath(dependency));
Expand Down Expand Up @@ -291,12 +296,19 @@ export const transformSourceDependencies = (
}
}

if (pngDependencies) {
for (const pngDependency of pngDependencies.values()) {
classifyDependency(pngDependency, pngSet);
}
}

const cmpStr = (a: string, b: string): number => a.localeCompare(b);

return {
css: Array.from(cssSet).sort(cmpStr),
js: Array.from(jsSet).sort(cmpStr),
modules: Array.from(modulesSet).sort(cmpStr),
png: Array.from(pngSet).sort(cmpStr),
};
};

Expand All @@ -305,12 +317,26 @@ export const mergeSourceDependencies = (
a: NormalizedDependencies,
b: NormalizedDependencies,
): NormalizedDependencies => {
const result: NormalizedDependencies = { css: [], js: [], modules: [] };
const result: NormalizedDependencies = { css: [], js: [], modules: [], png: [] };

for (const depType of Object.keys(result) as Array<keyof NormalizedDependencies>) {
let aInd = 0,
bInd = 0;

if (!a[depType]) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicit checks "what if old format had no png dependencies property"

if (!b[depType]) {
continue;
}

result[depType] = b[depType];

continue;
} else if (!b[depType]) {
result[depType] = a[depType];

continue;
}

while (aInd < a[depType].length || bInd < b[depType].length) {
let compareResult;

Expand Down Expand Up @@ -392,13 +418,30 @@ export const getTestDependenciesPath = (selectivityTestsPath: string, test: Test
path.join(selectivityTestsPath, `${getTestSelectivityDumpId(test)}.json`);

/** @returns `Promise<Record<BrowserID, Record<DepType, NormalizedDependencies>>>` */
export const readTestDependencies = (
export const readTestDependencies = async (
selectivityTestsPath: string,
test: Test,
compression: SelectivityCompressionType,
): Promise<TestDependenciesFileContents> =>
readJsonWithCompression(getTestDependenciesPath(selectivityTestsPath, test), compression, {
): Promise<TestDependenciesFileContents> => {
const result = (await readJsonWithCompression(getTestDependenciesPath(selectivityTestsPath, test), compression, {
defaultValue: {},
}).catch(() => ({}));
}).catch(() => ({}))) as Record<
string,
Record<string, Partial<NormalizedDependencies> & Pick<NormalizedDependencies, "css" | "js" | "modules" | "png">>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicit type check "those properties were added on read and guaranteed to exist"

If new property will be added, typescript would complain:

Types of property 'newTypeDeps' are incompatible.
            Type 'string[] | undefined' is not assignable to type 'string[]'.
              Type 'undefined' is not assignable to type 'string[]'.ts(2322)

>;

for (const browserId in result) {
for (const depType in result[browserId]) {
const currentDeps = result[browserId][depType];

currentDeps.css ||= [];
currentDeps.js ||= [];
currentDeps.modules ||= [];
currentDeps.png ||= [];
}
}

return result;
};

export const isCachedOnFs = (value: unknown): value is CachedOnFs => value === true;
9 changes: 8 additions & 1 deletion src/browser/commands/assert-view/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const { getCaptureProcessors } = require("./capture-processors");
const RuntimeConfig = require("../../../config/runtime-config");
const AssertViewResults = require("./assert-view-results");
const { BaseStateError } = require("./errors/base-state-error");
const { addTestplaneSelectivityPngDependency } = require("../../cdp/selectivity/testplane-selectivity");

const getIgnoreDiffPixelCountRatio = value => {
const percent = _.isString(value) && value.endsWith("%") ? parseFloat(value.slice(0, -1)) : false;
Expand Down Expand Up @@ -75,7 +76,7 @@ module.exports.default = browser => {
disableAnimation: opts.disableAnimation,
});

const { tempOpts } = RuntimeConfig.getInstance();
const { tempOpts, updateRefs: isUpdatingRefs } = RuntimeConfig.getInstance();
temp.attach(tempOpts);

const screenshoterOpts = _.pick(opts, [
Expand All @@ -99,9 +100,15 @@ module.exports.default = browser => {
if (!fs.existsSync(refImg.path)) {
await currImgInst.save(currImg.path);

if (isUpdatingRefs) {
addTestplaneSelectivityPngDependency(refImg.path);
Comment on lines +103 to +104
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"--update-refs" case

}

return handleNoRefImage(currImg, refImg, state, { emitter }).catch(e => handleCaptureProcessorError(e));
}

addTestplaneSelectivityPngDependency(refImg.path);

const { canHaveCaret, pixelRatio } = page;
const imageCompareOpts = {
tolerance: opts.tolerance,
Expand Down
Loading
Loading