From 05d99c5b443ca63bebf6f0d1f37ca867cf82e85d Mon Sep 17 00:00:00 2001 From: Jigar wala Date: Thu, 14 Sep 2023 18:52:53 +0530 Subject: [PATCH] Percy on Automate: Adds specific configs as global config (#1369) * initial commit * combine percyCSS similar to web projects * raise warning when using params that are invalid with non automate projects * remove defaults, perform validation only when token passed * lint fix * increase coverage * improve coverage * refactor for improved coverage * refactor: remove return --- packages/config/src/utils/normalize.js | 3 +- packages/config/src/validate.js | 12 +++ packages/config/test/index.test.js | 116 +++++++++++++++++++++++-- packages/core/src/api.js | 7 +- packages/core/src/config.js | 42 +++++++++ packages/core/src/utils.js | 32 +++++-- packages/core/test/api.test.js | 59 ++++++++++--- packages/core/test/unit/utils.test.js | 42 +-------- packages/webdriver-utils/src/index.js | 57 ++++-------- 9 files changed, 262 insertions(+), 108 deletions(-) diff --git a/packages/config/src/utils/normalize.js b/packages/config/src/utils/normalize.js index 1b07c4c42..844fc4de3 100644 --- a/packages/config/src/utils/normalize.js +++ b/packages/config/src/utils/normalize.js @@ -5,7 +5,8 @@ import { getSchema } from '../validate.js'; const CAMELCASE_MAP = new Map([ ['css', 'CSS'], ['javascript', 'JavaScript'], - ['dom', 'DOM'] + ['dom', 'DOM'], + ['xpaths', 'XPaths'] ]); // Regular expression that matches words from boundaries or consecutive casing diff --git a/packages/config/src/validate.js b/packages/config/src/validate.js index 02e553811..add0b2f96 100644 --- a/packages/config/src/validate.js +++ b/packages/config/src/validate.js @@ -19,6 +19,18 @@ const ajv = new AJV({ getDefaultSchema() ], keywords: [{ + keyword: 'onlyAutomate', + error: { + message: 'property only valid with Automate integration.' + }, + code: cxt => { + let isAutomateProjectToken = (process.env.PERCY_TOKEN || '').split('_')[0] === 'auto'; + // we do validation only when token is passed + if (!!process.env.PERCY_TOKEN && !isAutomateProjectToken) { + cxt.error(); + } + } + }, { // custom instanceof schema validation keyword: 'instanceof', metaSchema: { diff --git a/packages/config/test/index.test.js b/packages/config/test/index.test.js index 207bbf4e8..5e81e6da5 100644 --- a/packages/config/test/index.test.js +++ b/packages/config/test/index.test.js @@ -355,6 +355,104 @@ describe('PercyConfig', () => { expect(conf).toEqual({ foo: { bar: 'baz', qux: 'xyzzy' } }); }); + describe('validates automate integration specific properties', () => { + beforeEach(() => { + delete process.env.PERCY_TOKEN; + + PercyConfig.addSchema({ + test: { + type: 'object', + additionalProperties: false, + properties: { + foo: { + type: 'number', + onlyAutomate: true + }, + bar: { + type: 'number' + } + } + } + }); + }); + + it('passes when no token present', () => { + expect(PercyConfig.validate({ + test: { + foo: 1, + bar: 2 + } + })).toBeUndefined(); + }); + + it('passes when token is of automate project', () => { + process.env.PERCY_TOKEN = 'auto_PERCY_TOKEN'; + + expect(PercyConfig.validate({ + test: { + foo: 1, + bar: 2 + } + })).toBeUndefined(); + }); + + it('warns when token is of legacy web project', () => { + process.env.PERCY_TOKEN = 'PERCY_TOKEN'; + + expect(PercyConfig.validate({ + test: { + foo: 1, + bar: 2 + } + })).toEqual([{ + path: 'test.foo', + message: 'property only valid with Automate integration.' + }]); + }); + + it('warns when token is of web project', () => { + process.env.PERCY_TOKEN = 'web_PERCY_TOKEN'; + + expect(PercyConfig.validate({ + test: { + foo: 1, + bar: 2 + } + })).toEqual([{ + path: 'test.foo', + message: 'property only valid with Automate integration.' + }]); + }); + + it('warns when token is of app project', () => { + process.env.PERCY_TOKEN = 'app_PERCY_TOKEN'; + + expect(PercyConfig.validate({ + test: { + foo: 1, + bar: 2 + } + })).toEqual([{ + path: 'test.foo', + message: 'property only valid with Automate integration.' + }]); + }); + + it('warns when token is of self serve project', () => { + process.env.PERCY_TOKEN = 'ss_PERCY_TOKEN'; + + expect(PercyConfig.validate({ + test: { + foo: 1, + bar: 2 + } + })).toEqual([{ + path: 'test.foo', + message: 'property only valid with Automate integration.' + }]); + }); + }); + it('can validate functions and regular expressions', () => { PercyConfig.addSchema({ func: { instanceof: 'Function' }, @@ -1102,7 +1200,8 @@ describe('PercyConfig', () => { 'percy-css': '', 'enable-javascript': false, 'disable-shadow-dom': true, - 'cli-enable-javascript': true + 'cli-enable-javascript': true, + 'ignore-region-xpaths': [''] })).toEqual({ fooBar: 'baz', foo: { barBaz: 'qux' }, @@ -1111,7 +1210,8 @@ describe('PercyConfig', () => { percyCSS: '', enableJavaScript: false, disableShadowDOM: true, - cliEnableJavaScript: true + cliEnableJavaScript: true, + ignoreRegionXPaths: [''] }); }); @@ -1123,7 +1223,8 @@ describe('PercyConfig', () => { percyCSS: '', enableJavaScript: false, disableShadowDOM: true, - cliEnableJavaScript: true + cliEnableJavaScript: true, + ignoreRegionXPaths: [''] }, { kebab: true })).toEqual({ 'foo-bar': 'baz', foo: { 'bar-baz': 'qux' }, @@ -1131,7 +1232,8 @@ describe('PercyConfig', () => { 'percy-css': '', 'enable-javascript': false, 'disable-shadow-dom': true, - 'cli-enable-javascript': true + 'cli-enable-javascript': true, + 'ignore-region-xpaths': [''] }); }); @@ -1143,7 +1245,8 @@ describe('PercyConfig', () => { percyCSS: '', enableJavaScript: false, disableShadowDOM: true, - cliEnableJavaScript: true + cliEnableJavaScript: true, + ignoreRegionXPaths: [''] }, { snake: true })).toEqual({ foo_bar: 'baz', foo: { bar_baz: 'qux' }, @@ -1151,7 +1254,8 @@ describe('PercyConfig', () => { percy_css: '', enable_javascript: false, disable_shadow_dom: true, - cli_enable_javascript: true + cli_enable_javascript: true, + ignore_region_xpaths: [''] }); }); diff --git a/packages/core/src/api.js b/packages/core/src/api.js index b10c348ae..ffeb4325c 100644 --- a/packages/core/src/api.js +++ b/packages/core/src/api.js @@ -117,10 +117,9 @@ export function createPercyServer(percy, port) { success: await percy.flush(req.body).then(() => true) })) .route('post', '/percy/automateScreenshot', async (req, res) => { - req = percyAutomateRequestHandler(req, percy.build); - res.json(200, { - success: await (percy.upload(await new WebdriverUtils(req.body).automateScreenshot())).then(() => true) - }); + percyAutomateRequestHandler(req, percy); + percy.upload(await WebdriverUtils.automateScreenshot(req.body)); + res.json(200, { success: true }); }) // stops percy at the end of the current event loop .route('/percy/stop', (req, res) => { diff --git a/packages/core/src/config.js b/packages/core/src/config.js index 59e31af58..8b2024d4f 100644 --- a/packages/core/src/config.js +++ b/packages/core/src/config.js @@ -49,6 +49,48 @@ export const configSchema = { }, scope: { type: 'string' + }, + freezeAnimation: { + type: 'boolean', + onlyAutomate: true + }, + ignoreRegions: { + type: 'object', + additionalProperties: false, + onlyAutomate: true, + properties: { + ignoreRegionSelectors: { + type: 'array', + items: { + type: 'string' + } + }, + ignoreRegionXpaths: { + type: 'array', + items: { + type: 'string' + } + } + } + }, + considerRegions: { + type: 'object', + additionalProperties: false, + onlyAutomate: true, + properties: { + considerRegionSelectors: { + type: 'array', + items: { + type: 'string' + } + }, + considerRegionXPaths: { + type: 'array', + items: { + type: 'string' + } + } + } } } }, diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index 4a3f412e5..6e055a71a 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -1,5 +1,6 @@ import EventEmitter from 'events'; import { sha256hash } from '@percy/client/utils'; +import { camelcase, merge } from '@percy/config/utils'; export { request, @@ -23,19 +24,38 @@ export function normalizeURL(url) { return `${protocol}//${host}${pathname}${search}`; } +/* istanbul ignore next: tested, but coverage is stripped */ // Returns the body for automateScreenshot in structure -export function percyAutomateRequestHandler(req, buildInfo) { +export function percyAutomateRequestHandler(req, percy) { if (req.body.client_info) { req.body.clientInfo = req.body.client_info; } if (req.body.environment_info) { req.body.environmentInfo = req.body.environment_info; } - if (!req.body.options) { - req.body.options = {}; - } - req.body.buildInfo = buildInfo; - return req; + + // combines array and overrides global config with per-screenshot config + let camelCasedOptions = {}; + Object.entries(req.body.options || {}).forEach(([key, value]) => { + camelCasedOptions[camelcase(key)] = value; + }); + + req.body.options = merge([{ + percyCSS: percy.config.snapshot.percyCSS, + freezeAnimation: percy.config.snapshot.freezeAnimation, + ignoreRegionSelectors: percy.config.snapshot.ignoreRegions?.ignoreRegionSelectors, + ignoreRegionXpaths: percy.config.snapshot.ignoreRegions?.ignoreRegionXpaths, + considerRegionSelectors: percy.config.snapshot.considerRegions?.considerRegionSelectors, + considerRegionXPaths: percy.config.snapshot.considerRegions?.considerRegionXPaths + }, + camelCasedOptions + ], (path, prev, next) => { + switch (path.map(k => k.toString()).join('.')) { + case 'percyCSS': // concatenate percy css + return [path, [prev, next].filter(Boolean).join('\n')]; + } + }); + req.body.buildInfo = percy.build; } // Creates a local resource object containing the resource URL, mimetype, content, sha, and any diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index 033503739..06670302f 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -2,7 +2,7 @@ import path from 'path'; import PercyConfig from '@percy/config'; import { logger, setupTest, fs } from './helpers/index.js'; import Percy from '@percy/core'; -import WebdriverUtils from '@percy/webdriver-utils'; // eslint-disable-line import/no-extraneous-dependencies +import WebdriverUtils from '@percy/webdriver-utils'; describe('API Server', () => { let percy; @@ -131,17 +131,6 @@ describe('API Server', () => { }); }); - it('has a /automateScreenshot endpoint that calls #upload()', async () => { - spyOn(percy, 'upload').and.resolveTo(); - spyOn(WebdriverUtils.prototype, 'automateScreenshot').and.resolveTo(true); - await percy.start(); - - await expectAsync(request('/percy/automateScreenshot', { - body: { name: 'Snapshot name' }, - method: 'post' - })).toBeResolvedTo({ success: true }); - }); - it('has a /stop endpoint that calls #stop()', async () => { spyOn(percy, 'stop').and.resolveTo(); await percy.start(); @@ -277,6 +266,52 @@ describe('API Server', () => { await expectAsync(pending).toBeResolved(); }); + it('has a /automateScreenshot endpoint that calls #upload() async with provided options', async () => { + let resolve, test = new Promise(r => (resolve = r)); + spyOn(percy, 'upload').and.returnValue(test); + let mockWebdriverUtilResponse = 'TODO: mocked response'; + let automateScreenshotSpy = spyOn(WebdriverUtils, 'automateScreenshot').and.resolveTo(mockWebdriverUtilResponse); + + await percy.start(); + + percy.config.snapshot.percyCSS = '.global { color: blue }'; + percy.config.snapshot.freezeAnimation = false; + percy.config.snapshot.ignoreRegions = { ignoreRegionSelectors: ['.selector-global'] }; + percy.config.snapshot.considerRegions = { considerRegionXPaths: ['/xpath-global'] }; + + await expectAsync(request('/percy/automateScreenshot', { + body: { + name: 'Snapshot name', + client_info: 'client', + environment_info: 'environment', + options: { + percyCSS: '.percy-screenshot: { color: red }', + freeze_animation: true, + ignore_region_xpaths: ['/xpath-per-screenshot'], + consider_region_xpaths: ['/xpath-per-screenshot'] + } + }, + method: 'post' + })).toBeResolvedTo({ success: true }); + + expect(automateScreenshotSpy).toHaveBeenCalledOnceWith(jasmine.objectContaining({ + clientInfo: 'client', + environmentInfo: 'environment', + buildInfo: { id: '123', url: 'https://percy.io/test/test/123', number: 1 }, + options: { + freezeAnimation: true, + percyCSS: '.global { color: blue }\n.percy-screenshot: { color: red }', + ignoreRegionSelectors: ['.selector-global'], + ignoreRegionXPaths: ['/xpath-per-screenshot'], + considerRegionXPaths: ['/xpath-global', '/xpath-per-screenshot'] + } + })); + + expect(percy.upload).toHaveBeenCalledOnceWith(mockWebdriverUtilResponse); + await expectAsync(test).toBePending(); + resolve(); // no hanging promises + }); + it('returns a 500 error when an endpoint throws', async () => { spyOn(percy, 'snapshot').and.rejectWith(new Error('test error')); await percy.start(); diff --git a/packages/core/test/unit/utils.test.js b/packages/core/test/unit/utils.test.js index 97929771d..354dc1bb7 100644 --- a/packages/core/test/unit/utils.test.js +++ b/packages/core/test/unit/utils.test.js @@ -2,8 +2,7 @@ import { generatePromise, AbortController, yieldTo, - yieldAll, - percyAutomateRequestHandler + yieldAll } from '../../src/utils.js'; describe('Unit / Utils', () => { @@ -166,43 +165,4 @@ describe('Unit / Utils', () => { { done: true, value: [2, 4, null, 3, 6] }); }); }); - - describe('percyAutomateRequestHandler', () => { - let req; - let percyBuildInfo; - beforeAll(() => { - req = { - body: { - name: 'abc', - client_info: 'client', - environment_info: 'environment' - } - }; - - percyBuildInfo = { - id: '123', - url: 'https://percy.io/abc/123' - }; - }); - - it('converts client_info to clientInfo', () => { - const nreq = percyAutomateRequestHandler(req, percyBuildInfo); - expect(nreq.body.clientInfo).toBe('client'); - }); - - it('converts environment_info to environmentInfo', () => { - const nreq = percyAutomateRequestHandler(req, percyBuildInfo); - expect(nreq.body.environmentInfo).toBe('environment'); - }); - - it('adds options', () => { - const nreq = percyAutomateRequestHandler(req, percyBuildInfo); - expect(nreq.body.options).toEqual({}); - }); - - it('adds percyBuildInfo', () => { - const nreq = percyAutomateRequestHandler(req, percyBuildInfo); - expect(nreq.body.buildInfo).toEqual(percyBuildInfo); - }); - }); }); diff --git a/packages/webdriver-utils/src/index.js b/packages/webdriver-utils/src/index.js index e5cabca74..7f20b2415 100644 --- a/packages/webdriver-utils/src/index.js +++ b/packages/webdriver-utils/src/index.js @@ -1,52 +1,33 @@ import ProviderResolver from './providers/providerResolver.js'; import utils from '@percy/sdk-utils'; -import { camelcase } from '@percy/config/utils'; export default class WebdriverUtils { - log = utils.logger('webdriver-utils:main'); - constructor( - { - sessionId, - commandExecutorUrl, - capabilities, - sessionCapabilites, - snapshotName, - clientInfo, - environmentInfo, - options = {}, - buildInfo = {} - }) { - this.sessionId = sessionId; - this.commandExecutorUrl = commandExecutorUrl; - this.capabilities = capabilities; - this.sessionCapabilites = sessionCapabilites; - this.snapshotName = snapshotName; - const camelCasedOptions = {}; - Object.keys(options).forEach((key) => { - let newKey = camelcase(key); - camelCasedOptions[newKey] = options[key]; - }); - this.options = camelCasedOptions; - this.clientInfo = clientInfo; - this.environmentInfo = environmentInfo; - this.buildInfo = buildInfo; - } - - async automateScreenshot() { + static async automateScreenshot({ + sessionId, + commandExecutorUrl, + capabilities, + sessionCapabilites, + snapshotName, + clientInfo, + environmentInfo, + options = {}, + buildInfo = {} + }) { + const log = utils.logger('webdriver-utils:automateScreenshot'); try { const startTime = Date.now(); - this.log.info(`[${this.snapshotName}] : Starting automate screenshot ...`); - const automate = ProviderResolver.resolve(this.sessionId, this.commandExecutorUrl, this.capabilities, this.sessionCapabilites, this.clientInfo, this.environmentInfo, this.options, this.buildInfo); - this.log.debug(`[${this.snapshotName}] : Resolved provider ...`); + log.info(`[${snapshotName}] : Starting automate screenshot ...`); + const automate = ProviderResolver.resolve(sessionId, commandExecutorUrl, capabilities, sessionCapabilites, clientInfo, environmentInfo, options, buildInfo); + log.debug(`[${snapshotName}] : Resolved provider ...`); await automate.createDriver(); - this.log.debug(`[${this.snapshotName}] : Created driver ...`); - const comparisonData = await automate.screenshot(this.snapshotName, this.options); + log.debug(`[${snapshotName}] : Created driver ...`); + const comparisonData = await automate.screenshot(snapshotName, options); comparisonData.metadata.cliScreenshotStartTime = startTime; comparisonData.metadata.cliScreenshotEndTime = Date.now(); return comparisonData; } catch (e) { - this.log.error(`[${this.snapshotName}] : Error - ${e.message}`); - this.log.error(`[${this.snapshotName}] : Error Log - ${e.toString()}`); + log.error(`[${snapshotName}] : Error - ${e.message}`); + log.error(`[${snapshotName}] : Error Log - ${e.toString()}`); } } }