🐛 Fix supressed validation error causing options to not be scrubbed (#…

…853) * 🐛 Fix supressed validation error causing option to not be scrubbed For `allOf`, `anyOf`, and `oneOf`, errors are surpressed since the real error will typically bubble from one of the nested schemas. However, if those nested schemas are simple primative types, there is no additional error. And without an error the validation logic does not scrub the option. This change fixes the issue by adding explicit errors for two of these schemas. In addition, the execute option was relocated so additional snapshots are not allowed to use an object (only a single execute function is allowed for additional snapshots).
percy · Apr 1, 2022 · 079e57a · 079e57a
1 parent 674b290
commit 079e57a
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 15 deletions.
diff --git a/packages/core/src/config.js b/packages/core/src/config.js
@@ -148,6 +148,7 @@ export const snapshotSchema = {
       }
     },
     exec: {
+      error: 'must be a function, function body, or array of functions',
       oneOf: [
         { oneOf: [{ type: 'string' }, { instanceof: 'Function' }] },
         { type: 'array', items: { $ref: '/snapshot#/$defs/exec/oneOf/0' } }
@@ -157,7 +158,17 @@ export const snapshotSchema = {
       type: 'object',
       properties: {
         waitForSelector: { type: 'string' },
-        waitForTimeout: { type: 'integer', minimum: 1, maximum: 30000 },
+        waitForTimeout: { type: 'integer', minimum: 1, maximum: 30000 }
+      }
+    },
+    capture: {
+      type: 'object',
+      allOf: [
+        { $ref: '/snapshot#/$defs/common' },
+        { $ref: '/snapshot#/$defs/precapture' }
+      ],
+      properties: {
+        name: { type: 'string' },
         execute: {
           oneOf: [{ $ref: '/snapshot#/$defs/exec' }, {
             type: 'object',
@@ -169,17 +180,7 @@ export const snapshotSchema = {
               beforeSnapshot: { $ref: '/snapshot#/$defs/exec' }
             }
           }]
-        }
-      }
-    },
-    capture: {
-      type: 'object',
-      allOf: [
-        { $ref: '/snapshot#/$defs/common' },
-        { $ref: '/snapshot#/$defs/precapture' }
-      ],
-      properties: {
-        name: { type: 'string' },
+        },
         additionalSnapshots: {
           type: 'array',
           items: {
@@ -197,7 +198,8 @@ export const snapshotSchema = {
             properties: {
               name: { type: 'string' },
               prefix: { type: 'string' },
-              suffix: { type: 'string' }
+              suffix: { type: 'string' },
+              execute: { $ref: '/snapshot#/$defs/exec' }
             },
             errors: {
               oneOf: ({ params }) => params.passingSchemas
@@ -209,6 +211,7 @@ export const snapshotSchema = {
       }
     },
     predicate: {
+      error: 'must be a pattern or an array of patterns',
       oneOf: [{
         oneOf: [
           { type: 'string' },

diff --git a/packages/core/test/snapshot.test.js b/packages/core/test/snapshot.test.js
@@ -95,15 +95,20 @@ describe('Snapshot', () => {
           'still-not-a-hostname.io/with-a-path',
           'finally.a-real.hostname.org'
         ]
-      }
+      },
+      additionalSnapshots: [{
+        suffix: '-test',
+        execute: { beforeSnapshot: () => {} }
+      }]
     });
 
     expect(logger.stderr).toEqual([
       '[percy] Invalid snapshot options:',
       '[percy] - widths[0]: must be an integer, received a string',
       '[percy] - minHeight: must be <= 2000',
       '[percy] - discovery.allowedHostnames[0]: must not include a protocol',
-      '[percy] - discovery.allowedHostnames[1]: must not include a pathname'
+      '[percy] - discovery.allowedHostnames[1]: must not include a pathname',
+      '[percy] - additionalSnapshots[0].execute: must be a function, function body, or array of functions'
     ]);
   });