Merge branch 'master' into express-close-callback

alxndrsn · Dec 31, 2023 · d2389d5 · d2389d5
2 parents a590d4b + a6b47e2
commit d2389d5
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 13 deletions.
diff --git a/bin/test-browser.js b/bin/test-browser.js
@@ -6,12 +6,16 @@ const playwright = require('playwright');
 const { identity, pickBy } = require('lodash');
 
 var MochaSpecReporter = require('mocha').reporters.Spec;
+const createMochaStatsCollector = require('mocha/lib/stats-collector');
 
 var devserver = require('./dev-server.js');
 
 // BAIL=0 to disable bailing
 var bail = process.env.BAIL !== '0';
 
+// Track if the browser has closed at the request of this script, or due to an external event.
+let closeRequested;
+
 // Playwright BrowserType whitelist.
 // See: https://playwright.dev/docs/api/class-playwright
 const SUPPORTED_BROWSERS = [ 'chromium', 'firefox', 'webkit' ];
@@ -56,20 +60,30 @@ const qs = {
 testUrl += '?';
 testUrl += new URLSearchParams(pickBy(qs, identity));
 
+class ArrayMap extends Map {
+  get(key) {
+    if (!this.has(key)) {
+      this.set(key, []);
+    }
+    return super.get(key);
+  }
+}
+
 class RemoteRunner {
   constructor(browser) {
     this.browser = browser;
-    this.handlers = {};
+    this.handlers = new ArrayMap();
+    this.onceHandlers = new ArrayMap();
     this.handleEvent = this.handleEvent.bind(this);
+    createMochaStatsCollector(this);
   }
 
-  on(name, handler) {
-    var handlers = this.handlers;
+  once(name, handler) {
+    this.onceHandlers.get(name).push(handler);
+  }
 
-    if (!handlers[name]) {
-      handlers[name] = [];
-    }
-    handlers[name].push(handler);
+  on(name, handler) {
+    this.handlers.get(name).push(handler);
   }
 
   async handleEvent(event) {
@@ -80,9 +94,12 @@ class RemoteRunner {
       };
       var obj = Object.assign({}, event.obj, additionalProps);
 
-      this.handlers[event.name].forEach(function (handler) {
-        handler(obj, event.err);
-      });
+      const triggerHandler = handler => handler(obj, event.err);
+
+      this.onceHandlers.get(event.name).forEach(triggerHandler);
+      this.onceHandlers.delete(event.name);
+
+      this.handlers.get(event.name).forEach(triggerHandler);
 
       switch (event.name) {
         case 'fail': this.handleFailed(); break;
@@ -91,12 +108,14 @@ class RemoteRunner {
     } catch (e) {
       console.error('Tests failed:', e);
 
+      closeRequested = true;
       await this.browser.close();
       process.exit(3);
     }
   }
 
   async handleEnd(failed) {
+    closeRequested = true;
     await this.browser.close();
     process.exit(!process.env.PERF && failed ? 1 : 0);
   }
@@ -147,8 +166,20 @@ async function startTest() {
     headless: true,
   };
   const browser = await playwright[browserName].launch(options);
+
   const page = await browser.newPage();
 
+  // Playwright's Browser.on('close') event handler would be the more obvious
+  // choice here, but it does not seem to be triggered if the browser is closed
+  // by an external event (e.g. process is killed, user closes non-headless
+  // browser window).
+  page.on('close', () => {
+    if (!closeRequested) {
+      console.log('!!! Browser closed by external event.');
+      process.exit(1);
+    }
+  });
+
   const runner = new RemoteRunner(browser);
   new MochaSpecReporter(runner);
   new BenchmarkConsoleReporter(runner);

diff --git a/bin/test-node.sh b/bin/test-node.sh
@@ -37,17 +37,21 @@ fi
 if [ $PERF ]; then
     node tests/performance/index.js
 elif [ ! $COVERAGE ]; then
+    # --exit required to workaround #8839
     ./node_modules/.bin/mocha \
+        --exit \
         $BAIL_OPT \
         --timeout "$TIMEOUT" \
         --require=./tests/integration/node.setup.js \
         --reporter="$REPORTER" \
         --grep="$GREP" \
         $TESTS_PATH
 else
+    # --exit required to workaround #8839
     ./node_modules/.bin/istanbul cover \
        --no-default-excludes -x 'tests/**' -x 'node_modules/**' \
        ./node_modules/mocha/bin/_mocha -- \
+        --exit \
         $BAIL_OPT \
         --timeout "$TIMEOUT" \
         --require=./tests/integration/node.setup.js \

diff --git a/package.json b/package.json
@@ -10,7 +10,7 @@
     "build-modules": "node bin/build-modules.js",
     "test-unit": "mocha tests/unit",
     "test-node": "./bin/test-node.sh",
-    "test-component": "mocha tests/component",
+    "test-component": "mocha --exit tests/component # --exit for #8839",
     "test-fuzzy": "TYPE=fuzzy npm run test",
     "test-browser": "npm run build-test && node ./bin/test-browser.js",
     "test-memleak": "mocha -gc tests/memleak",
@@ -41,7 +41,7 @@
     "abort-controller": "3.0.0",
     "clone-buffer": "1.0.0",
     "double-ended-queue": "2.1.0-0",
-    "fetch-cookie": "0.11.0",
+    "fetch-cookie": "2.1.0",
     "fruitdown": "1.0.2",
     "immediate": "3.3.0",
     "level": "6.0.1",
@@ -90,7 +90,7 @@
     "marky": "1.2.0",
     "median": "0.0.2",
     "mkdirp": "0.5.1",
-    "mocha": "3.5.0",
+    "mocha": "10.2.0",
     "mockery": "2.1.0",
     "ncp": "2.0.0",
     "playwright": "1.36.1",