refactor(logger): add a logger to handle all printing and writing to …

…stdout
angular · Dec 3, 2014 · 7f15570 · 7f15570
1 parent 90ed11c
commit 7f15570
Show file tree

Hide file tree

Showing 16 changed files with 77 additions and 29 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -23,7 +23,7 @@ Before submitting an issue, please search the issue archive to help reduce dupli
 [FAQ](https://github.com/angular/protractor/blob/master/docs/faq.md).
 
 When submitting an issue, please include context from your test and
-your application. If there's an error, please include the error text.
+your application. If there's an error, please include the error text. Try running with troubleshooting messages (`protractor --troubleshoot`) against your configuration to make sure that there is not an error with your setup.
 
 Please format code and markup in your issue using [github markdown](https://help.github.com/articles/github-flavored-markdown).
 

diff --git a/lib/cli.js b/lib/cli.js
@@ -28,7 +28,6 @@ process.argv.slice(2).forEach(function(arg) {
   }
 });
 
-var util = require('util');
 var path = require('path');
 var fs = require('fs');
 var optimist = require('optimist').
@@ -51,6 +50,7 @@ var optimist = require('optimist').
     describe('params', 'Param object to be passed to the tests').
     describe('framework', 'Test framework to use: jasmine, cucumber or mocha').
     describe('resultJsonOutputFile', 'Path to save JSON test result').
+    describe('troubleshoot', 'Turn on troubleshooting output').
     alias('browser', 'capabilities.browserName').
     alias('name', 'capabilities.name').
     alias('platform', 'capabilities.platform').
@@ -69,7 +69,7 @@ var optimist = require('optimist').
 var argv = optimist.parse(args);
 
 if (argv.version) {
-  util.puts('Version ' + require(path.join(__dirname, '../package.json')).version);
+  console.log('Version ' + require(path.join(__dirname, '../package.json')).version);
   process.exit(0);
 }
 

diff --git a/lib/configParser.js b/lib/configParser.js
@@ -1,6 +1,6 @@
 var path = require('path'),
     glob = require('glob'),
-    util = require('util'),
+    log = require('./logger'),
     _ = require('lodash'),
     helper = require('./util');
 
@@ -98,7 +98,7 @@ ConfigParser.resolveFilePatterns =
     for (var i = 0; i < patterns.length; ++i) {
       var matches = glob.sync(patterns[i], {cwd: cwd});
       if (!matches.length && !opt_omitWarnings) {
-        util.puts('Warning: pattern ' + patterns[i] + ' did not match any files.');
+        log.puts('Warning: pattern ' + patterns[i] + ' did not match any files.');
       }
       for (var j = 0; j < matches.length; ++j) {
         resolvedFiles.push(path.resolve(cwd, matches[j]));
@@ -164,7 +164,7 @@ ConfigParser.prototype.addConfig_ = function(additionalConfig, relativeTo) {
 
   // chromeOnly is deprecated, use directConnect instead.
   if (additionalConfig.chromeOnly) {
-    console.log('Warning: chromeOnly is deprecated. Use directConnect');
+    log.puts('Warning: chromeOnly is deprecated. Use directConnect');
     additionalConfig.directConnect = true;
   }
   merge_(this.config_, additionalConfig);

diff --git a/lib/driverProviders/direct.js b/lib/driverProviders/direct.js
@@ -11,7 +11,8 @@ var webdriver = require('selenium-webdriver'),
     fs = require('fs'),
     path = require('path'),
     util = require('util'),
-    DriverProvider = require('./driverProvider');
+    DriverProvider = require('./driverProvider'),
+    log = require('../logger');
 
 var DirectDriverProvider = function(config) {
   DriverProvider.call(this, config);
@@ -27,10 +28,10 @@ util.inherits(DirectDriverProvider, DriverProvider);
 DirectDriverProvider.prototype.setupEnv = function() {
   switch (this.config_.capabilities.browserName) {
     case 'chrome':
-      console.log('Using ChromeDriver directly...');
+      log.puts('Using ChromeDriver directly...');
       break;
     case 'firefox':
-      console.log('Using FirefoxDriver directly...');
+      log.puts('Using FirefoxDriver directly...');
       break;
     default:
       throw new Error('browserName (' + this.config_.capabilities.browserName +

diff --git a/lib/driverProviders/hosted.js b/lib/driverProviders/hosted.js
@@ -6,7 +6,8 @@
 
 var util = require('util'),
     q = require('q'),
-    DriverProvider = require('./driverProvider');
+    DriverProvider = require('./driverProvider'),
+    log = require('../logger');
 
 var HostedDriverProvider = function(config) {
   DriverProvider.call(this, config);
@@ -26,10 +27,10 @@ HostedDriverProvider.prototype.setupEnv = function() {
   if (q.isPromiseAlike(seleniumAddress)) {
     return q.when(seleniumAddress).then(function(resolvedAddress) {
       config.seleniumAddress = resolvedAddress;
-      util.puts('Using the selenium server at ' + config.seleniumAddress);
+      log.puts('Using the selenium server at ' + config.seleniumAddress);
     });
   } else {
-    util.puts('Using the selenium server at ' + this.config_.seleniumAddress);
+    log.puts('Using the selenium server at ' + this.config_.seleniumAddress);
     return q.fcall(function() {});
   }
 };

diff --git a/lib/driverProviders/local.js b/lib/driverProviders/local.js
@@ -7,6 +7,7 @@
  * so that we only start the local selenium once per entire launch.
  */
 var util = require('util'),
+    log = require('../logger.js'),
     path = require('path'),
     remote = require('selenium-webdriver/remote'),
     fs = require('fs'),
@@ -62,7 +63,7 @@ LocalDriverProvider.prototype.setupEnv = function() {
       'location. Do you have the correct version?');
   }
 
-  util.puts('Starting selenium standalone server...');
+  log.puts('Starting selenium standalone server...');
 
   // configure server
   if (this.config_.chromeDriver) {
@@ -76,7 +77,7 @@ LocalDriverProvider.prototype.setupEnv = function() {
 
   //start local server, grab hosted address, and resolve promise
   this.server_.start().then(function(url) {
-    util.puts('Selenium standalone server started at ' + url);
+    log.puts('Selenium standalone server started at ' + url);
     self.server_.address().then(function(address) {
       self.config_.seleniumAddress = address;
       deferred.resolve();
@@ -99,7 +100,7 @@ LocalDriverProvider.prototype.teardownEnv = function() {
   var self = this;
   var deferred = q.defer();
   DriverProvider.prototype.teardownEnv.call(this).then(function() {
-    util.puts('Shutting down selenium standalone server.');
+    log.puts('Shutting down selenium standalone server.');
     self.server_.stop().then(function() {
       deferred.resolve();
     });

diff --git a/lib/driverProviders/sauce.js b/lib/driverProviders/sauce.js
@@ -5,6 +5,7 @@
  */
 
 var util = require('util'),
+    log = require('../logger.js'),
     SauceLabs = require('saucelabs'),
     q = require('q'),
     DriverProvider = require('./driverProvider');
@@ -29,7 +30,7 @@ SauceDriverProvider.prototype.updateJob = function(update) {
   var deferredArray = this.drivers_.map(function(driver) {
     var deferred = q.defer();
     driver.getSession().then(function(session) {
-      console.log('SauceLabs results available at http://saucelabs.com/jobs/' +
+      log.puts('SauceLabs results available at http://saucelabs.com/jobs/' +
           session.getId());
       self.sauceServer_.updateJob(session.getId(), update, function(err) {
         if (err) {
@@ -72,7 +73,7 @@ SauceDriverProvider.prototype.setupEnv = function() {
         ':' + this.config_.specs.toString().replace(/^.*[\\\/]/, ''));
   }
 
-  util.puts('Using SauceLabs selenium server at ' +
+  log.puts('Using SauceLabs selenium server at ' +
       this.config_.seleniumAddress.replace(/\/\/.+@/, '//'));
   deferred.resolve();
   return deferred.promise;

diff --git a/lib/frameworks/debugprint.js b/lib/frameworks/debugprint.js
@@ -1,5 +1,6 @@
 var util = require('util'),
-  q = require('q');
+    q = require('q'),
+    log = require('../logger');
 
 /**
  * A debug framework which does not actually run any tests, just spits
@@ -11,7 +12,7 @@ var util = require('util'),
  */
 exports.run = function(runner, specs) {
   return q.promise(function (resolve) {
-    console.log('Resolved spec files: ' + util.inspect(specs));
+    log.puts('Resolved spec files: ' + util.inspect(specs));
     resolve({
       failedCount: 0
     });

diff --git a/lib/launcher.js b/lib/launcher.js
@@ -7,6 +7,7 @@
 var ConfigParser = require('./configParser'),
     TaskScheduler = require('./taskScheduler'),
     helper = require('./util'),
+    log = require('./logger'),
     q = require('q'),
     TaskRunner = require('./taskRunner');
 
@@ -20,7 +21,7 @@ var RUNNERS_FAILED_EXIT_CODE = 100;
  */
 var log_ = function() {
   var args = [logPrefix].concat([].slice.call(arguments));
-  console.log.apply(console, args);
+  log.puts.apply(log, args);
 };
 
 /**
@@ -109,6 +110,7 @@ var init = function(configFile, additionalConfig) {
     configParser.addConfig(additionalConfig);
   }
   var config = configParser.getConfig();
+  log.set(config);
   var scheduler = new TaskScheduler(config);
 
   process.on('exit', function(code) {

diff --git a/lib/logger.js b/lib/logger.js
@@ -0,0 +1,33 @@
+/**
+ * Utility functions for command line output logging from Protractor.
+ * May be used in different processes, since the launcher spawns
+ * child processes for each runner.
+ *
+ * All logging directly from Protractor, its driver providers, or runners,
+ * should go through this file so that it can be customized.
+ */
+
+var troubleshoot = false;
+
+var set = function(config) {
+  troubleshoot = config.troubleshoot;
+};
+
+var print = function(msg) {
+  process.stdout.write(msg);
+};
+
+var puts = function() {
+  console.log.apply(console, arguments);
+};
+
+var debug = function(msg) {
+  if (troubleshoot) {
+    puts(msg);
+  }
+};
+
+exports.set = set;
+exports.print = print;
+exports.puts = puts;
+exports.debug = debug;
diff --git a/lib/protractor.js b/lib/protractor.js
@@ -2,6 +2,7 @@ var url = require('url');
 var util = require('util');
 var webdriver = require('selenium-webdriver');
 var helper = require('./util');
+var log = require('./logger.js');
 
 var clientSideScripts = require('./clientsidescripts.js');
 var ProtractorBy = require('./locators.js').ProtractorBy;
@@ -702,7 +703,7 @@ var buildElementHelper = function(ptor) {
                   elementArrayFinder.locator_.toString());
         } else {
           if (webElements.length > 1) {
-            console.log('warning: more than one element found for locator ' +
+            log.puts('warning: more than one element found for locator ' +
                 elementArrayFinder.locator_.toString() +
                 ' - you may need to be more specific');
           }
@@ -1518,7 +1519,7 @@ Protractor.prototype.pause = function(opt_debugPort) {
   var flow = webdriver.promise.controlFlow();
 
   flow.execute(function() {
-    console.log('Starting WebDriver debugger in a child process. Pause is ' +
+    log.puts('Starting WebDriver debugger in a child process. Pause is ' +
         'still beta, please report issues at github.com/angular/protractor');
     var nodedebug = require('child_process').
         fork(__dirname + '/wddebugger.js', [process.debugPort]);

diff --git a/lib/runner.js b/lib/runner.js
@@ -4,6 +4,7 @@ var protractor = require('./protractor'),
     q = require('q'),
     EventEmitter = require('events').EventEmitter,
     helper = require('./util'),
+    log = require('./logger'),
     Plugins = require('./plugins');
 
 /*
@@ -19,6 +20,7 @@ var protractor = require('./protractor'),
  * @constructor
  */
 var Runner = function(config) {
+  log.set(config);
   this.preparer_ = null;
   this.driverprovider_ = null;
   this.config_ = config;

diff --git a/lib/runnerCli.js b/lib/runnerCli.js
@@ -5,6 +5,7 @@
 
 var ConfigParser = require('./configParser');
 var Runner = require('./runner');
+var log = require('./logger');
 
 process.on('message', function(m) {
   switch (m.command) {
@@ -21,6 +22,7 @@ process.on('message', function(m) {
         configParser.addConfig(m.additionalConfig);
       }
       var config = configParser.getConfig();
+      log.set(config);
 
       // Grab capabilities to run from launcher.
       config.capabilities = m.capabilities;
@@ -52,7 +54,7 @@ process.on('message', function(m) {
       runner.run().then(function(exitCode) {
         process.exit(exitCode);
       }).catch(function(err) {
-        console.log(err.message);
+        log.puts(err.message);
         process.exit(1);
       });
       break;

diff --git a/lib/taskLogger.js b/lib/taskLogger.js
@@ -1,4 +1,5 @@
 var EOL = require('os').EOL;
+var log = require('./logger.js');
 
 /**
  * Log output such that metadata are appended.
@@ -38,9 +39,9 @@ TaskLogger.prototype.logHeader_ = function() {
 TaskLogger.prototype.flush = function() {
   if (this.buffer) {
     // Flush buffer if nonempty
-    process.stdout.write(EOL + '------------------------------------' + EOL);
-    process.stdout.write(this.buffer);
-    process.stdout.write(EOL);
+    log.print(EOL + '------------------------------------' + EOL);
+    log.print(this.buffer);
+    log.print(EOL);
     this.buffer = '';
   }
 };

diff --git a/lib/taskRunner.js b/lib/taskRunner.js
@@ -3,6 +3,7 @@ var q = require('q');
 var TaskLogger = require('./taskLogger.js');
 var EventEmitter = require('events').EventEmitter;
 var util = require('util');
+var log = require('./logger.js');
 
 
 /**
@@ -68,10 +69,10 @@ TaskRunner.prototype.run = function() {
     childProcess.on('message', function(m) {
       switch (m.event) {
         case 'testPass':
-          process.stdout.write('.');
+          log.print('.');
           break;
         case 'testFail':
-          process.stdout.write('F');
+          log.print('F');
           break;
         case 'testsDone':
           runResults.failedCount = m.results.failedCount;

diff --git a/lib/taskScheduler.js b/lib/taskScheduler.js
@@ -5,6 +5,7 @@
 'use strict';
 
 var ConfigParser = require('./configParser');
+var log = require('./logger.js');
 
 // A queue of specs for a particular capacity
 var TaskQueue = function(capabilities, specLists) {
@@ -33,7 +34,7 @@ var TaskScheduler = function(config) {
 
   if (config.capabilities) {
       if (config.multiCapabilities.length) {
-          console.log('Running using config.multiCapabilities - ' +
+          log.puts('Running using config.multiCapabilities - ' +
             'config.capabilities will be ignored');
       } else {
           // Use capabilities if multiCapabilities is empty.