From fa6dd2ee4448aa14dd3ae1dbf66dad9128b4fb8e Mon Sep 17 00:00:00 2001 From: Mikhail Losev Date: Fri, 31 Aug 2018 17:12:28 +0300 Subject: [PATCH] fix '${BROWSER} resolves to undefined in screenshot pattern' (close #2742) (#2794) * fix '${BROWSER} resolves to undefined in screenshot pattern' (close #2742) * fix review issue --- src/api/structure/testing-unit.js | 2 +- src/api/test-controller/assertion.js | 4 +-- src/api/test-controller/index.js | 22 ++++++------ src/api/test-controller/proxy.js | 6 ++-- src/api/test-page-url.js | 10 +++--- src/api/test-run-tracker.js | 22 ++++++------ src/api/wrap-test-function.js | 6 ++-- src/assertions/executor.js | 6 ++-- src/screenshots/crop.js | 22 ++++++------ src/screenshots/generate-mark.js | 12 +++---- src/screenshots/path-pattern.js | 8 ++--- test/server/path-pattern-test.js | 51 ++++++++++++++-------------- 12 files changed, 85 insertions(+), 86 deletions(-) diff --git a/src/api/structure/testing-unit.js b/src/api/structure/testing-unit.js index e1d79a59e7..1b3866bb79 100644 --- a/src/api/structure/testing-unit.js +++ b/src/api/structure/testing-unit.js @@ -18,7 +18,7 @@ export default class TestingUnit { this.disablePageReloads = void 0; - var unit = this; + const unit = this; this.apiOrigin = function apiOrigin (...args) { return unit._add(...args); diff --git a/src/api/test-controller/assertion.js b/src/api/test-controller/assertion.js index cd35f6c97f..2a8e145168 100644 --- a/src/api/test-controller/assertion.js +++ b/src/api/test-controller/assertion.js @@ -7,8 +7,8 @@ export default class Assertion { } _enqueueAssertion (apiMethodName, assertionArgs) { - var options = assertionArgs.opts || {}; - var message = assertionArgs.message; + let options = assertionArgs.opts || {}; + let message = assertionArgs.message; if (typeof message === 'object') { options = assertionArgs.message; diff --git a/src/api/test-controller/index.js b/src/api/test-controller/index.js index 735e0c84a8..240235534d 100644 --- a/src/api/test-controller/index.js +++ b/src/api/test-controller/index.js @@ -61,9 +61,9 @@ export default class TestController { // t.click('#btn2'); // <-- stores new callsiteWithoutAwait // await t2.click('#btn3'); // <-- without check it will set callsiteWithoutAwait = null, so we will lost tracking _createExtendedPromise (promise, callsite) { - var extendedPromise = promise.then(identity); - var originalThen = extendedPromise.then; - var markCallsiteAwaited = () => this.callsitesWithoutAwait.delete(callsite); + const extendedPromise = promise.then(identity); + const originalThen = extendedPromise.then; + const markCallsiteAwaited = () => this.callsitesWithoutAwait.delete(callsite); extendedPromise.then = function () { @@ -80,8 +80,8 @@ export default class TestController { } _enqueueTask (apiMethodName, createTaskExecutor) { - var callsite = getCallsiteForMethod(apiMethodName); - var executor = createTaskExecutor(callsite); + const callsite = getCallsiteForMethod(apiMethodName); + const executor = createTaskExecutor(callsite); this.executionChain = this.executionChain.then(executor); @@ -92,7 +92,7 @@ export default class TestController { _enqueueCommand (apiMethodName, CmdCtor, cmdArgs) { return this._enqueueTask(apiMethodName, callsite => { - var command = null; + let command = null; try { command = new CmdCtor(cmdArgs, this.testRun); @@ -200,7 +200,7 @@ export default class TestController { } _takeElementScreenshot$ (selector, ...args) { - var commandArgs = { selector }; + const commandArgs = { selector }; if (args[1]) { commandArgs.path = args[0]; @@ -238,8 +238,8 @@ export default class TestController { if (!isNullOrUndefined(options)) options = assign({}, options, { boundTestRun: this }); - var builder = new ClientFunctionBuilder(fn, options, { instantiation: 'eval', execution: 'eval' }); - var clientFn = builder.getFunction(); + const builder = new ClientFunctionBuilder(fn, options, { instantiation: 'eval', execution: 'eval' }); + const clientFn = builder.getFunction(); return clientFn(); } @@ -251,13 +251,13 @@ export default class TestController { } _getNativeDialogHistory$ () { - var callsite = getCallsiteForMethod('getNativeDialogHistory'); + const callsite = getCallsiteForMethod('getNativeDialogHistory'); return this.testRun.executeCommand(new GetNativeDialogHistoryCommand(), callsite); } _getBrowserConsoleMessages$ () { - var callsite = getCallsiteForMethod('getBrowserConsoleMessages'); + const callsite = getCallsiteForMethod('getBrowserConsoleMessages'); return this.testRun.executeCommand(new GetBrowserConsoleMessagesCommand(), callsite); } diff --git a/src/api/test-controller/proxy.js b/src/api/test-controller/proxy.js index d254483338..13b13c9656 100644 --- a/src/api/test-controller/proxy.js +++ b/src/api/test-controller/proxy.js @@ -4,14 +4,14 @@ import testRunTracker from '../test-run-tracker'; import { APIError } from '../../errors/runtime'; import MESSAGE from '../../errors/runtime/message'; -var testControllerProxy = Object.create(null); +const testControllerProxy = Object.create(null); delegateAPI(testControllerProxy, TestController.API_LIST, { getHandler (propName, accessor) { - var testRun = testRunTracker.resolveContextTestRun(); + const testRun = testRunTracker.resolveContextTestRun(); if (!testRun) { - var callsiteName = null; + let callsiteName = null; if (accessor === 'getter') callsiteName = 'get'; diff --git a/src/api/test-page-url.js b/src/api/test-page-url.js index 8237615717..2ffd162fdd 100644 --- a/src/api/test-page-url.js +++ b/src/api/test-page-url.js @@ -16,7 +16,7 @@ function isAbsolutePath (url) { } function resolveFileUrl (url, testFileName) { - var testFileDir = path.dirname(testFileName); + const testFileDir = path.dirname(testFileName); if (RELATIVE_PATH_RE.test(url)) url = path.join(testFileDir, url); @@ -25,9 +25,9 @@ function resolveFileUrl (url, testFileName) { } export function assertUrl (url, callsiteName) { - var protocol = url.match(PROTOCOL_RE); - var hasUnsupportedProtocol = protocol && !SUPPORTED_PROTOCOL_RE.test(url); - var isWinAbsolutePath = OS.win && WIN_ABSOLUTE_PATH_RE.test(url); + const protocol = url.match(PROTOCOL_RE); + const hasUnsupportedProtocol = protocol && !SUPPORTED_PROTOCOL_RE.test(url); + const isWinAbsolutePath = OS.win && WIN_ABSOLUTE_PATH_RE.test(url); if (hasUnsupportedProtocol && !isWinAbsolutePath && url !== 'about:blank') throw new APIError(callsiteName, MESSAGE.unsupportedUrlProtocol, url, protocol[0]); @@ -40,7 +40,7 @@ export function resolvePageUrl (url, testFileName) { if (isAbsolutePath(url) || RELATIVE_PATH_RE.test(url)) return resolveFileUrl(url, testFileName); - var protocol = IMPLICIT_PROTOCOL_RE.test(url) ? 'http:' : 'http://'; + const protocol = IMPLICIT_PROTOCOL_RE.test(url) ? 'http:' : 'http://'; return protocol + url; } diff --git a/src/api/test-run-tracker.js b/src/api/test-run-tracker.js index cadfff4c66..fdd90db539 100644 --- a/src/api/test-run-tracker.js +++ b/src/api/test-run-tracker.js @@ -11,13 +11,13 @@ export default { activeTestRuns: {}, _createContextSwitchingFunctionHook (ctxSwitchingFn, patchedArgsCount) { - var tracker = this; + const tracker = this; return function () { - var testRunId = tracker.getContextTestRunId(); + const testRunId = tracker.getContextTestRunId(); if (testRunId) { - for (var i = 0; i < patchedArgsCount; i++) { + for (let i = 0; i < patchedArgsCount; i++) { if (typeof arguments[i] === 'function') arguments[i] = tracker.addTrackingMarkerToFunction(testRunId, arguments[i]); } @@ -29,11 +29,11 @@ export default { _getStackFrames () { // NOTE: increase stack capacity to seek deep stack entries - var savedLimit = Error.stackTraceLimit; + const savedLimit = Error.stackTraceLimit; Error.stackTraceLimit = STACK_CAPACITY; - var frames = getStackFrames(); + const frames = getStackFrames(); Error.stackTraceLimit = savedLimit; @@ -60,7 +60,7 @@ export default { }, addTrackingMarkerToFunction (testRunId, fn) { - var markerFactoryBody = ` + const markerFactoryBody = ` return function $$testcafe_test_run$$${testRunId}$$ () { switch (arguments.length) { case 0: return fn.call(this); @@ -77,16 +77,16 @@ export default { }, getContextTestRunId () { - var frames = this._getStackFrames(); + const frames = this._getStackFrames(); // OPTIMIZATION: we start traversing from the bottom of the stack, // because we'll more likely encounter a marker there. // Async/await and Promise machinery executes lots of intrinsics // on timers (where we have a marker). And, since a timer initiates a new // stack, the marker will be at the very bottom of it. - for (var i = frames.length - 1; i >= 0; i--) { - var fnName = frames[i].getFunctionName(); - var match = fnName && fnName.match(TRACKING_MARK_RE); + for (let i = frames.length - 1; i >= 0; i--) { + const fnName = frames[i].getFunctionName(); + const match = fnName && fnName.match(TRACKING_MARK_RE); if (match) return match[1]; @@ -96,7 +96,7 @@ export default { }, resolveContextTestRun () { - var testRunId = this.getContextTestRunId(); + const testRunId = this.getContextTestRunId(); return this.activeTestRuns[testRunId]; } diff --git a/src/api/wrap-test-function.js b/src/api/wrap-test-function.js index 167f6c7f2f..5c1b0623be 100644 --- a/src/api/wrap-test-function.js +++ b/src/api/wrap-test-function.js @@ -6,9 +6,9 @@ import { MissingAwaitError } from '../errors/test-run'; export default function wrapTestFunction (fn) { return async testRun => { - var result = null; - var errList = new TestCafeErrorList(); - var markeredfn = testRunTracker.addTrackingMarkerToFunction(testRun.id, fn); + let result = null; + const errList = new TestCafeErrorList(); + const markeredfn = testRunTracker.addTrackingMarkerToFunction(testRun.id, fn); testRun.controller = new TestController(testRun); diff --git a/src/assertions/executor.js b/src/assertions/executor.js index fe506b6292..6382fc73f3 100644 --- a/src/assertions/executor.js +++ b/src/assertions/executor.js @@ -19,8 +19,8 @@ export default class AssertionExecutor extends EventEmitter { this.passed = false; this.inRetry = false; - var fn = getFn(this.command); - var actualCommand = this.command.actual; + const fn = getFn(this.command); + const actualCommand = this.command.actual; if (actualCommand instanceof ReExecutablePromise) this.fn = this._wrapFunction(fn); @@ -41,7 +41,7 @@ export default class AssertionExecutor extends EventEmitter { _wrapFunction (fn) { return async () => { - var resultPromise = this.command.actual; + const resultPromise = this.command.actual; while (!this.passed) { this.command.actual = await resultPromise._reExecute(); diff --git a/src/screenshots/crop.js b/src/screenshots/crop.js index be614674d7..498b2f596a 100644 --- a/src/screenshots/crop.js +++ b/src/screenshots/crop.js @@ -11,8 +11,8 @@ import WARNING_MESSAGES from '../notifications/warning-message'; function readPng (filePath) { - var png = new PNG(); - var parsedPromise = Promise.race([ + const png = new PNG(); + const parsedPromise = Promise.race([ promisifyEvent(png, 'parsed'), promisifyEvent(png, 'error') ]); @@ -24,8 +24,8 @@ function readPng (filePath) { } function writePng (filePath, png) { - var outStream = fs.createWriteStream(filePath); - var finishPromise = Promise.race([ + const outStream = fs.createWriteStream(filePath); + const finishPromise = Promise.race([ promisifyEvent(outStream, 'finish'), promisifyEvent(outStream, 'error') ]); @@ -53,14 +53,14 @@ function detectClippingArea (srcImage, { markSeed, clientAreaDimensions, cropDim let clipHeight = srcImage.height; if (markSeed && clientAreaDimensions) { - var mark = Buffer.from(markSeed); + const mark = Buffer.from(markSeed); - var markIndex = srcImage.data.indexOf(mark); + const markIndex = srcImage.data.indexOf(mark); if (markIndex < 0) throw new Error(renderTemplate(WARNING_MESSAGES.screenshotMarkNotFound, screenshotPath, markSeedToId(markSeed))); - var endPosition = markIndex / MARK_BYTES_PER_PIXEL + MARK_LENGTH + MARK_RIGHT_MARGIN; + const endPosition = markIndex / MARK_BYTES_PER_PIXEL + MARK_LENGTH + MARK_RIGHT_MARGIN; clipRight = endPosition % srcImage.width || srcImage.width; clipBottom = (endPosition - clipRight) / srcImage.width + 1; @@ -94,11 +94,11 @@ function detectClippingArea (srcImage, { markSeed, clientAreaDimensions, cropDim } function copyImagePart (srcImage, { left, top, width, height }) { - var dstImage = new PNG({ width, height }); - var stride = dstImage.width * MARK_BYTES_PER_PIXEL; + const dstImage = new PNG({ width, height }); + const stride = dstImage.width * MARK_BYTES_PER_PIXEL; for (let i = 0; i < height; i++) { - var srcStartIndex = (srcImage.width * (i + top) + left) * MARK_BYTES_PER_PIXEL; + const srcStartIndex = (srcImage.width * (i + top) + left) * MARK_BYTES_PER_PIXEL; srcImage.data.copy(dstImage.data, stride * i, srcStartIndex, srcStartIndex + stride); } @@ -107,7 +107,7 @@ function copyImagePart (srcImage, { left, top, width, height }) { } export default async function (screenshotPath, markSeed, clientAreaDimensions, cropDimensions) { - var srcImage = await readPng(screenshotPath); + const srcImage = await readPng(screenshotPath); const clippingArea = detectClippingArea(srcImage, { markSeed, clientAreaDimensions, cropDimensions, screenshotPath }); diff --git a/src/screenshots/generate-mark.js b/src/screenshots/generate-mark.js index cd2478b845..c95d52a9c0 100644 --- a/src/screenshots/generate-mark.js +++ b/src/screenshots/generate-mark.js @@ -8,21 +8,21 @@ const ALPHABET = '01'; export default function () { // NOTE: 32-bit id - var id = generateId(ALPHABET, MARK_LENGTH); + const id = generateId(ALPHABET, MARK_LENGTH); // NOTE: array of RGB values - var markSeed = flatten(map(id, bit => bit === '0' ? [0, 0, 0, 255] : [255, 255, 255, 255])); + const markSeed = flatten(map(id, bit => bit === '0' ? [0, 0, 0, 255] : [255, 255, 255, 255])); // NOTE: macOS browsers can't display an element, if it's CSS height is lesser than 1. // It happens on Retina displays, because they have more than 1 physical pixel in a CSS pixel. // So increase mark size by prepending transparent pixels before the actual mark. - var imageData = times(MARK_BYTES_PER_PIXEL * MARK_LENGTH * (MARK_HEIGHT - 1), constant(0)).concat(markSeed); - var imageDataBuffer = Buffer.from(imageData); - var pngImage = new PNG({ width: MARK_LENGTH, height: MARK_HEIGHT }); + const imageData = times(MARK_BYTES_PER_PIXEL * MARK_LENGTH * (MARK_HEIGHT - 1), constant(0)).concat(markSeed); + const imageDataBuffer = Buffer.from(imageData); + const pngImage = new PNG({ width: MARK_LENGTH, height: MARK_HEIGHT }); imageDataBuffer.copy(pngImage.data); - var markData = 'data:image/png;base64,' + PNG.sync.write(pngImage).toString('base64'); + const markData = 'data:image/png;base64,' + PNG.sync.write(pngImage).toString('base64'); return { markSeed, markData }; } diff --git a/src/screenshots/path-pattern.js b/src/screenshots/path-pattern.js index fd241d3296..59bc880bd6 100644 --- a/src/screenshots/path-pattern.js +++ b/src/screenshots/path-pattern.js @@ -63,10 +63,10 @@ export default class PathPattern { [PLACEHOLDERS.TEST]: this.data.test, [PLACEHOLDERS.FILE_INDEX]: forError => forError ? this.data.errorFileIndex : this.data.fileIndex, [PLACEHOLDERS.USERAGENT]: this.data.parsedUserAgent.toString(), - [PLACEHOLDERS.BROWSER]: this.data.parsedUserAgent.browser, - [PLACEHOLDERS.BROWSER_VERSION]: this.data.parsedUserAgent.browserVersion, - [PLACEHOLDERS.OS]: this.data.parsedUserAgent.os, - [PLACEHOLDERS.OS_VERSION]: this.data.parsedUserAgent.osVersion + [PLACEHOLDERS.BROWSER]: this.data.parsedUserAgent.family, + [PLACEHOLDERS.BROWSER_VERSION]: this.data.parsedUserAgent.toVersion(), + [PLACEHOLDERS.OS]: this.data.parsedUserAgent.os.family, + [PLACEHOLDERS.OS_VERSION]: this.data.parsedUserAgent.os.toVersion() }; } diff --git a/test/server/path-pattern-test.js b/test/server/path-pattern-test.js index e9310a9a66..867769d959 100644 --- a/test/server/path-pattern-test.js +++ b/test/server/path-pattern-test.js @@ -1,12 +1,18 @@ const PathPattern = require('../../lib/screenshots/path-pattern'); const expect = require('chai').expect; const moment = require('moment'); +const userAgent = require('useragent'); describe('Screenshot path pattern', () => { + const parsedUserAgentMock = { + toVersion: () => {}, + os: { toVersion: () => {} } + }; + const createPathPattern = (pattern, data) => { data = data || {}; data.now = data.now || moment(); - data.parsedUserAgent = data.parsedUserAgent || {}; + data.parsedUserAgent = data.parsedUserAgent || parsedUserAgentMock; data.quarantineAttempt = data.quarantineAttempt || null; return new PathPattern(pattern, data); @@ -27,39 +33,32 @@ describe('Screenshot path pattern', () => { }); it('Should replace all placeholders', () => { - const pattern = Object.getOwnPropertyNames(PathPattern.PLACEHOLDERS).map(name => PathPattern.PLACEHOLDERS[name]).join('#'); - const dateStr = '2010-01-02'; - const timeStr = '11:12:13'; + const pattern = Object.getOwnPropertyNames(PathPattern.PLACEHOLDERS).map(name => PathPattern.PLACEHOLDERS[name]).join('#'); + const dateStr = '2010-01-02'; + const timeStr = '11:12:13'; + const parsedUserAgent = userAgent.parse('Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36'); const data = { now: moment(dateStr + ' ' + timeStr), testIndex: 12, fileIndex: 34, - quarantineAttempt: 56, + quarantineAttempt: 2, fixture: 'fixture', test: 'test', - parsedUserAgent: { - browser: 'Chrome', - browserVersion: '67.0.3396', - os: 'Windows', - osVersion: '8.1.0.0', - toString: function () { - return 'full_user_agent'; - } - } + parsedUserAgent }; const expectedParsedPattern = [ - dateStr, - timeStr.replace(/:/g, '-'), - data.testIndex, - data.fileIndex, - data.quarantineAttempt, - data.fixture, - data.test, - data.parsedUserAgent.toString(), - data.parsedUserAgent.browser, - data.parsedUserAgent.browserVersion, - data.parsedUserAgent.os, - data.parsedUserAgent.osVersion + '2010-01-02', + '11-12-13', + '12', + '34', + '2', + 'fixture', + 'test', + 'Chrome_68.0.3440_Windows_8.1.0.0', + 'Chrome', + '68.0.3440', + 'Windows', + '8.1.0.0' ].join('#') + '.png'; const pathPattern = createPathPattern(pattern, data);