Implement test run warnings (#671 part2) (#679)

* Implement warnings infrastructure. "Screenshots dir not specified" warning

* "Screenshots creation error" warning

* "Screenshots and resizing doesn't supported on Linux" warning

* Bump version

.gitignore

@@ -4,3 +4,4 @@ lib
 site
 .sass-cache
 .publish
+___test-screenshots___

package.json

@@ -1,7 +1,7 @@
 {
   "name": "testcafe",
   "license": "MIT",
-  "version": "0.0.25-alpha",
+  "version": "0.1.0-alpha",
   "main": "lib/index",
   "bin": {
     "testcafe": "./bin/testcafe.js"

src/errors/runtime/index.js

@@ -2,17 +2,12 @@ import { renderers } from 'callsite-record';
 import MESSAGE from './message';
 import createStackFilter from '../create-stack-filter';
 import getCallsite from '../get-callsite';
-
-
-// Utils
-function getText (template, ...args) {
-    return args.reduce((msg, arg) => msg.replace(/{.+?}/, arg), template);
-}
+import renderTemplate from '../../utils/render-template';
 
 // Errors
 export class GeneralError extends Error {
     constructor () {
-        super(getText.apply(null, arguments));
+        super(renderTemplate.apply(null, arguments));
         Error.captureStackTrace(this, GeneralError);
 
         // HACK: workaround for the `instanceof` problem
@@ -23,19 +18,19 @@ export class GeneralError extends Error {
 
 export class TestCompilationError extends Error {
     constructor (originalError) {
-        super(getText(MESSAGE.cannotPrepareTestsDueToError, originalError.toString()));
+        super(renderTemplate(MESSAGE.cannotPrepareTestsDueToError, originalError.toString()));
 
         // NOTE: stack includes message as well.
-        this.stack       = getText(MESSAGE.cannotPrepareTestsDueToError, originalError.stack);
+        this.stack       = renderTemplate(MESSAGE.cannotPrepareTestsDueToError, originalError.stack);
         this.constructor = TestCompilationError;
     }
 }
 
 export class APIError extends Error {
     constructor (methodName, template, ...args) {
-        var rawMessage = getText(template, ...args);
+        var rawMessage = renderTemplate(template, ...args);
 
-        super(getText(MESSAGE.cannotPrepareTestsDueToError, rawMessage));
+        super(renderTemplate(MESSAGE.cannotPrepareTestsDueToError, rawMessage));
 
         // NOTE: `rawMessage` is used in error substitution if it occurs in test run.
         this.rawMessage  = rawMessage;

src/legacy/test-run/index.js

@@ -27,8 +27,8 @@ export default class LegacyTestRun extends Session {
 
         this.isFileDownloading = false;
 
-        // TODO remove it then we move shared data to session storage
         this.errs                       = [];
+        this.warnings                   = [];
         this.nativeDialogsInfo          = null;
         this.nativeDialogsInfoTimeStamp = 0;
         this.stepsSharedData            = {};

src/reporter/index.js

@@ -1,4 +1,4 @@
-import { find, sortBy } from 'lodash';
+import { find, sortBy, uniqBy } from 'lodash';
 import ReporterPluginHost from './plugin-host';
 
 export default class Reporter {
@@ -26,11 +26,22 @@ export default class Reporter {
             screenshotPath: null,
             pendingRuns:    runsPerTest,
             errs:           [],
+            warnings:       [],
             unstable:       false,
             startTime:      null
         };
     }
 
+    static _createTestRunInfo (reportItem) {
+        return {
+            errs:           sortBy(reportItem.errs, ['userAgent', 'type']),
+            warnings:       reportItem.warnings,
+            durationMs:     new Date() - reportItem.startTime,
+            unstable:       reportItem.unstable,
+            screenshotPath: reportItem.screenshotPath
+        };
+    }
+
     _getReportItemForTestRun (testRun) {
         return find(this.reportQueue, i => i.test === testRun.test);
     }
@@ -41,14 +52,11 @@ export default class Reporter {
         // previous one frees all browser connections.
         // Therefore, tests always get completed sequentially.
         var reportItem = this.reportQueue.shift();
-        var durationMs = new Date() - reportItem.startTime;
 
         if (!reportItem.errs.length)
             this.passed++;
 
-        reportItem.errs = sortBy(reportItem.errs, ['userAgent', 'type']);
-
-        this.plugin.reportTestDone(reportItem.test.name, reportItem.errs, durationMs, reportItem.unstable, reportItem.screenshotPath);
+        this.plugin.reportTestDone(reportItem.test.name, Reporter._createTestRunInfo(reportItem));
 
         // NOTE: here we assume that tests are sorted by fixture.
         // Therefore, if the next report item has a different
@@ -80,8 +88,12 @@ export default class Reporter {
             var reportItem = this._getReportItemForTestRun(testRun);
 
             reportItem.pendingRuns--;
-            reportItem.errs     = reportItem.errs.concat(testRun.errs);
             reportItem.unstable = reportItem.unstable || testRun.unstable;
+            reportItem.errs     = reportItem.errs.concat(testRun.errs);
+
+            // NOTE: avoid duplicate warnings
+            reportItem.warnings = reportItem.warnings.concat(testRun.warnings);
+            reportItem.warnings = uniqBy(reportItem.warnings, 'message');
 
             if (!reportItem.pendingRuns) {
                 if (task.screenshots.hasCapturedFor(testRun.test))

src/runner/screenshots/capturer.js

@@ -21,13 +21,9 @@ export default class Capturer {
     async _takeScreenshot (windowId, filePath) {
         await ensureDir(dirname(filePath));
         await takeScreenshot(windowId, filePath);
-
-        return filePath;
     }
 
     async captureAction (windowId, { stepName, customPath }) {
-        this.testEntry.screenshotCapturingCalled = true;
-
         if (!this.enabled)
             return null;
 
@@ -38,7 +34,11 @@ export default class Capturer {
         if (customPath)
             this.testEntry.path = this.baseScreenshotsPath;
 
-        return await this._takeScreenshot(windowId, filePath);
+        await this._takeScreenshot(windowId, filePath);
+
+        this.testEntry.hasScreenshots = true;
+
+        return filePath;
     }
 
     async captureError (windowId, { stepName, screenshotRequired }) {
@@ -47,7 +47,9 @@ export default class Capturer {
 
         var filePath = joinPath(this.testScreenshotsPath, 'errors', Capturer._getFileName(stepName));
 
-        return await this._takeScreenshot(windowId, filePath);
+        await this._takeScreenshot(windowId, filePath);
+
+        return filePath;
     }
 }
 

src/runner/screenshots/index.js

@@ -2,7 +2,6 @@ import { join as joinPath } from 'path';
 import shortId from 'shortid';
 import { find } from 'lodash';
 import Capturer from './capturer';
-import SCREENSHOTS_WARNING_MESSAGES from './warning-messages';
 
 export default class Screenshots {
     constructor (path) {
@@ -21,9 +20,9 @@ export default class Screenshots {
 
     _addTestEntry (test) {
         var testEntry = {
-            test:                      test,
-            path:                      this.screenshotsPath ? joinPath(this.screenshotsPath, shortId.generate()) : '',
-            screenshotCapturingCalled: false
+            test:           test,
+            path:           this.screenshotsPath ? joinPath(this.screenshotsPath, shortId.generate()) : '',
+            hasScreenshots: false
         };
 
         this.testEntries.push(testEntry);
@@ -36,13 +35,10 @@ export default class Screenshots {
     }
 
     hasCapturedFor (test) {
-        return this._getTestEntry(test).screenshotCapturingCalled;
+        return this._getTestEntry(test).hasScreenshots;
     }
 
     getPathFor (test) {
-        if (this._getTestEntry(test).screenshotCapturingCalled && !this.enabled)
-            return SCREENSHOTS_WARNING_MESSAGES.screenshotDirNotSet;
-
         return this._getTestEntry(test).path;
     }
 

src/test-run/browser-manipulation-queue.js

@@ -1,12 +1,16 @@
+import { EventEmitter } from 'events';
 import shortId from 'shortid';
+import OS from 'os-family';
 import { resize as resizeWindow, getViewportSize } from 'testcafe-browser-natives';
-import SCREENSHOTS_WARNING_MESSAGES from '../runner/screenshots/warning-messages';
 import { isServiceCommand } from './commands/utils';
 import COMMAND_TYPE from './commands/type';
+import Warning from '../warnings';
+import WARNING_MESSAGE from '../warnings/message';
 
-
-export default class BrowserManipulationQueue {
+export default class BrowserManipulationQueue extends EventEmitter {
     constructor (windowId, screenshotCapturer) {
+        super();
+
         this.commands           = [];
         this.windowId           = windowId;
         this.screenshotCapturer = screenshotCapturer;
@@ -24,52 +28,42 @@ export default class BrowserManipulationQueue {
         return await resizeWindow(this.windowId, currentWidth, currentHeight, width, height);
     }
 
-    async _takeScreenshot (customPath) {
-        try {
-            return await this.screenshotCapturer.captureAction(this.windowId, {
-                stepName:   shortId.generate(),
-                customPath: customPath
-            });
-        }
-        catch (e) {
-            // NOTE: swallow the error silently if we can't take screenshots for some
-            // reason (e.g. we don't have permissions to write a screenshot file).
+    async _takeScreenshot (capture) {
+        if (!this.screenshotCapturer.enabled) {
+            this.emit('warning', new Warning(WARNING_MESSAGE.screenshotsPathNotSpecified));
             return null;
         }
-    }
-
-    async _takeScreenshotOnFail () {
-        if (!this.screenshotCapturer.enabled)
-            return SCREENSHOTS_WARNING_MESSAGES.screenshotDirNotSet;
 
         try {
-            return await this.screenshotCapturer.captureError(this.windowId, {
-                stepName:           shortId.generate(),
-                screenshotRequired: true
-            });
+            return await capture();
         }
-        catch (e) {
-            return SCREENSHOTS_WARNING_MESSAGES.cannotCreate;
+        catch (err) {
+            this.emit('warning', new Warning(WARNING_MESSAGE.screenshotError, err.message));
+            return null;
         }
     }
 
-    push (command) {
-        this.commands.push(command);
-    }
-
-    removeAllNonServiceManipulations () {
-        this.commands = this.commands.filter(command => isServiceCommand(command));
-    }
-
     async executePendingManipulation (driverMsg) {
+        // TODO: remove once https://github.com/DevExpress/testcafe-browser-natives/issues/12 implemented
+        if (OS.linux) {
+            this.emit('warning', new Warning(WARNING_MESSAGE.browserManipulationsNotSupportedOnLinux));
+            return null;
+        }
+
         var command = this.commands.shift();
 
         switch (command.type) {
             case COMMAND_TYPE.takeScreenshot:
-                return await this._takeScreenshot(command.path);
+                return await this._takeScreenshot(() => this.screenshotCapturer.captureAction(this.windowId, {
+                    stepName:   shortId.generate(),
+                    customPath: command.path
+                }));
 
             case COMMAND_TYPE.takeScreenshotOnFail:
-                return await this._takeScreenshotOnFail();
+                return await this._takeScreenshot(() => this.screenshotCapturer.captureError(this.windowId, {
+                    stepName:           shortId.generate(),
+                    screenshotRequired: true
+                }));
 
             case COMMAND_TYPE.resizeWindow:
                 return await this._resizeWindow(driverMsg.currentWidth, driverMsg.currentHeight, command.width, command.height);
@@ -80,4 +74,12 @@ export default class BrowserManipulationQueue {
 
         return null;
     }
+
+    push (command) {
+        this.commands.push(command);
+    }
+
+    removeAllNonServiceManipulations () {
+        this.commands = this.commands.filter(command => isServiceCommand(command));
+    }
 }

src/test-run/index.js

@@ -1,4 +1,5 @@
 import path from 'path';
+import { some } from 'lodash';
 import { readSync as read } from 'read-file-relative';
 import Promise from 'pinkie';
 import Mustache from 'mustache';
@@ -37,10 +38,9 @@ export default class TestRun extends Session {
 
         super(uploadsRoot);
 
-        this.opts                     = opts;
-        this.test                     = test;
-        this.browserConnection        = browserConnection;
-        this.browserManipulationQueue = new BrowserManipulationQueue(this.id, screenshotCapturer);
+        this.opts              = opts;
+        this.test              = test;
+        this.browserConnection = browserConnection;
 
         this.running = false;
         this.state   = STATE.initial;
@@ -51,17 +51,23 @@ export default class TestRun extends Session {
         this.pendingRequest   = null;
         this.pendingPageError = null;
 
+        this.errs     = [];
+        this.warnings = [];
+
+        this.lastDriverStatusId       = null;
+        this.lastDriverStatusResponse = null;
+
+        this.browserManipulationQueue = new BrowserManipulationQueue(this.id, screenshotCapturer);
+
+        this.browserManipulationQueue.on('warning', warning => this._addWarning(warning));
+
         this.debugLog = new TestRunDebugLog(this.browserConnection.userAgent);
 
         this.injectable.scripts.push('/testcafe-core.js');
         this.injectable.scripts.push('/testcafe-ui.js');
         this.injectable.scripts.push('/testcafe-runner.js');
         this.injectable.scripts.push('/testcafe-driver.js');
         this.injectable.styles.push('/testcafe-ui-styles.css');
-
-        this.errs                     = [];
-        this.lastDriverStatusId       = null;
-        this.lastDriverStatusResponse = null;
     }
 
 
@@ -166,6 +172,11 @@ export default class TestRun extends Session {
         this.errs.push(adapter);
     }
 
+    _addWarning (warning) {
+        // NOTE: avoid duplicates
+        if (!some(this.warnings, { message: warning.message }))
+            this.warnings.push(warning);
+    }
 
     // Task queue
     _enqueueCommand (command, callsite) {

src/utils/render-template.js

@@ -0,0 +1,3 @@
+export default function renderTemplate (template, ...args) {
+    return args.reduce((msg, arg) => msg.replace(/{.+?}/, arg), template);
+}

src/warnings/index.js

@@ -0,0 +1,7 @@
+import renderTemplate from '../utils/render-template';
+
+export default class Warning {
+    constructor () {
+        this.message = renderTemplate.apply(null, arguments);
+    }
+}

src/warnings/message.js

@@ -0,0 +1,5 @@
+export default {
+    screenshotsPathNotSpecified:             'Cannot take screenshots because the screenshot directory is not specified. To specify it, use the "-s" or "--screenshots" command line option or the "screenshots" method of the test runner in case you are using API.',
+    screenshotError:                         'Was unable to take a screenshot due to error.\n\n{errMessage}',
+    browserManipulationsNotSupportedOnLinux: 'The screenshot and window resize functionality are not yet supported on Linux. Subscribe to the following issue to keep track: https://github.com/DevExpress/testcafe-browser-natives/issues/12'
+};