Merge pull request #1211 from gemini-testing/TESTPLANE-904.calc_hash_in_worker

perf(selectivity): speed up pre-test checks

Author: KuznetsovRoman

src/browser/cdp/selectivity/hash-provider.ts

@@ -18,7 +18,7 @@ const calculateFileMd5Hash = (filePath: string): Promise<string> =>
 export class HashProvider {
     private static readonly _fileHashStore: Map<string, Promise<string>> = new Map();
     private static readonly _patternHashStore: Map<string, Promise<string>> = new Map();
-    private static readonly _limited = pLimit(10);
+    private static readonly _limited = pLimit(16);
 
     async calculateForFile(filePath: string): Promise<string> {
         const cachedHash = HashProvider._fileHashStore.get(filePath);

src/browser/cdp/selectivity/hash-reader.ts

@@ -10,6 +10,7 @@ export class HashReader {
     private readonly _selectivityHashesPath: string;
     private readonly _compresion: SelectivityCompressionType;
     private _hashFileContents: Promise<HashFileContents> | null = null;
+    private _fileStateCache = new Map<string, boolean>();
 
     constructor(selectivityRootPath: string, compression: SelectivityCompressionType) {
         this._selectivityHashesPath = getSelectivityHashesPath(selectivityRootPath);
@@ -24,60 +25,64 @@ export class HashReader {
         return (this._hashFileContents = readHashFileContents(this._selectivityHashesPath, this._compresion));
     }
 
-    private _readHashForFile(filePath: string): Promise<string> {
-        return this._getHashFileContents().then(hashFileContents => hashFileContents.files[filePath]);
-    }
-
-    private _readHashForModule(moduleName: string): Promise<string> {
-        return this._getHashFileContents().then(hashFileContents => hashFileContents.modules[moduleName]);
-    }
-
-    private _readHashForPattern(pattern: string): Promise<string> {
-        return this._getHashFileContents().then(hashFileContents => hashFileContents.patterns[pattern]);
-    }
-
     async patternHasChanged(pattern: string): Promise<boolean> {
-        const [cachedPatternHash, calculatedPatternHash] = await Promise.all([
-            this._readHashForPattern(pattern),
-            this._hashProvider.calculateForPattern(pattern),
-        ]);
+        const fileContents = await this._getHashFileContents();
+        const cachedPatternHash = fileContents.patterns[pattern];
+        const calculatedPatternHash = await this._hashProvider.calculateForPattern(pattern);
 
         return cachedPatternHash !== calculatedPatternHash;
     }
 
     /** @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 result: NormalizedDependencies = { css: [], js: [], modules: [] };
+        const fileContents = await this._getHashFileContents();
 
-        let hasAnythingChanged = false;
+        let result: NormalizedDependencies | null = null;
 
         const checkForDepFileType = async (depFileType: keyof NormalizedDependencies): Promise<void> => {
-            await Promise.all(
-                testDeps[depFileType].map(async filePath => {
-                    const adjustedFilePath = depFileType === "modules" ? path.join(filePath, "package.json") : filePath;
-                    const [cachedFileHash, calculatedFileHash] = await Promise.all([
-                        depFileType === "modules" ? this._readHashForModule(filePath) : this._readHashForFile(filePath),
-                        this._hashProvider.calculateForFile(adjustedFilePath).catch((err: Error) => err),
-                    ]);
-
-                    if (calculatedFileHash instanceof Error) {
-                        debugSelectivity(
-                            `Couldn't calculate hash for ${adjustedFilePath}: ${calculatedFileHash.message}`,
-                        );
-                    }
-
-                    if (cachedFileHash !== calculatedFileHash) {
-                        hasAnythingChanged = true;
-                        result[depFileType].push(filePath);
-                    }
-                }),
-            );
+            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[depFileType].push(filePath);
+                    continue;
+                }
+
+                const adjustedFilePath = depFileType === "modules" ? path.join(filePath, "package.json") : filePath;
+                const cachedFileHash =
+                    depFileType === "modules" ? fileContents.modules[filePath] : fileContents.files[filePath];
+
+                const calculatedFileHash = await this._hashProvider
+                    .calculateForFile(adjustedFilePath)
+                    .catch((err: Error) => err);
+
+                if (calculatedFileHash instanceof Error) {
+                    debugSelectivity(`${calculatedFileHash.message}: ${calculatedFileHash.cause}`);
+                }
+
+                if (cachedFileHash !== calculatedFileHash) {
+                    result ||= { css: [], js: [], modules: [] };
+                    result[depFileType].push(filePath);
+                    this._fileStateCache.set(filePath, true);
+                } else {
+                    this._fileStateCache.set(filePath, false);
+                }
+            }
         };
 
-        await Promise.all(depFileTypes.map(depFileType => checkForDepFileType(depFileType)));
+        for (const depFileType of depFileTypes) {
+            await checkForDepFileType(depFileType);
+        }
+
+        return result;
+    }
 
-        return hasAnythingChanged ? result : null;
+    clearCache(): void {
+        this._fileStateCache.clear();
     }
 }
 

src/browser/cdp/selectivity/index.ts

@@ -39,7 +39,11 @@ export const updateSelectivityHashes = async (config: Config): Promise<void> =>
             }
         }
 
-        await hashWriter.commit();
+        try {
+            await hashWriter.commit();
+        } catch (cause) {
+            throw new Error("Selectivity: couldn't save test dependencies hash", { cause });
+        }
     }
 };
 

src/browser/cdp/selectivity/runner.ts

@@ -119,7 +119,7 @@ export class SelectivityRunner {
     private readonly _opts?: SelectivityRunnerOptions;
     private readonly _browserSelectivityDisabledCache: Record<string, void | Promise<boolean>> = {};
     private readonly _testsToRun: [Test, string][] = [];
-    private readonly _processingTestLimit = pLimit(10);
+    private readonly _processingTestLimit = pLimit(16);
     private readonly _processingTestPromises: Array<Promise<void>> = [];
 
     static create(...args: ConstructorParameters<typeof this>): SelectivityRunner {
@@ -197,5 +197,6 @@ export class SelectivityRunner {
 
         shouldDisableBrowserSelectivity.cache.clear?.();
         shouldDisableTestBySelectivity.cache.clear?.();
+        getHashReader(this._config.selectivity.testDependenciesPath, this._config.selectivity.compression).clearCache();
     }
 }

src/testplane.ts

@@ -208,7 +208,9 @@ export class Testplane extends BaseTestplane {
         );
 
         if (!shouldDisableSelectivity && !this.isFailed()) {
-            await updateSelectivityHashes(this.config);
+            await updateSelectivityHashes(this.config).catch(err => {
+                console.error("Skipping selectivity state update because of an error:", err);
+            });
         }
 
         if (this.config.afterAll) {

test/src/browser/cdp/selectivity/index.ts

@@ -412,5 +412,26 @@ describe("CDP/Selectivity", () => {
             assert.calledTwice(hashWriterMock.addPatternDependencyHash);
             assert.calledTwice(hashWriterMock.commit);
         });
+
+        it("should throw error with cause when commit fails", async () => {
+            configMock.getBrowserIds.returns(["chrome"]);
+            configMock.forBrowser.withArgs("chrome").returns({
+                selectivity: {
+                    enabled: true,
+                    testDependenciesPath: "/test/path",
+                    compression: "none",
+                    disableSelectivityPatterns: ["pattern1"],
+                },
+            });
+
+            hashReaderMock.patternHasChanged.resolves(true);
+            const commitError = new Error("Failed to write file");
+            hashWriterMock.commit.rejects(commitError);
+
+            await assert.isRejected(
+                updateSelectivityHashes(configMock as any),
+                /Selectivity: couldn't save test dependencies hash/,
+            );
+        });
     });
 });