Merge pull request #357 from step2yeung/completebrowsers

Improve complete browser book keeping & improve request next module conditions
ember-cli · Sep 11, 2019 · 662719a · 662719a
2 parents 2121b28 + e51b0f4
commit 662719a
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 12 deletions.
diff --git a/addon-test-support/-private/ember-exam-qunit-test-loader.js b/addon-test-support/-private/ember-exam-qunit-test-loader.js
@@ -133,6 +133,12 @@ export default class EmberExamQUnitTestLoader extends TestLoader {
     });
 
     const nextModuleHandler = () => {
+      // if there are already tests queued up, don't request next module
+      // this is possible if a test file has multiple qunit modules
+      if (this._qunit.config.queue.length > 0) {
+        return;
+      }
+
       return nextModuleAsyncIterator
         .next()
         .then((response) => {

diff --git a/lib/commands/exam.js b/lib/commands/exam.js
@@ -268,25 +268,26 @@ module.exports = TestCommand.extend({
     const ui = this.ui;
     const events = config.custom_browser_socket_events || {};
     const testExecutionFileName = `test-execution-${Date.now()}.json`;
+    const browserCount = Object.keys(config.testPage).length;
     const browserExitHandler = function() {
-      const browserCount = Object.keys(config.testPage).length;
+      const browserId = getBrowserId(this.launcher.settings.test_page);
+
       testemEvents.completedBrowsersHandler(
         browserCount,
+        browserId,
         ui,
         commands.get('loadBalance'),
         testExecutionFileName,
         commands.get('writeExecutionFile')
       );
     };
     const browserFailureHandler = function() {
-      if (this.finished) {
-        return;
-      } else if (commands.get('writeExecutionFile')) {
+      if (commands.get('writeExecutionFile')) {
         const browserId = getBrowserId(this.launcher.settings.test_page);
         testemEvents.recordFailedBrowserId(browserId);
       }
 
-      browserExitHandler();
+      browserExitHandler.call(this);
     };
 
     let init = false;

diff --git a/lib/utils/execution-state-manager.js b/lib/utils/execution-state-manager.js
@@ -15,7 +15,7 @@ class ExecutionStateManager {
 
     // An array keeping the browserId of a browser with failing test
     this._failedBrowsers = [];
-    this._completedBrowsers = 0;
+    this._completedBrowsers = new Map();
 
     // An array of modules to load balance against browsers. This is used by `--load-balance`
     this._testModuleQueue = null;
@@ -171,14 +171,16 @@ class ExecutionStateManager {
    * @returns {number}
    */
   getCompletedBrowser() {
-    return this._completedBrowsers;
+    return this._completedBrowsers.size;
   }
 
   /**
-   * Increment the number of completed browsers
+   * Book keep the browser id that has completed
+   *
+   * @param {number} browserId
    */
-  incrementCompletedBrowsers() {
-    this._completedBrowsers = this._completedBrowsers + 1;
+  incrementCompletedBrowsers(browserId) {
+    this._completedBrowsers.set(browserId, true);
   }
 }
 

diff --git a/lib/utils/testem-events.js b/lib/utils/testem-events.js
@@ -157,15 +157,16 @@ class TestemEvents {
    * When all browsers complete, write test-execution.json to disk and clean up the stateManager
    *
    * @param {number} browserCount
+   * @param {number} browserid
    * @param {Object} ui
    * @param {boolean} loadBalance
    * @param {string} fileName
    * @param {string} writeExecutionFile
    */
-  completedBrowsersHandler(browserCount, ui, loadBalance, fileName, writeExecutionFile) {
+  completedBrowsersHandler(browserCount, browserId, ui, loadBalance, fileName, writeExecutionFile) {
     let browsersCompleted = false;
 
-    this.stateManager.incrementCompletedBrowsers();
+    this.stateManager.incrementCompletedBrowsers(browserId);
     if (this.stateManager.getCompletedBrowser() === browserCount) {
       if (writeExecutionFile && loadBalance) {
         const moduleMapJson = this._generatesModuleMapJsonObject(browserCount);

diff --git a/node-tests/unit/utils/execution-state-manager-test.js b/node-tests/unit/utils/execution-state-manager-test.js
@@ -89,4 +89,13 @@ describe('ExecutionStateManager', function() {
       );
     });
   });
+
+  describe('completedBrowsers', function() {
+    it('incrementCompletedBrowsers called for the same browserId will only be accounted once', function() {
+      this.stateManager.incrementCompletedBrowsers(1);
+      this.stateManager.incrementCompletedBrowsers(1);
+
+      assert.deepEqual(this.stateManager.getCompletedBrowser(), 1);
+    });
+  });
 });
diff --git a/node-tests/unit/utils/testem-events-test.js b/node-tests/unit/utils/testem-events-test.js
@@ -180,6 +180,7 @@ describe('TestemEvents', function() {
     it('should increment completedBrowsers only when completedBrowsers is less than browserCount', function() {
       this.testemEvents.completedBrowsersHandler(
         2,
+        1,
         mockUi,
         true,
         'test-execution.json',
@@ -196,6 +197,7 @@ describe('TestemEvents', function() {
     it('should write test-execution file and cleanup state when completedBrowsers equals browserCount and load-balance is true', function() {
       this.testemEvents.stateManager.addModuleNameToReplayExecutionMap('a', 1);
       this.testemEvents.completedBrowsersHandler(
+        1,
         1,
         mockUi,
         true,
@@ -219,6 +221,7 @@ describe('TestemEvents', function() {
     it('should increment completedBrowsers when load-balance is false', function() {
       this.testemEvents.completedBrowsersHandler(
         2,
+        1,
         mockUi,
         false,
         'test-execution.json',
@@ -238,6 +241,7 @@ describe('TestemEvents', function() {
 
       this.testemEvents.completedBrowsersHandler(
         2,
+        1,
         mockUi,
         true,
         'test-execution.json',
@@ -254,6 +258,7 @@ describe('TestemEvents', function() {
         mockReplayExecutionMap
       );
       this.testemEvents.completedBrowsersHandler(
+        1,
         1,
         mockUi,
         true,