Screenshot path pattern (closes #1602, #1651, #1974, #1975)

src/browser/connection/index.js

@@ -11,14 +11,10 @@ import STATUS from './status';
 import { GeneralError } from '../../errors/runtime';
 import MESSAGE from '../../errors/runtime/message';
 
-
-// Const
 const IDLE_PAGE_TEMPLATE = read('../../client/browser/idle-page/index.html.mustache');
 
-
 var connections = {};
 
-
 export default class BrowserConnection extends EventEmitter {
     constructor (gateway, browserInfo, permanent) {
         super();
@@ -202,7 +198,11 @@ export default class BrowserConnection extends EventEmitter {
     establish (userAgent) {
         this.ready = true;
 
-        this.browserInfo.userAgent = parseUserAgent(userAgent).toString();
+        const parsedUserAgent = parseUserAgent(userAgent);
+
+        this.browserInfo.userAgent       = parsedUserAgent.toString();
+        this.browserInfo.fullUserAgent   = userAgent;
+        this.browserInfo.parsedUserAgent = parsedUserAgent;
 
         this._waitForHeartbeat();
         this.emit('ready');

src/cli/argument-parser.js

@@ -19,7 +19,7 @@ const DEFAULT_TEST_LOOKUP_DIRS = ['test/', 'tests/'];
 const TEST_FILE_GLOB_PATTERN   = `./**/*@(${Compiler.getSupportedTestFileExtensions().join('|')})`;
 
 const DESCRIPTION = dedent(`
-    In the browser list, you can use browser names (e.g. "ie9", "chrome", etc.) as well as paths to executables.
+    In the browser list, you can use browser names (e.g. "ie", "chrome", etc.) as well as paths to executables.
 
     To run tests against all installed browsers, use the "all" alias.
 
@@ -33,7 +33,6 @@ const DESCRIPTION = dedent(`
     More info: https://devexpress.github.io/testcafe/documentation
 `);
 
-
 export default class CLIArgumentParser {
     constructor (cwd) {
         this.program = new Command('testcafe');
@@ -89,6 +88,7 @@ export default class CLIArgumentParser {
             .option('-r, --reporter <name[:outputFile][,...]>', 'specify the reporters and optionally files where reports are saved')
             .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('-p, --screenshot-path-pattern <pattern>', 'use patterns to compose screenshot file names and paths: ${BROWSER}, ${BROWSER_VERSION}, ${OS}, etc.')
             .option('-q, --quarantine-mode', 'enable the quarantine mode')
             .option('-d, --debug-mode', 'execute test steps one by one pausing the test after each step')
             .option('-e, --skip-js-errors', 'make tests not fail when a JS error happens on a page')

src/cli/cli.js

@@ -93,7 +93,7 @@ async function runTests (argParser) {
         .browsers(browsers)
         .concurrency(concurrency)
         .filter(argParser.filter)
-        .screenshots(opts.screenshots, opts.screenshotsOnFails)
+        .screenshots(opts.screenshots, opts.screenshotsOnFails, opts.screenshotPathPattern)
         .startApp(opts.app, opts.appInitDelay);
 
     runner.once('done-bootstrapping', () => log.hideSpinner());

src/runner/index.js

@@ -11,12 +11,10 @@ import { GeneralError } from '../errors/runtime';
 import MESSAGE from '../errors/runtime/message';
 import { assertType, is } from '../errors/runtime/type-assertions';
 
-
 const DEFAULT_SELECTOR_TIMEOUT  = 10000;
 const DEFAULT_ASSERTION_TIMEOUT = 3000;
 const DEFAULT_PAGE_LOAD_TIMEOUT = 3000;
 
-
 export default class Runner extends EventEmitter {
     constructor (proxy, browserConnectionGateway) {
         super();
@@ -30,6 +28,7 @@ export default class Runner extends EventEmitter {
             proxyBypass:            null,
             screenshotPath:         null,
             takeScreenshotsOnFails: false,
+            screenshotPathPattern:  null,
             skipJsErrors:           false,
             quarantineMode:         false,
             debugMode:              false,
@@ -198,9 +197,10 @@ export default class Runner extends EventEmitter {
         return this;
     }
 
-    screenshots (path, takeOnFails = false) {
+    screenshots (path, takeOnFails = false, pattern = null) {
         this.opts.takeScreenshotsOnFails = takeOnFails;
         this.opts.screenshotPath         = path;
+        this.opts.screenshotPathPattern  = pattern;
 
         return this;
     }

src/runner/task.js

@@ -12,7 +12,7 @@ export default class Task extends EventEmitter {
         this.running                 = false;
         this.browserConnectionGroups = browserConnectionGroups;
         this.tests                   = tests;
-        this.screenshots             = new Screenshots(opts.screenshotPath);
+        this.screenshots             = new Screenshots(opts.screenshotPath, opts.screenshotPathPattern);
         this.warningLog              = new WarningLog();
 
         this.fixtureHookController = new FixtureHookController(tests, browserConnectionGroups.length);

src/screenshots/capturer.js

@@ -1,46 +1,21 @@
 import { join as joinPath, dirname, basename } from 'path';
-import sanitizeFilename from 'sanitize-filename';
 import { generateThumbnail } from 'testcafe-browser-tools';
 import cropScreenshot from './crop';
 import { ensureDir } from '../utils/promisified-functions';
 import { isInQueue, addToQueue } from '../utils/async-queue';
 import WARNING_MESSAGE from '../notifications/warning-message';
-
-
-const PNG_EXTENSION_RE = /(\.png)$/;
-
+import escapeUserAgent from '../utils/escape-user-agent';
+import correctFilePath from '../utils/correct-file-path';
 
 export default class Capturer {
-    constructor (baseScreenshotsPath, testEntry, connection, namingOptions, warningLog) {
-        this.enabled              = !!baseScreenshotsPath;
-        this.baseScreenshotsPath  = baseScreenshotsPath;
-        this.testEntry            = testEntry;
-        this.provider             = connection.provider;
-        this.browserId            = connection.id;
-        this.baseDirName          = namingOptions.baseDirName;
-        this.userAgentName        = namingOptions.userAgentName;
-        this.quarantine           = namingOptions.quarantine;
-        this.attemptNumber        = this.quarantine ? this.quarantine.getNextAttemptNumber() : null;
-        this.testIndex            = namingOptions.testIndex;
-        this.screenshotIndex      = 1;
-        this.errorScreenshotIndex = 1;
-        this.warningLog           = warningLog;
-
-        var testDirName     = `test-${this.testIndex}`;
-        var screenshotsPath = this.enabled ? joinPath(this.baseScreenshotsPath, this.baseDirName, testDirName) : '';
-
-        this.screenshotsPath         = screenshotsPath;
-        this.screenshotPathForReport = screenshotsPath;
-    }
-
-    static _correctFilePath (path) {
-        var correctedPath = path
-            .replace(/\\/g, '/')
-            .split('/')
-            .map(str => sanitizeFilename(str))
-            .join('/');
-
-        return PNG_EXTENSION_RE.test(correctedPath) ? correctedPath : `${correctedPath}.png`;
+    constructor (baseScreenshotsPath, testEntry, connection, pathPattern, warningLog) {
+        this.enabled             = !!baseScreenshotsPath;
+        this.baseScreenshotsPath = baseScreenshotsPath;
+        this.testEntry           = testEntry;
+        this.provider            = connection.provider;
+        this.browserId           = connection.id;
+        this.warningLog          = warningLog;
+        this.pathPattern         = pathPattern;
     }
 
     static _getDimensionWithoutScrollbar (fullDimension, documentDimension, bodyDimension) {
@@ -80,30 +55,50 @@ export default class Capturer {
         };
     }
 
-    _getFileName (forError) {
-        var fileName = `${forError ? this.errorScreenshotIndex : this.screenshotIndex}.png`;
+    _joinWithBaseScreenshotPath (path) {
+        return joinPath(this.baseScreenshotsPath, path);
+    }
+
+    _updateScreenshotPathForTestEntry (customPath) {
+        // NOTE: if test contains takeScreenshot action with custom path
+        // we should specify the most common screenshot folder in report
+        let screenshotPathForTestEntry = this.baseScreenshotsPath;
+
+        if (!customPath) {
+            const pathForReport = this.pathPattern.getPathForReport();
+
+            screenshotPathForTestEntry = this._joinWithBaseScreenshotPath(pathForReport);
+        }
 
+
+        this.testEntry.path = screenshotPathForTestEntry;
+    }
+
+    _incrementFileIndexes (forError) {
         if (forError)
-            this.errorScreenshotIndex++;
+            this.pathPattern.data.errorFileIndex++;
+
         else
-            this.screenshotIndex++;
+            this.pathPattern.data.fileIndex++;
+    }
+
+    _getCustomScreenshotPath (customPath) {
+        const correctedCustomPath = correctFilePath(customPath);
 
-        return fileName;
+        return this._joinWithBaseScreenshotPath(correctedCustomPath);
     }
 
-    _getScreenshotPath (fileName, customPath) {
-        if (customPath)
-            return joinPath(this.baseScreenshotsPath, Capturer._correctFilePath(customPath));
+    _getScreenshotPath (forError) {
+        const path = this.pathPattern.getPath(forError);
 
-        var screenshotPath = this.attemptNumber !== null ?
-            joinPath(this.screenshotsPath, `run-${this.attemptNumber}`) : this.screenshotsPath;
+        this._incrementFileIndexes(forError);
 
-        return joinPath(screenshotPath, this.userAgentName, fileName);
+        return this._joinWithBaseScreenshotPath(path);
     }
 
     _getThumbnailPath (screenshotPath) {
-        var imageName = basename(screenshotPath);
-        var imageDir  = dirname(screenshotPath);
+        const imageName = basename(screenshotPath);
+        const imageDir  = dirname(screenshotPath);
 
         return joinPath(imageDir, 'thumbnails', imageName);
     }
@@ -117,12 +112,8 @@ export default class Capturer {
         if (!this.enabled)
             return null;
 
-        var fileName = this._getFileName(forError);
-
-        fileName = forError ? joinPath('errors', fileName) : fileName;
-
-        var screenshotPath = this._getScreenshotPath(fileName, customPath);
-        var thumbnailPath  = this._getThumbnailPath(screenshotPath);
+        const screenshotPath = customPath ? this._getCustomScreenshotPath(customPath) : this._getScreenshotPath(forError);
+        const thumbnailPath  = this._getThumbnailPath(screenshotPath);
 
         if (isInQueue(screenshotPath))
             this.warningLog.addWarning(WARNING_MESSAGE.screenshotRewritingError, screenshotPath);
@@ -135,18 +126,13 @@ export default class Capturer {
             await generateThumbnail(screenshotPath, thumbnailPath);
         });
 
-        // NOTE: if test contains takeScreenshot action with custom path
-        // we should specify the most common screenshot folder in report
-        if (customPath)
-            this.screenshotPathForReport = this.baseScreenshotsPath;
-
-        this.testEntry.path = this.screenshotPathForReport;
+        this._updateScreenshotPathForTestEntry(customPath);
 
         const screenshot = {
             screenshotPath,
             thumbnailPath,
-            userAgent:           this.userAgentName,
-            quarantineAttemptID: this.attemptNumber,
+            userAgent:           escapeUserAgent(this.pathPattern.data.parsedUserAgent),
+            quarantineAttemptID: this.pathPattern.data.quarantineAttempt,
             takenOnFail:         forError,
         };
 
@@ -155,7 +141,6 @@ export default class Capturer {
         return screenshotPath;
     }
 
-
     async captureAction (options) {
         return await this._capture(false, options);
     }

src/screenshots/index.js

@@ -1,56 +1,19 @@
 import { find } from 'lodash';
-import sanitizeFilename from 'sanitize-filename';
 import moment from 'moment';
 import Capturer from './capturer';
+import PathPattern from './path-pattern';
 
 export default class Screenshots {
-    constructor (path) {
-        this.enabled               = !!path;
-        this.screenshotsPath       = path;
-        this.testEntries           = [];
-        this.screenshotBaseDirName = Screenshots._getScreenshotBaseDirName();
-        this.userAgentNames        = [];
-    }
-
-    static _getScreenshotBaseDirName () {
-        var now = Date.now();
-
-        return moment(now).format('YYYY-MM-DD_hh-mm-ss');
-    }
-
-    static _escapeUserAgent (userAgent) {
-        return sanitizeFilename(userAgent.toString()).replace(/\s+/g, '_');
-    }
-
-    _getUsedUserAgent (name, testIndex, quarantineAttemptNum) {
-        var userAgent = null;
-
-        for (var i = 0; i < this.userAgentNames.length; i++) {
-            userAgent = this.userAgentNames[i];
-
-            if (userAgent.name === name && userAgent.testIndex === testIndex &&
-                userAgent.quarantineAttemptNum === quarantineAttemptNum)
-                return userAgent;
-        }
-
-        return null;
-    }
-
-    _getUserAgentName (userAgent, testIndex, quarantineAttemptNum) {
-        var userAgentName = Screenshots._escapeUserAgent(userAgent);
-        var usedUserAgent = this._getUsedUserAgent(userAgentName, testIndex, quarantineAttemptNum);
-
-        if (usedUserAgent) {
-            usedUserAgent.index++;
-            return `${userAgentName}_${usedUserAgent.index}`;
-        }
-
-        this.userAgentNames.push({ name: userAgentName, index: 0, testIndex, quarantineAttemptNum });
-        return userAgentName;
+    constructor (path, pattern) {
+        this.enabled            = !!path;
+        this.screenshotsPath    = path;
+        this.screenshotsPattern = pattern;
+        this.testEntries        = [];
+        this.now                = moment();
     }
 
     _addTestEntry (test) {
-        var testEntry = {
+        const testEntry = {
             test:        test,
             path:        this.screenshotsPath || '',
             screenshots: []
@@ -65,6 +28,15 @@ export default class Screenshots {
         return find(this.testEntries, entry => entry.test === test);
     }
 
+    _ensureTestEntry (test) {
+        let testEntry = this._getTestEntry(test);
+
+        if (!testEntry)
+            testEntry = this._addTestEntry(test);
+
+        return testEntry;
+    }
+
     getScreenshotsInfo (test) {
         return this._getTestEntry(test).screenshots;
     }
@@ -78,20 +50,16 @@ export default class Screenshots {
     }
 
     createCapturerFor (test, testIndex, quarantine, connection, warningLog) {
-        var testEntry = this._getTestEntry(test);
-
-        if (!testEntry)
-            testEntry = this._addTestEntry(test);
-
-        const quarantineAttemptNum = quarantine ? quarantine.getNextAttemptNumber() : null;
-
-        var namingOptions = {
+        const testEntry   = this._ensureTestEntry(test);
+        const pathPattern = new PathPattern(this.screenshotsPattern, {
             testIndex,
-            quarantine,
-            baseDirName:   this.screenshotBaseDirName,
-            userAgentName: this._getUserAgentName(connection.userAgent, testIndex, quarantineAttemptNum)
-        };
-
-        return new Capturer(this.screenshotsPath, testEntry, connection, namingOptions, warningLog);
+            quarantineAttempt: quarantine ? quarantine.getNextAttemptNumber() : null,
+            now:               this.now,
+            fixture:           test.fixture.name,
+            test:              test.name,
+            parsedUserAgent:   connection.browserInfo.parsedUserAgent,
+        });
+
+        return new Capturer(this.screenshotsPath, testEntry, connection, pathPattern, warningLog);
     }
 }