chore: enable unit tests for Firefox on Windows (#6895)

Co-authored-by: Jan Scheffler <janscheffler@chromium.org>

.github/workflows/main.yml

@@ -155,3 +155,10 @@ jobs:
           CHROMIUM: true
         run: |
           npm run unit
+
+      - name: Run unit tests on Firefox
+        env:
+          FIREFOX: true
+          MOZ_WEBRENDER: 0
+        run: |
+          npm run funit

package.json

@@ -15,7 +15,7 @@
     "unit-debug": "npm run tsc-cjs && mocha --inspect-brk --config mocha-config/puppeteer-unit-tests.js",
     "unit-with-coverage": "cross-env COVERAGE=1 npm run unit",
     "assert-unit-coverage": "cross-env COVERAGE=1 mocha --config mocha-config/coverage-tests.js",
-    "funit": "PUPPETEER_PRODUCT=firefox npm run unit",
+    "funit": "cross-env PUPPETEER_PRODUCT=firefox npm run unit",
     "test": "npm run tsc && npm run lint --silent && npm run unit-with-coverage && npm run test-browser",
     "prepare": "node typescript-if-required.js",
     "prepublishOnly": "npm run build",

src/node/BrowserRunner.ts

@@ -24,6 +24,7 @@ import { LaunchOptions } from './LaunchOptions.js';
 import { Connection } from '../common/Connection.js';
 import { NodeWebSocketTransport as WebSocketTransport } from '../node/NodeWebSocketTransport.js';
 import { PipeTransport } from './PipeTransport.js';
+import { Product } from '../common/Product.js';
 import * as readline from 'readline';
 import { TimeoutError } from '../common/Errors.js';
 import { promisify } from 'util';
@@ -36,6 +37,7 @@ Please check your open processes and ensure that the browser processes that Pupp
 If you think this is a bug, please report it on the Puppeteer issue tracker.`;
 
 export class BrowserRunner {
+  private _product: Product;
   private _executablePath: string;
   private _processArguments: string[];
   private _tempDirectory?: string;
@@ -48,10 +50,12 @@ export class BrowserRunner {
   private _processClosing: Promise<void>;
 
   constructor(
+    product: Product,
     executablePath: string,
     processArguments: string[],
     tempDirectory?: string
   ) {
+    this._product = product;
     this._executablePath = executablePath;
     this._processArguments = processArguments;
     this._tempDirectory = tempDirectory;
@@ -128,7 +132,7 @@ export class BrowserRunner {
 
   close(): Promise<void> {
     if (this._closed) return Promise.resolve();
-    if (this._tempDirectory) {
+    if (this._tempDirectory && this._product !== 'firefox') {
       this.kill();
     } else if (this.connection) {
       // Attempt to close the browser gracefully

src/node/Launcher.ts

@@ -116,6 +116,7 @@ class ChromeLauncher implements ProductLauncher {
 
     const usePipe = chromeArguments.includes('--remote-debugging-pipe');
     const runner = new BrowserRunner(
+      this.product,
       chromeExecutable,
       chromeArguments,
       temporaryUserDataDir
@@ -281,6 +282,7 @@ class FirefoxLauncher implements ProductLauncher {
     }
 
     const runner = new BrowserRunner(
+      this.product,
       firefoxExecutable,
       firefoxArguments,
       temporaryUserDataDir

test/launcher.spec.ts

@@ -22,7 +22,6 @@ import {
   getTestState,
   itFailsFirefox,
   itOnlyRegularInstall,
-  itFailsWindows,
 } from './mocha-utils'; // eslint-disable-line import/extensions
 import utils from './utils.js';
 import expect from 'expect';
@@ -339,7 +338,7 @@ describe('Launcher specs', function () {
         if (isChrome) expect(puppeteer.product).toBe('chrome');
         else if (isFirefox) expect(puppeteer.product).toBe('firefox');
       });
-      it('should work with no default arguments', async () => {
+      itFailsFirefox('should work with no default arguments', async () => {
         const { defaultBrowserOptions, puppeteer } = getTestState();
         const options = Object.assign({}, defaultBrowserOptions);
         options.ignoreDefaultArgs = true;
@@ -470,11 +469,7 @@ describe('Launcher specs', function () {
         ]);
       });
 
-      /* We think there's a bug in the FF Windows launcher, or some
+      it('should be able to launch Firefox', async function () {
-       * combo of that plus it running on CI, but it's hard to track down.
-       * See comment here: https://github.com/puppeteer/puppeteer/issues/5673#issuecomment-670141377.
-       */
-      itFailsWindows('should be able to launch Firefox', async function () {
         this.timeout(FIREFOX_TIMEOUT);
         const { puppeteer } = getTestState();
         const browser = await puppeteer.launch({ product: 'firefox' });

test/screenshot.spec.ts

@@ -203,39 +203,42 @@ describe('Screenshots', function () {
       const screenshot = await elementHandle.screenshot();
       expect(screenshot).toBeGolden('screenshot-element-padding-border.png');
     });
-    it('should capture full element when larger than viewport', async () => {
+    itFailsFirefox(
-      const { page } = getTestState();
+      'should capture full element when larger than viewport',
+      async () => {
+        const { page } = getTestState();
 
-      await page.setViewport({ width: 500, height: 500 });
+        await page.setViewport({ width: 500, height: 500 });
 
-      await page.setContent(`
+        await page.setContent(`
-        something above
+          something above
-        <style>
+          <style>
-        div.to-screenshot {
+          div.to-screenshot {
-          border: 1px solid blue;
+            border: 1px solid blue;
-          width: 600px;
+            width: 600px;
-          height: 600px;
+            height: 600px;
-          margin-left: 50px;
+            margin-left: 50px;
-        }
+          }
-        ::-webkit-scrollbar{
+          ::-webkit-scrollbar{
-          display: none;
+            display: none;
-        }
+          }
-        </style>
+          </style>
-        <div class="to-screenshot"></div>
+          <div class="to-screenshot"></div>
-      `);
+        `);
-      const elementHandle = await page.$('div.to-screenshot');
+        const elementHandle = await page.$('div.to-screenshot');
-      const screenshot = await elementHandle.screenshot();
+        const screenshot = await elementHandle.screenshot();
-      expect(screenshot).toBeGolden(
+        expect(screenshot).toBeGolden(
-        'screenshot-element-larger-than-viewport.png'
+          'screenshot-element-larger-than-viewport.png'
-      );
+        );
 
-      expect(
+        expect(
-        await page.evaluate(() => ({
+          await page.evaluate(() => ({
-          w: window.innerWidth,
+            w: window.innerWidth,
-          h: window.innerHeight,
+            h: window.innerHeight,
-        }))
+          }))
-      ).toEqual({ w: 500, h: 500 });
+        ).toEqual({ w: 500, h: 500 });
-    });
+      }
+    );
     it('should scroll element into view', async () => {
       const { page } = getTestState();