From 15529fee429418417d22200c9ed02c0aff72d944 Mon Sep 17 00:00:00 2001 From: Shobhit Kumar Date: Wed, 19 Feb 2025 11:32:52 +0530 Subject: [PATCH 1/4] Added normalization function to make options case-insensitive --- packages/core/src/snapshot.js | 34 +++++++++- packages/core/test/percy.test.js | 103 +++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 3 deletions(-) diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index fd4b55625..e649171be 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -165,13 +165,41 @@ function getSnapshotOptions(options, { config, meta }) { } }); } +const optionMappings = { + 'name': 'name', + 'widths': 'widths', + 'scope': 'scope', + 'scopeoptions': 'scopeOptions', + 'minheight': 'minHeight', + 'enablejavascript': 'enableJavaScript', + 'enablelayout': 'enableLayout', + 'clientinfo': 'clientInfo', + 'environmentinfo': 'environmentInfo', + 'sync': 'sync', + 'testcase': 'testCase', + 'labels': 'labels', + 'thtestcaseexecutionid': 'thTestCaseExecutionId', + 'resources': 'resources', + 'meta': 'meta', + 'snapshot': 'snapshot', +}; + +function normalizeOptions(options) { + const normalizedOptions = {}; + + for (const key in options) { + const lowerCaseKey = key.toLowerCase().replace(/[-_]/g, ''); + const normalizedKey = optionMappings[lowerCaseKey] ? optionMappings[lowerCaseKey] : key; + normalizedOptions[normalizedKey] = options[key]; + } + return normalizedOptions; +} // Validates and migrates snapshot options against the correct schema based on provided // properties. Eagerly throws an error when missing a URL for any snapshot, and warns about all // other invalid options which are also scrubbed from the returned migrated options. export function validateSnapshotOptions(options) { let log = logger('core:snapshot'); - // decide which schema to validate against let schema = ( (['domSnapshot', 'dom-snapshot', 'dom_snapshot'] @@ -182,6 +210,8 @@ export function validateSnapshotOptions(options) { ('snapshots' in options && '/snapshot/list') || ('/snapshot')); + options = normalizeOptions(options); + let { // normalize, migrate, and remove certain properties from validating clientInfo, environmentInfo, snapshots, ...migrated @@ -213,10 +243,8 @@ export function validateSnapshotOptions(options) { log.warn('Encountered snapshot serialization warnings:'); for (let w of domWarnings) log.warn(`- ${w}`); } - // warn on validation errors let errors = PercyConfig.validate(migrated, schema); - if (errors?.length > 0) { log.warn('Invalid snapshot options:'); for (let e of errors) log.warn(`- ${e.path}: ${e.message}`); diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js index f7d5e38e8..e6a06219b 100644 --- a/packages/core/test/percy.test.js +++ b/packages/core/test/percy.test.js @@ -3,6 +3,7 @@ import { generatePromise, AbortController, base64encode } from '../src/utils.js' import Percy from '@percy/core'; import Pako from 'pako'; import DetectProxy from '@percy/client/detect-proxy'; +import { validateSnapshotOptions } from '../src/snapshot.js'; describe('Percy', () => { let percy, server; @@ -1713,4 +1714,106 @@ describe('Percy', () => { }); }); }); + describe('#validateSnapshotOptions', () => { + it('normalizes snapshot options to camelCase and does not produce errors', async () => { + const options = { + name: 'Snapshot 1', + url: 'http://localhost:8000', + 'scope-options': { scroll: true }, + 'min-height': 1024, + 'enable-javascript': true, + 'client-info': 'client-info', + 'environment-info': 'env-info', + 'test-case': 'testCase', + 'th-test-case-execution-id': '12345', + }; + + const result = validateSnapshotOptions(options); + expect(result).toEqual({ + clientInfo: 'client-info', + environmentInfo: 'env-info', + name: 'Snapshot 1', + url: 'http://localhost:8000', + scopeOptions: { scroll: true }, + minHeight: 1024, + enableJavaScript: true, + testCase: 'testCase', + thTestCaseExecutionId: '12345', + }); + }); + + it('handles different casing formats and does not produce errors', async () => { + const options = { + Name: 'Snapshot 1', + url: 'http://localhost:8000', + WIDTHS: [375, 1280], + Scope: '#app', + ScopeOptions: { scroll: true }, + MinHeight: 1024, + EnableJavaScript: true, + EnableLayout: false, + ClientInfo: 'client-info', + EnvironmentInfo: 'env-info', + Sync: true, + TestCase: 'testCase', + Labels: 'label1', + ThTestCaseExecutionId: '12345', + }; + + const result = validateSnapshotOptions(options); + expect(result).toEqual({ + clientInfo: 'client-info', + environmentInfo: 'env-info', + name: 'Snapshot 1', + url: 'http://localhost:8000', + widths: [375, 1280], + scope: '#app', + scopeOptions: { scroll: true }, + minHeight: 1024, + enableJavaScript: true, + enableLayout: false, + sync: true, + testCase: 'testCase', + labels: 'label1', + thTestCaseExecutionId: '12345', + }); + }); + + it('handles mixed casing and special characters and does not produce errors', async () => { + const options = { + nAmE: 'Snapshot 1', + url: 'http://localhost:8000', + WiDtHs: [375, 1280], + sCoPe: '#app', + sCoPeOpTiOnS: { scroll: true }, + mInHeIgHt: 1024, + eNaBlEJaVaScRiPt: true, + eNaBlELaYoUt: false, + cLiEnTInFo: 'client-info', + eNvIrOnMeNtInFo: 'env-info', + sYnC: true, + tEsTcAsE: 'testCase', + lAbElS: 'label1', + tHtEsTcAsEeXeCuTiOnId: '12345', + }; + + const result = validateSnapshotOptions(options); + expect(result).toEqual({ + clientInfo: 'client-info', + environmentInfo: 'env-info', + name: 'Snapshot 1', + url: 'http://localhost:8000', + widths: [375, 1280], + scope: '#app', + scopeOptions: { scroll: true }, + minHeight: 1024, + enableJavaScript: true, + enableLayout: false, + sync: true, + testCase: 'testCase', + labels: 'label1', + thTestCaseExecutionId: '12345', + }); + }); + }); }); From ff9d9be60ab014b2fe3f81ba788dd42182c285f5 Mon Sep 17 00:00:00 2001 From: Shobhit Kumar Date: Wed, 19 Feb 2025 17:17:32 +0530 Subject: [PATCH 2/4] Fixed lint issues --- packages/core/src/snapshot.js | 32 ++++++++++++++++---------------- packages/core/test/percy.test.js | 12 ++++++------ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index e649171be..b2fc5b3d4 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -166,22 +166,22 @@ function getSnapshotOptions(options, { config, meta }) { }); } const optionMappings = { - 'name': 'name', - 'widths': 'widths', - 'scope': 'scope', - 'scopeoptions': 'scopeOptions', - 'minheight': 'minHeight', - 'enablejavascript': 'enableJavaScript', - 'enablelayout': 'enableLayout', - 'clientinfo': 'clientInfo', - 'environmentinfo': 'environmentInfo', - 'sync': 'sync', - 'testcase': 'testCase', - 'labels': 'labels', - 'thtestcaseexecutionid': 'thTestCaseExecutionId', - 'resources': 'resources', - 'meta': 'meta', - 'snapshot': 'snapshot', + name: 'name', + widths: 'widths', + scope: 'scope', + scopeoptions: 'scopeOptions', + minheight: 'minHeight', + enablejavascript: 'enableJavaScript', + enablelayout: 'enableLayout', + clientinfo: 'clientInfo', + environmentinfo: 'environmentInfo', + sync: 'sync', + testcase: 'testCase', + labels: 'labels', + thtestcaseexecutionid: 'thTestCaseExecutionId', + resources: 'resources', + meta: 'meta', + snapshot: 'snapshot' }; function normalizeOptions(options) { diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js index e6a06219b..c79835173 100644 --- a/packages/core/test/percy.test.js +++ b/packages/core/test/percy.test.js @@ -1725,7 +1725,7 @@ describe('Percy', () => { 'client-info': 'client-info', 'environment-info': 'env-info', 'test-case': 'testCase', - 'th-test-case-execution-id': '12345', + 'th-test-case-execution-id': '12345' }; const result = validateSnapshotOptions(options); @@ -1738,7 +1738,7 @@ describe('Percy', () => { minHeight: 1024, enableJavaScript: true, testCase: 'testCase', - thTestCaseExecutionId: '12345', + thTestCaseExecutionId: '12345' }); }); @@ -1757,7 +1757,7 @@ describe('Percy', () => { Sync: true, TestCase: 'testCase', Labels: 'label1', - ThTestCaseExecutionId: '12345', + ThTestCaseExecutionId: '12345' }; const result = validateSnapshotOptions(options); @@ -1775,7 +1775,7 @@ describe('Percy', () => { sync: true, testCase: 'testCase', labels: 'label1', - thTestCaseExecutionId: '12345', + thTestCaseExecutionId: '12345' }); }); @@ -1794,7 +1794,7 @@ describe('Percy', () => { sYnC: true, tEsTcAsE: 'testCase', lAbElS: 'label1', - tHtEsTcAsEeXeCuTiOnId: '12345', + tHtEsTcAsEeXeCuTiOnId: '12345' }; const result = validateSnapshotOptions(options); @@ -1812,7 +1812,7 @@ describe('Percy', () => { sync: true, testCase: 'testCase', labels: 'label1', - thTestCaseExecutionId: '12345', + thTestCaseExecutionId: '12345' }); }); }); From ac682d3a21f6733a5c7f30a5956d59795cf1c0f4 Mon Sep 17 00:00:00 2001 From: Shobhit Kumar Date: Wed, 19 Feb 2025 18:31:29 +0530 Subject: [PATCH 3/4] fixed naming convention for OPTION_MAPPINGS --- packages/core/src/snapshot.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index b2fc5b3d4..f499f84af 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -165,7 +165,7 @@ function getSnapshotOptions(options, { config, meta }) { } }); } -const optionMappings = { +const OPTION_MAPPINGS = { name: 'name', widths: 'widths', scope: 'scope', @@ -189,7 +189,7 @@ function normalizeOptions(options) { for (const key in options) { const lowerCaseKey = key.toLowerCase().replace(/[-_]/g, ''); - const normalizedKey = optionMappings[lowerCaseKey] ? optionMappings[lowerCaseKey] : key; + const normalizedKey = OPTION_MAPPINGS[lowerCaseKey] ? OPTION_MAPPINGS[lowerCaseKey] : key; normalizedOptions[normalizedKey] = options[key]; } From 9f29f88e2b0309703feedfd4a8e6a47abed31328 Mon Sep 17 00:00:00 2001 From: Shobhit Kumar Date: Wed, 19 Feb 2025 18:50:10 +0530 Subject: [PATCH 4/4] Moved the normalizeOptions function to utils --- packages/core/src/snapshot.js | 32 ++------------------------------ packages/core/src/utils.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index f499f84af..abcfb492f 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -9,7 +9,8 @@ import { yieldTo, snapshotLogName, decodeAndEncodeURLWithLogging, - compareObjectTypes + compareObjectTypes, + normalizeOptions } from './utils.js'; import { JobData } from './wait-for-job.js'; @@ -165,36 +166,7 @@ function getSnapshotOptions(options, { config, meta }) { } }); } -const OPTION_MAPPINGS = { - name: 'name', - widths: 'widths', - scope: 'scope', - scopeoptions: 'scopeOptions', - minheight: 'minHeight', - enablejavascript: 'enableJavaScript', - enablelayout: 'enableLayout', - clientinfo: 'clientInfo', - environmentinfo: 'environmentInfo', - sync: 'sync', - testcase: 'testCase', - labels: 'labels', - thtestcaseexecutionid: 'thTestCaseExecutionId', - resources: 'resources', - meta: 'meta', - snapshot: 'snapshot' -}; - -function normalizeOptions(options) { - const normalizedOptions = {}; - - for (const key in options) { - const lowerCaseKey = key.toLowerCase().replace(/[-_]/g, ''); - const normalizedKey = OPTION_MAPPINGS[lowerCaseKey] ? OPTION_MAPPINGS[lowerCaseKey] : key; - normalizedOptions[normalizedKey] = options[key]; - } - return normalizedOptions; -} // Validates and migrates snapshot options against the correct schema based on provided // properties. Eagerly throws an error when missing a URL for any snapshot, and warns about all // other invalid options which are also scrubbed from the returned migrated options. diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index 6fb3d7e48..76f186386 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -543,3 +543,34 @@ export function compareObjectTypes(obj1, obj2) { return true; } + +const OPTION_MAPPINGS = { + name: 'name', + widths: 'widths', + scope: 'scope', + scopeoptions: 'scopeOptions', + minheight: 'minHeight', + enablejavascript: 'enableJavaScript', + enablelayout: 'enableLayout', + clientinfo: 'clientInfo', + environmentinfo: 'environmentInfo', + sync: 'sync', + testcase: 'testCase', + labels: 'labels', + thtestcaseexecutionid: 'thTestCaseExecutionId', + resources: 'resources', + meta: 'meta', + snapshot: 'snapshot' +}; + +export function normalizeOptions(options) { + const normalizedOptions = {}; + + for (const key in options) { + const lowerCaseKey = key.toLowerCase().replace(/[-_]/g, ''); + const normalizedKey = OPTION_MAPPINGS[lowerCaseKey] ? OPTION_MAPPINGS[lowerCaseKey] : key; + normalizedOptions[normalizedKey] = options[key]; + } + + return normalizedOptions; +}