-
Notifications
You must be signed in to change notification settings - Fork 74
feat(selectivity): consider png reference deps #1245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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>> = {}; | ||
|
|
||
|
|
@@ -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 => { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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>, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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)); | ||
|
|
@@ -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), | ||
| }; | ||
| }; | ||
|
|
||
|
|
@@ -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]) { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
|
||
|
|
@@ -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">> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: |
||
| >; | ||
|
|
||
| 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; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -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, [ | ||
|
|
@@ -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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
давай просто объединим типы зависимостей в один аргумент deps. Сейчас набор аргументов вида
null, jsDeps, nullвыглядит нечитаено. Ну и ты еще должен запоминать в какой последовательности нужно передать эти зависимости.