From 76ab4921c101cd18005b2b32f0f7cf5ba6a6e597 Mon Sep 17 00:00:00 2001 From: Thi Nguyen Date: Sat, 27 Jan 2018 11:44:04 -0800 Subject: [PATCH] Screenshot file patterns (closes #1602, #1651, #1974, #1975) --- src/cli/argument-parser.js | 7 +++ src/cli/index.js | 2 +- src/runner/index.js | 4 +- src/runner/task.js | 2 +- src/screenshots/capturer.js | 36 +++++++++--- src/screenshots/index.js | 74 +++++++++++++++++++++---- test/server/cli-argument-parser-test.js | 1 + 7 files changed, 103 insertions(+), 23 deletions(-) diff --git a/src/cli/argument-parser.js b/src/cli/argument-parser.js index ca80dfc259a..1ea94b1b18b 100644 --- a/src/cli/argument-parser.js +++ b/src/cli/argument-parser.js @@ -89,6 +89,7 @@ export default class CLIArgumentParser { .option('-b, --list-browsers [provider]', 'output the aliases for local browsers or browsers available through the specified browser provider') .option('-r, --reporter ', 'specify the reporters and optionally files where reports are saved') .option('-s, --screenshots ', 'enable screenshot capturing and specify the path to save the screenshots to') + .option('-p, --pattern ', 'screenshot files are named using specific patterns: %BROWSER%, %BROWSERVERSION%, %OS%, %OSVERSION%, %USERAGENT%, %DATE%, %TIME%, %TEST%, %TESTNUMBER%, %FILENUMBER%') .option('-S, --screenshots-on-fails', 'take a screenshot whenever a test fails') .option('-q, --quarantine-mode', 'enable the quarantine mode') .option('-d, --debug-mode', 'execute test steps one by one pausing the test after each step') @@ -186,6 +187,11 @@ export default class CLIArgumentParser { this.opts.speed = parseFloat(this.opts.speed); } + _parseScreenshotsPattern () { + if (!this.opts.pattern) + this.opts.pattern = '%DATE%_%TIME%/test-%TESTNUMBER%/%USERAGENT%/%TESTNUMBER%'; + } + _parseConcurrency () { if (this.opts.concurrency) this.concurrency = parseInt(this.opts.concurrency, 10); @@ -311,6 +317,7 @@ export default class CLIArgumentParser { this._parsePorts(); this._parseBrowserList(); this._parseConcurrency(); + this._parseScreenshotsPattern(); await Promise.all([ this._parseScreenshotsPath(), diff --git a/src/cli/index.js b/src/cli/index.js index b8b11810b98..89f8b905c10 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -113,7 +113,7 @@ async function runTests (argParser) { .browsers(browsers) .concurrency(concurrency) .filter(argParser.filter) - .screenshots(opts.screenshots, opts.screenshotsOnFails) + .screenshots(opts.screenshots, opts.screenshotsOnFails, opts.pattern) .startApp(opts.app, opts.appInitDelay); runner.once('done-bootstrapping', () => log.hideSpinner()); diff --git a/src/runner/index.js b/src/runner/index.js index 0783cf1d200..512a5a43d3d 100644 --- a/src/runner/index.js +++ b/src/runner/index.js @@ -28,6 +28,7 @@ export default class Runner extends EventEmitter { externalProxyHost: null, screenshotPath: null, takeScreenshotsOnFails: false, + screenshotsPattern: null, skipJsErrors: false, quarantineMode: false, debugMode: false, @@ -171,9 +172,10 @@ export default class Runner extends EventEmitter { return this; } - screenshots (path, takeOnFails = false) { + screenshots (path, takeOnFails = false, pattern) { this.opts.takeScreenshotsOnFails = takeOnFails; this.opts.screenshotPath = path; + this.opts.screenshotsPattern = pattern; return this; } diff --git a/src/runner/task.js b/src/runner/task.js index 7793506f1b9..7a276b54f3c 100644 --- a/src/runner/task.js +++ b/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.screenshotsPattern); this.warningLog = new WarningLog(); this.fixtureHookController = new FixtureHookController(tests, browserConnectionGroups.length); diff --git a/src/screenshots/capturer.js b/src/screenshots/capturer.js index 47a09782d83..f953a40b710 100644 --- a/src/screenshots/capturer.js +++ b/src/screenshots/capturer.js @@ -14,18 +14,18 @@ export default class Capturer { this.testEntry = testEntry; this.provider = connection.provider; this.browserId = connection.id; - this.baseDirName = namingOptions.baseDirName; - this.userAgentName = namingOptions.userAgentName; this.quarantineAttemptNum = namingOptions.quarantineAttemptNum; this.testIndex = namingOptions.testIndex; this.screenshotIndex = 1; this.errorScreenshotIndex = 1; - var testDirName = `test-${this.testIndex}`; - var screenshotsPath = this.enabled ? joinPath(this.baseScreenshotsPath, this.baseDirName, testDirName) : ''; + var screenshotsPath = this.enabled ? this.baseScreenshotsPath : ''; this.screenshotsPath = screenshotsPath; this.screenshotPathForReport = screenshotsPath; + this.screenshotsPatternName = namingOptions.patternName; + + this.patternMap = namingOptions.patternMap; } static _correctFilePath (path) { @@ -38,8 +38,20 @@ export default class Capturer { return PNG_EXTENSION_RE.test(correctedPath) ? correctedPath : `${correctedPath}.png`; } + _parseFileNumber (fileName) { + if (fileName.indexOf('%FILENUMBER%') !== -1) + return fileName.replace(new RegExp('%FILENUMBER%', 'g'), (this.screenshotIndex - 1).toString().padStart(3, 0)); + + return fileName; + } + _getFileName (forError) { - var fileName = `${forError ? this.errorScreenshotIndex : this.screenshotIndex}.png`; + let fileName = ''; + + if (this.screenshotsPatternName) + fileName = `${this.screenshotsPatternName}.png`; + else + fileName = `${forError ? this.errorScreenshotIndex : this.screenshotIndex}.png`; if (forError) this.errorScreenshotIndex++; @@ -49,15 +61,22 @@ export default class Capturer { return fileName; } + _parsePattern (namePattern) { + for (const pattern in this.patternMap) + namePattern = namePattern.replace(new RegExp(`%${pattern}%`, 'g'), this.patternMap[pattern]); + + return namePattern; + } + _getSreenshotPath (fileName, customPath) { if (customPath) - return joinPath(this.baseScreenshotsPath, Capturer._correctFilePath(customPath)); + return joinPath(this.baseScreenshotsPath, Capturer._correctFilePath(this._parsePattern(customPath))); var screenshotPath = this.quarantineAttemptNum !== null ? joinPath(this.screenshotsPath, `run-${this.quarantineAttemptNum}`) : this.screenshotsPath; - return joinPath(screenshotPath, this.userAgentName, fileName); + return joinPath(screenshotPath, fileName); } async _takeScreenshot (filePath, pageWidth, pageHeight) { @@ -69,14 +88,13 @@ export default class Capturer { if (!this.enabled) return null; - var fileName = this._getFileName(forError); + var fileName = this._parseFileNumber(this._getFileName(forError)); fileName = forError ? joinPath('errors', fileName) : fileName; var screenshotPath = this._getSreenshotPath(fileName, customScreenshotPath); await this._takeScreenshot(screenshotPath, pageWidth, pageHeight); - await generateThumbnail(screenshotPath); // NOTE: if test contains takeScreenshot action with custom path diff --git a/src/screenshots/index.js b/src/screenshots/index.js index e40763b141e..0288db65398 100644 --- a/src/screenshots/index.js +++ b/src/screenshots/index.js @@ -4,18 +4,29 @@ import moment from 'moment'; import Capturer from './capturer'; export default class Screenshots { - constructor (path) { + constructor (path, pattern) { + const now = moment(Date.now()); + this.enabled = !!path; this.screenshotsPath = path; + this.screenshotsPattern = pattern; this.testEntries = []; - this.screenshotBaseDirName = Screenshots._getScreenshotBaseDirName(); this.userAgentNames = []; - } - - static _getScreenshotBaseDirName () { - var now = Date.now(); - - return moment(now).format('YYYY-MM-DD_hh-mm-ss'); + this.currentDate = now.format('YYYY-MM-DD'); + this.currentTime = now.format('HH-mm-ss'); + // FILENUMBER is handled in the Capturer class + this.patternMap = { + FIXTURE: null, + TEST: null, + TESTNUMBER: null, + DATE: this.currentDate, + TIME: this.currentTime, + USERAGENT: null, + BROWSER: null, + BROWSERVERSION: null, + OS: null, + OSVERSION: null + }; } static _escapeUserAgent (userAgent) { @@ -46,7 +57,19 @@ export default class Screenshots { } this.userAgentNames.push({ name: userAgentName, index: 0, testIndex, quarantineAttemptNum }); - return userAgentName; + + const userAgentMatches = userAgentName.match(new RegExp('^(.+?)_([0-9.]+)_(.+?)_([0-9.]+)$')); + + userAgentMatches.shift(); + const [browser, browserVersion, os, osVersion] = userAgentMatches; + + return { + full: userAgentName, + browser, + browserVersion, + os, + osVersion + }; } _addTestEntry (test) { @@ -73,17 +96,46 @@ export default class Screenshots { return this._getTestEntry(test).path; } + _parsePattern (namePattern, options) { + if (!namePattern) return ''; + + this.patternMap = Object.assign(this.patternMap, { + FIXTURE: options.fixture.replace(' ', '-'), + TEST: options.test.replace(' ', '-'), + TESTNUMBER: options.testNumber, + DATE: this.currentDate, + TIME: this.currentTime, + USERAGENT: options.userAgent.full, + BROWSER: options.userAgent.browser, + BROWSERVERSION: options.userAgent.browserVersion, + OS: options.userAgent.os, + OSVERSION: options.userAgent.osVersion + }); + + for (const pattern in this.patternMap) + namePattern = namePattern.replace(new RegExp(`%${pattern}%`, 'g'), this.patternMap[pattern]); + + return namePattern; + } + createCapturerFor (test, testIndex, quarantineAttemptNum, connection) { var testEntry = this._getTestEntry(test); if (!testEntry) testEntry = this._addTestEntry(test); + const patternOptions = { + testNumber: testIndex, + fixture: test.fixture.name, + test: test.name, + userAgent: this._getUserAgentName(connection.userAgent, testIndex, quarantineAttemptNum) + }; + var namingOptions = { testIndex, quarantineAttemptNum, - baseDirName: this.screenshotBaseDirName, - userAgentName: this._getUserAgentName(connection.userAgent, testIndex, quarantineAttemptNum) + patternMap: this.patternMap, + patternName: this._parsePattern(this.screenshotsPattern, patternOptions) }; return new Capturer(this.screenshotsPath, testEntry, connection, namingOptions); diff --git a/test/server/cli-argument-parser-test.js b/test/server/cli-argument-parser-test.js index 2e6ecdbddc0..9e45d934a5f 100644 --- a/test/server/cli-argument-parser-test.js +++ b/test/server/cli-argument-parser-test.js @@ -358,6 +358,7 @@ describe('CLI argument parser', function () { { long: '--list-browsers', short: '-b' }, { long: '--reporter', short: '-r' }, { long: '--screenshots', short: '-s' }, + { long: '--pattern', short: '-p' }, { long: '--screenshots-on-fails', short: '-S' }, { long: '--quarantine-mode', short: '-q' }, { long: '--debug-mode', short: '-d' },