Reduce time for fidelity test CI locally by 50s, Github CI by 130s (g…

…oogle#4543) * reuse browser, and create page ahead of time. * fix log messages, now that browser only launches once.
vidinoti · Apr 22, 2024 · da21abf · da21abf
1 parent 9aad6f5
commit da21abf
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 16 deletions.
diff --git a/packages/render-fidelity-tools/src/artifact-creator.ts b/packages/render-fidelity-tools/src/artifact-creator.ts
@@ -17,11 +17,12 @@ import {promises as fs} from 'fs';
 import mkdirp from 'mkdirp';
 import {join, resolve} from 'path';
 import pngjs from 'pngjs';
-import puppeteer from 'puppeteer';
+import puppeteer, { Browser } from 'puppeteer';
 
 import {DEVICE_PIXEL_RATIO, Dimensions, FIDELITY_TEST_THRESHOLD, FidelityRegressionResults, GoldenConfig, ImageComparator, ImageComparisonAnalysis, ImageComparisonConfig, ScenarioConfig, toDecibel} from './common.js';
 import {ConfigReader} from './config-reader.js';
 
+
 const $configReader = Symbol('configReader');
 
 export type AnalysisResults = Array<ImageComparisonAnalysis>;
@@ -32,13 +33,27 @@ export interface ScenarioRecord extends ScenarioConfig {
 
 export class ArtifactCreator {
   private[$configReader]: ConfigReader = new ConfigReader(this.config);
+  private browser: Browser | undefined = undefined;
+  private pagePromise: Promise<any> | undefined = undefined;
 
   constructor(
       protected config: ImageComparisonConfig, protected rootDirectory: string,
       protected baseUrl: string) {
     console.log('🌈 Preparing to capture screenshots for fidelity comparison');
   }
 
+  async close() {
+    if( this.pagePromise !== undefined ) {
+      const page = await this.pagePromise;
+      await page.close();
+      this.pagePromise = undefined;
+    }
+
+    if (this.browser !== undefined) {
+      await this.browser.close();
+      this.browser = undefined;
+    }
+  }
   protected get outputDirectory(): string {
     return join(resolve(this.rootDirectory), 'results');
   }
@@ -248,6 +263,7 @@ export class ArtifactCreator {
 
     return analysis;
   }
+
 
   async captureScreenshot(
       renderer: string, scenarioName: string, dimensions: Dimensions,
@@ -263,21 +279,26 @@ export class ArtifactCreator {
       return;
     }
 
-    console.log(`🚀 Launching browser`);
-
-    const browser = await puppeteer.launch({
-      defaultViewport: {
-        width: scaledWidth,
-        height: scaledHeight,
-        deviceScaleFactor: DEVICE_PIXEL_RATIO
-      },
-      headless: false
-    });
+
+    if( this.browser == undefined) {
+      console.log(`🚀 Launching browser`);
+      this.browser = await puppeteer.launch({
+        headless: false,          
+      });
+      this.pagePromise = this.browser.newPage();
+    }
 
-    const page = await browser.newPage();
+    const page = await this.pagePromise;
+    this.pagePromise = undefined;
     const url = `${this.baseUrl}?hide-ui&config=../../config.json&scenario=${
         encodeURIComponent(scenarioName)}`;
 
+    await page.setViewport({
+      width: scaledWidth,
+      height: scaledHeight,
+      deviceScaleFactor: DEVICE_PIXEL_RATIO
+    });
+
     page.on('error', (error: any) => {
       console.log(`🚨 ${error}`);
     });
@@ -303,7 +324,7 @@ export class ArtifactCreator {
     // variables are captured in its closure scope. TypeScript compiler
     // currently has no mechanism to detect this and will happily tell you
     // your code is correct when it isn't.
-    const evaluateError = await page.evaluate(async (maxTimeInSec) => {
+    const evaluateError = await page.evaluate(async (maxTimeInSec: number) => {
       const modelBecomesReady = new Promise<void>((resolve, reject) => {
         let timeout: NodeJS.Timeout;
         if (maxTimeInSec > 0) {
@@ -331,7 +352,8 @@ export class ArtifactCreator {
 
     if (evaluateError) {
       console.log(evaluateError);
-      await browser.close();
+      await this.browser.close();
+      this.browser = undefined;
       throw new Error(evaluateError);
     }
 
@@ -346,8 +368,9 @@ export class ArtifactCreator {
     const screenshot =
         await page.screenshot({path: outputPath, omitBackground: true});
 
-    await browser.close();
-
+    page.close();
+    this.pagePromise = this.browser.newPage();
+
     return screenshot;
   }
 }
diff --git a/packages/render-fidelity-tools/src/workflows/test-fidelity.ts b/packages/render-fidelity-tools/src/workflows/test-fidelity.ts
@@ -62,6 +62,7 @@ screenshotCreator.fidelityTest(scenarioWhitelist)
     .then(() => {
       console.log(`✅ Results recorded to ${outputDirectory}`);
       server.close();
+      screenshotCreator.close();
 
       const fidelityRegressionPath =
           join(outputDirectory, 'fidelityRegressionResults.json');

diff --git a/packages/render-fidelity-tools/src/workflows/update-screenshots/renderer-screenshot.ts b/packages/render-fidelity-tools/src/workflows/update-screenshots/renderer-screenshot.ts
@@ -53,5 +53,6 @@ export const rendererScreenshot = async(
         renderer, scenarioName, dimensions, outputFile);
   } finally {
     server.close();
+    screenshotCreator.close();
   }
 };