Merge pull request DevExpress#193 from inikulin/gh192

Implement correct CLI coloring and logging. Get rid of `--report-path` option, since report can now be saved via piping (fixes DevExpress#192)

src/cli/argument-parser.js

@@ -1,6 +1,4 @@
-import chalk from 'chalk';
-import { resolve, dirname } from 'path';
-import { createWriteStream } from 'fs';
+import { resolve } from 'path';
 import { Command } from 'commander';
 import { Promise } from 'es6-promise';
 import promisify from 'es6-promisify';
@@ -40,7 +38,6 @@ export default class CliArgumentParser {
         this.browsers        = null;
         this.filter          = null;
         this.remoteCount     = 0;
-        this.reportOutStream = null;
         this.opts            = null;
 
         this._describeProgram();
@@ -91,7 +88,6 @@ export default class CliArgumentParser {
 
             .option('-b, --list-browsers', 'output the available browser aliases')
             .option('-r, --reporter <name>', 'specify the reporter type to use')
-            .option('-p, --report-path <path>', 'send the report to a file at <path>')
             .option('-s, --screenshots <path>', 'enable screenshot capturing and specify the path to save the screenshots to')
             .option('-S, --screenshots-on-fails', 'take a screenshot whenever a test fails')
             .option('-q, --quarantine-mode', 'enable the quarantine mode')
@@ -103,6 +99,9 @@ export default class CliArgumentParser {
             .option('--ports <port1,port2>', 'specify custom port numbers')
             .option('--hostname <name>', 'specify the hostname')
             .option('--qr-code', 'outputs QR-code that repeats URLs used to connect the remote browsers')
+
+            // NOTE: these options will be handled by chalk internally
+            .option('--color', 'force colors in command line')
             .option('--no-color', 'disable colors in command line');
     }
 
@@ -181,25 +180,11 @@ export default class CliArgumentParser {
         }
     }
 
-    async _parseReportPath () {
-        if (this.opts.reportPath) {
-            this.opts.reportPath = resolve(this.cwd, this.opts.reportPath);
-
-            var dir = dirname(this.opts.reportPath);
-
-            await ensureDir(dir);
-
-            this.reportOutStream = createWriteStream(this.opts.reportPath);
-        }
-    }
-
     async parse (argv) {
         this.program.parse(argv);
 
         this.opts = this.program.opts();
 
-        chalk.enabled = this.opts.color;
-
         // NOTE: the '-list-browsers' option only lists browsers and immediately exits the app.
         // Therefore, we don't need to process other arguments.
         if (!this.opts.listBrowsers) {
@@ -208,7 +193,6 @@ export default class CliArgumentParser {
             await Promise.all([
                 this._parsePorts(),
                 this._parseScreenshotsPath(),
-                this._parseReportPath(),
                 this._parseBrowserList(),
                 this._parseFileList()
             ]);

src/cli/index.js

@@ -1,23 +1,23 @@
 import chalk from 'chalk';
 import { getInstallations as getBrowserInstallations } from 'testcafe-browser-natives';
 import CliArgumentParser from './argument-parser';
-import spinner from './spinner';
+import log from './log';
 import remotesWizard from './remotes-wizard';
 import createTestCafe from '../';
 
 function exit (code) {
-    spinner.hide();
+    log.hideSpinner();
 
     // NOTE: give a process time to flush the output.
     // It's necessary in some environments.
     setTimeout(() => process.exit(code), 0);
 }
 
 function error (err) {
-    spinner.hide();
+    log.hideSpinner();
 
-    console.error(chalk.red('ERROR ') + err.message);
-    console.error(chalk.gray('Type "tescafe -h" for help.'));
+    log.write(chalk.red('ERROR ') + err.message);
+    log.write(chalk.gray('Type "tescafe -h" for help.'));
 
     exit(1);
 }
@@ -27,7 +27,7 @@ async function runTests (argParser) {
     var port1 = opts.ports && opts.ports[0];
     var port2 = opts.ports && opts.ports[1];
 
-    spinner.showBootstrapIndicator();
+    log.showSpinner();
 
     var testCafe       = await createTestCafe(opts.hostname, port1, port2);
     var remoteBrowsers = await remotesWizard(testCafe, argParser.remoteCount, opts.qrCode);
@@ -37,19 +37,11 @@ async function runTests (argParser) {
     runner
         .src(argParser.src)
         .browsers(browsers)
-        .reporter(opts.reporter, argParser.reportOutStream)
+        .reporter(opts.reporter)
         .filter(argParser.filter)
         .screenshots(opts.screenshots, opts.screenshotsOnFails);
 
-    runner.once('done-bootstrapping', () => {
-        spinner.hide();
-
-        if (opts.reportPath) {
-            console.log(`Report will be written to: ${chalk.underline.blue(opts.reportPath)}`);
-            console.log();
-            spinner.showRunIndicator();
-        }
-    });
+    runner.once('done-bootstrapping', () => log.hideSpinner());
 
     var failed = await runner.run({
         skipJsErrors:   opts.skipJsErrors,

src/cli/log.js

@@ -0,0 +1,39 @@
+import tty from 'tty';
+import elegantSpinner from 'elegant-spinner';
+import logUpdate from 'log-update';
+import chalk from 'chalk';
+
+// NOTE: To support piping, we use stderr as the log output
+// stream, while stdout is used for the report output.
+export default {
+    animation: null,
+    isTTY:     tty.isatty(1),
+
+    showSpinner () {
+        // NOTE: we can use the spinner only if stderr is a TTY,
+        // otherwise we can't repaint animation frames
+        if (this.isTTY) {
+            var spinnerFrame = elegantSpinner();
+
+            this.animation = setInterval(() => {
+                var frame = chalk.cyan(spinnerFrame());
+
+                logUpdate.stderr(frame);
+            }, 50);
+        }
+    },
+
+    hideSpinner () {
+        if (this.animation) {
+            clearInterval(this.animation);
+            logUpdate.stderr.clear();
+
+            this.animation = null;
+        }
+    },
+
+    write (text) {
+        console.error(text);
+    }
+};
+

src/cli/remotes-wizard.js

@@ -1,6 +1,6 @@
 import qrcode from 'qrcode-terminal';
 import chalk from 'chalk';
-import spinner from './spinner';
+import log from './log';
 import { Promise } from 'es6-promise';
 
 function waitBrowserConnectionReady (browserConnection) {
@@ -11,25 +11,25 @@ export default async function (testCafe, remoteCount, showQRCode) {
     var connections = [];
 
     if (remoteCount) {
-        spinner.hide();
+        log.hideSpinner();
 
         for (var i = 0; i < remoteCount; i++) {
             var browserConnection = testCafe.createBrowserConnection();
 
-            console.log(`To connect a remote browser #${i + 1}, use it to open ${chalk.underline.blue(browserConnection.url)}`);
+            log.write(`To connect a remote browser #${i + 1}, use it to open ${chalk.underline.blue(browserConnection.url)}`);
 
             if (showQRCode) {
-                console.log('or scan this QR-code:\n');
+                log.write('or scan this QR-code:\n');
                 qrcode.generate(browserConnection.url);
             }
 
             await waitBrowserConnectionReady(browserConnection);
 
             connections.push(browserConnection);
-            console.log(`${chalk.green('CONNECTED')} ${browserConnection.userAgent}\n`);
+            log.write(`${chalk.green('CONNECTED')} ${browserConnection.userAgent}\n`);
         }
 
-        spinner.showBootstrapIndicator();
+        log.showSpinner();
     }
 
     return connections;

src/reporters/base.js

@@ -12,14 +12,11 @@ export default class BaseReporter {
     constructor (task, outStream, errorDecorator) {
         this.outStream = outStream || process.stdout;
 
-        var disableColors = this.outStream !== process.stdout;
+        var useColors = this.outStream === process.stdout && chalk.enabled;
 
-        this.errorDecorator = errorDecorator || (disableColors ? plainTextDecorator : coloredTextDecorator);
+        this.errorDecorator = errorDecorator || (useColors ? coloredTextDecorator : plainTextDecorator);
 
-        // NOTE: force colors disabling only if we don't write to the
-        // stdout. Otherwise use global instance of `chalk`, so it will
-        // work with respect to the `--no-color` flag.
-        this.chalk         = disableColors ? new chalk.constructor({ enabled: false }) : chalk;
+        this.chalk         = new chalk.constructor({ enabled: useColors });
         this.viewportWidth = getViewportWidth(this.outStream);
         this.useWordWrap   = false;
         this.indent        = 0;

test/server/cli-argument-parser-test.js

@@ -1,14 +1,11 @@
 var expect                  = require('chai').expect;
 var path                    = require('path');
 var fs                      = require('fs');
-var promisify               = require('es6-promisify');
 var tmp                     = require('tmp');
 var Promise                 = require('es6-promise').Promise;
 var getBrowserInstallations = require('testcafe-browser-natives').getInstallations;
 var CliArgumentParser       = require('../../lib/cli/argument-parser');
 
-var readFile = promisify(fs.readFile);
-
 describe('CLI argument parser', function () {
     tmp.setGracefulCleanup();
 
@@ -252,22 +249,6 @@ describe('CLI argument parser', function () {
             .catch(done);
     });
 
-    it('Should parse report path and provide file stream', function (done) {
-        var file = path.join(tmp.dirSync().name, 'my/reports/report1');
-
-        parse('-p ' + file)
-            .then(function (parser) {
-                parser.reportOutStream.end('42');
-
-                return readFile(file, 'utf8');
-            })
-            .then(function (content) {
-                expect(content).eql('42');
-                done();
-            })
-            .catch(done);
-    });
-
     it('Should parse command line arguments', function (done) {
         parse('-r list -S -q -e --hostname myhost --qr-code ie test/server/data/file-list/file-1.js')
             .then(function (parser) {
@@ -276,7 +257,6 @@ describe('CLI argument parser', function () {
                 expect(parser.opts.reporter).eql('list');
                 expect(parser.opts.hostname).eql('myhost');
                 expect(parser.opts.screenshots).to.be.undefined;
-                expect(parser.reportOutStream).to.be.null;
                 expect(parser.opts.screenshotsOnFails).to.be.ok;
                 expect(parser.opts.quarantineMode).to.be.ok;
                 expect(parser.opts.skipJsErrors).to.be.ok;