feat: don't wait for all browsers and start executing immediately

vojtajina · vojtajina · commit 8647266fd592 · 2013-08-25T16:12:19.000-07:00
Once a browser gets captured, Karma immediately starts test execution in that browser. Also, immediately after a browser is finished, Karma kills this browser. This should allow us to use SauceLabs/BrowserStack browsers much more efficiently. This only affects `--single-run` mode. Without `--single-run` mode, Karma still waits for all browsers to get captured and then execute. Closes #57
diff --git a/lib/executor.js b/lib/executor.js
@@ -24,6 +24,11 @@ var Executor = function(capturedBrowsers, config, emitter) {
       runningBrowsers = capturedBrowsers.clone();
       emitter.emit('run_start', runningBrowsers);
       self.socketIoSockets.emit('execute', config.client);
+
+      // TODO(vojta): browser should do this, once it has specs total count, etc.
+      capturedBrowsers.forEach(function(browser) {
+        emitter.emit('browser_start', browser);
+      });
       return true;
     }
 
diff --git a/lib/launcher.js b/lib/launcher.js
@@ -38,12 +38,27 @@ var Launcher = function(emitter, injector) {
       browser.start(url);
       browsers.push(browser);
     });
+
+    return browsers;
   };
 
   this.launch.$inject = ['config.browsers', 'config.hostname', 'config.port', 'config.urlRoot'];
 
 
-  this.kill = function(callback) {
+  this.kill = function(id, callback) {
+    for (var i = 0; i < browsers.length; i++) {
+      if (browsers[i].id === id) {
+        browsers[i].kill(callback);
+        return true;
+      }
+    }
+
+    process.nextTick(callback);
+    return false;
+  };
+
+
+  this.killAll = function(callback) {
     log.debug('Disconnecting all browsers');
 
     var remaining = 0;
@@ -82,7 +97,7 @@ var Launcher = function(emitter, injector) {
 
 
   // register events
-  emitter.on('exit', this.kill);
+  emitter.on('exit', this.killAll);
 };
 
 Launcher.$inject = ['emitter', 'injector'];
diff --git a/lib/reporters/Base.js b/lib/reporters/Base.js
@@ -6,10 +6,13 @@ var helper = require('../helper');
 var BaseReporter = function(formatError, reportSlow, adapter) {
   this.adapters = [adapter || process.stdout.write.bind(process.stdout)];
 
-  this.onRunStart = function(browsers) {
-    this._browsers = browsers;
+  this.onRunStart = function() {
+    this._browsers = [];
   };
 
+  this.onBrowserStart = function(browser) {
+    this._browsers.push(browser);
+  };
 
   this.renderBrowser = function(browser) {
     var results = browser.lastResult;
diff --git a/lib/reporters/Dots.js b/lib/reporters/Dots.js
@@ -6,11 +6,15 @@ var DotsReporter = function(formatError, reportSlow) {
 
   var DOTS_WRAP = 80;
 
-  this.onRunStart = function(browsers) {
-    this._browsers = browsers;
+  this.onRunStart = function() {
+    this._browsers = [];
     this._dotsCount = 0;
   };
 
+  this.onBrowserStart = function(browser) {
+    this._browsers.push(browser);
+  };
+
   this.writeCommonMsg = function(msg) {
     if (this._dotsCount) {
       this._dotsCount = 0;
diff --git a/lib/reporters/Progress.js b/lib/reporters/Progress.js
@@ -19,12 +19,21 @@ var ProgressReporter = function(formatError, reportSlow) {
     this.write(this._refresh());
   };
 
-
-  this.onRunStart = function(browsers) {
-    this._browsers = browsers;
+  this.onRunStart = function() {
+    this._browsers = [];
     this._isRendered = false;
   };
 
+  this.onBrowserStart = function(browser) {
+    this._browsers.push(browser);
+
+    if (this._isRendered) {
+      this.write('\n');
+    }
+
+    this.write(this._refresh());
+  };
+
 
   this._remove = function() {
     if (!this._isRendered) {
diff --git a/lib/server.js b/lib/server.js
@@ -46,12 +46,24 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
     }
   });
 
+  // A map of launched browsers (key is the launchId).
+  var singleRunDoneBrowsers = Object.create(null);
+
+  // Passing fake event emitter, so that it does not emit on the global,
+  // we don't care about these changes.
+  var singleRunBrowsers = new browser.Collection(new EventEmitter());
+
+  // Some browsers did not get captured.
+  var singleRunBrowserNotCaptured = false;
+
   webServer.listen(config.port, function() {
     log.info('Karma v%s server started at https://%s:%s%s', constant.VERSION, config.hostname,
         config.port, config.urlRoot);
 
     if (config.browsers && config.browsers.length) {
-      injector.invoke(launcher.launch, launcher);
+      injector.invoke(launcher.launch, launcher).forEach(function(browserLauncher) {
+        singleRunDoneBrowsers[browserLauncher.id] = false;
+      });
     }
   });
 
@@ -68,17 +80,11 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
 
     // TODO(vojta): This is lame, browser can get captured and then crash (before other browsers get
     // captured).
-    if ((config.autoWatch || config.singleRun) && launcher.areAllCaptured()) {
+    if (config.autoWatch && launcher.areAllCaptured()) {
       executor.schedule();
     }
   });
 
-  globalEmitter.on('run_complete', function(browsers, results) {
-    if (config.singleRun) {
-      disconnectBrowsers(results.exitCode);
-    }
-  });
-
   socketServer.sockets.on('connection', function (socket) {
     log.debug('A browser has connected on socket ' + socket.id);
 
@@ -104,9 +110,66 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
       }
 
       replySocketEvents();
+
+      // execute in this browser immediately
+      if (config.singleRun) {
+        // emitter.emit('run_start', runningBrowsers);
+        socket.emit('execute', config.client);
+
+        // TODO(vojta): encapsulate this into browser
+        newBrowser.state = browser.Browser.STATE_EXECUTING;
+
+        // TODO(vojta): emit this once we have info from the browser (total, specNames, etc.)
+        globalEmitter.emit('browser_start', newBrowser);
+
+        singleRunBrowsers.add(newBrowser);
+      }
     });
   });
 
+  var emitRunCompleteIfAllBrowsersDone = function() {
+    // all browsers done
+    var isDone = Object.keys(singleRunDoneBrowsers).reduce(function(isDone, id) {
+      return isDone && singleRunDoneBrowsers[id];
+    }, true);
+
+    if (isDone) {
+      var results = singleRunBrowsers.getResults();
+      if (singleRunBrowserNotCaptured) {
+        results.exitCode = 1;
+      }
+
+      globalEmitter.emit('run_complete', singleRunBrowsers, results);
+    }
+  }
+
+  if (config.singleRun) {
+    globalEmitter.on('browser_complete', function(completedBrowser, result) {
+      singleRunDoneBrowsers[completedBrowser.launchId] = true;
+
+      if (launcher.kill(completedBrowser.launchId)) {
+        // workaround to supress "disconnect" warning
+        completedBrowser.state = browser.Browser.STATE_DISCONNECTED;
+      }
+
+      emitRunCompleteIfAllBrowsersDone();
+    });
+
+    globalEmitter.on('browser_process_failure', function(browserLauncher) {
+      singleRunDoneBrowsers[browserLauncher.id] = true;
+      singleRunBrowserNotCaptured = true;
+
+      emitRunCompleteIfAllBrowsersDone();
+    });
+
+    globalEmitter.on('run_complete', function(browsers, results) {
+      disconnectBrowsers(results.exitCode);
+    });
+
+    globalEmitter.emit('run_start', singleRunBrowsers);
+  }
+
+
   if (config.autoWatch) {
     globalEmitter.on('file_list_modified', function() {
       log.debug('List of files has changed, trying to execute');
@@ -129,13 +192,6 @@ var start = function(injector, config, launcher, globalEmitter, preprocess, file
   };
 
 
-  if (config.singleRun) {
-    globalEmitter.on('browser_process_failure', function(browser) {
-      log.debug('%s failed to capture, aborting the run.', browser);
-      disconnectBrowsers(1);
-    });
-  }
-
   try {
     process.on('SIGINT', disconnectBrowsers);
     process.on('SIGTERM', disconnectBrowsers);
diff --git a/test/unit/launcher.spec.coffee b/test/unit/launcher.spec.coffee
@@ -76,14 +76,36 @@ describe 'launcher', ->
 
 
     describe 'kill', ->
+      it 'should kill browser with given id', ->
+        killSpy = sinon.spy()
+
+        l.launch ['Fake']
+        browser = FakeBrowser._instances.pop()
+
+        l.kill browser.id, killSpy
+        expect(browser.kill).to.have.been.called
+
+        browser.kill.invokeCallback()
+        expect(killSpy).to.have.been.called
+
+
+      it 'should return false if browser does not exist, but still resolve the callback', (done) ->
+        l.launch ['Fake']
+        browser = FakeBrowser._instances.pop()
+
+        expect(l.kill 'weid-id', done).to.equal false
+        expect(browser.kill).not.to.have.been.called
+
+
+    describe 'killAll', ->
       exitSpy = null
 
       beforeEach ->
         exitSpy = sinon.spy()
 
       it 'should kill all running processe', ->
         l.launch ['Fake', 'Fake'], 'localhost', 1234
-        l.kill()
+        l.killAll()
 
         browser = FakeBrowser._instances.pop()
         expect(browser.kill).to.have.been.called
@@ -94,7 +116,7 @@ describe 'launcher', ->
 
       it 'should call callback when all processes killed', ->
         l.launch ['Fake', 'Fake'], 'localhost', 1234
-        l.kill exitSpy
+        l.killAll exitSpy
 
         expect(exitSpy).not.to.have.been.called
 
@@ -111,7 +133,7 @@ describe 'launcher', ->
 
 
       it 'should call callback even if no browsers lanunched', (done) ->
-        l.kill done
+        l.killAll done
 
 
     describe 'areAllCaptured', ->
