Fixing Race Condition incase of Screenshot Cmd (#1793)

packages/core/src/snapshot.js

@@ -8,7 +8,8 @@ import {
   hostnameMatches,
   yieldTo,
   snapshotLogName,
-  decodeAndEncodeURLWithLogging
+  decodeAndEncodeURLWithLogging,
+  compareObjectTypes
 } from './utils.js';
 import { JobData } from './wait-for-job.js';
 
@@ -364,9 +365,11 @@ export function createSnapshotsQueue(percy) {
       }
     })
     // snapshots are unique by name and testCase both
-    .handle('find', ({ name, testCase }, snapshot) => (
-      snapshot.testCase === testCase && snapshot.name === name
-    ))
+    .handle('find', ({ name, testCase, tag }, snapshot) => {
+      return snapshot.testCase === testCase &&
+        snapshot.name === name &&
+        compareObjectTypes(tag, snapshot.tag);
+    })
     // when pushed, maybe flush old snapshots or possibly merge with existing snapshots
     .handle('push', (snapshot, existing) => {
       let { name, meta } = snapshot;

packages/core/src/utils.js

@@ -512,3 +512,19 @@ export class DefaultMap extends Map {
     return super.get(key);
   };
 };
+
+export function compareObjectTypes(obj1, obj2) {
+  if (obj1 === obj2) return true; // Handles primitives
+  if (typeof obj1 !== 'object' || typeof obj2 !== 'object' || obj1 === null || obj2 === null) return false;
+
+  const keys1 = Object.keys(obj1);
+  const keys2 = Object.keys(obj2);
+
+  if (keys1.length !== keys2.length) return false;
+
+  for (const key of keys1) {
+    if (!keys2.includes(key) || !compareObjectTypes(obj1[key], obj2[key])) return false;
+  }
+
+  return true;
+}

packages/core/test/utils.test.js

@@ -1,4 +1,4 @@
-import { decodeAndEncodeURLWithLogging, waitForSelectorInsideBrowser } from '../src/utils.js';
+import { decodeAndEncodeURLWithLogging, waitForSelectorInsideBrowser, compareObjectTypes } from '../src/utils.js';
 import { logger, setupTest } from './helpers/index.js';
 import percyLogger from '@percy/logger';
 import Percy from '@percy/core';
@@ -73,4 +73,109 @@ describe('utils', () => {
       expect(error).toEqual(expectedError);
     });
   });
+  describe('compareObjectTypes', () => {
+    describe('Primitive comparisons', () => {
+      it('should return true for identical numbers', () => {
+        expect(compareObjectTypes(42, 42)).toBe(true);
+      });
+
+      it('should return false for different numbers', () => {
+        expect(compareObjectTypes(42, 43)).toBe(false);
+      });
+
+      it('should return true for identical strings', () => {
+        expect(compareObjectTypes('hello', 'hello')).toBe(true);
+      });
+
+      it('should return false for different strings', () => {
+        expect(compareObjectTypes('hello', 'world')).toBe(false);
+      });
+
+      it('should return true for null compared with null', () => {
+        expect(compareObjectTypes(null, null)).toBe(true);
+      });
+
+      it('should return true for undefined compared with undefined', () => {
+        expect(compareObjectTypes(undefined, undefined)).toBe(true);
+      });
+
+      it('should return false for null compared with an object', () => {
+        expect(compareObjectTypes(null, { a: 1 })).toBe(false);
+      });
+    });
+
+    describe('Shallow object comparisons', () => {
+      it('should return true for identical shallow objects', () => {
+        const obj1 = { a: 1, b: 2 };
+        const obj2 = { a: 1, b: 2 };
+        expect(compareObjectTypes(obj1, obj2)).toBe(true);
+      });
+
+      it('should return false for objects with different keys', () => {
+        const obj1 = { a: 1, b: 2 };
+        const obj2 = { a: 1, c: 2 };
+        expect(compareObjectTypes(obj1, obj2)).toBe(false);
+      });
+
+      it('should return false for objects with different values', () => {
+        const obj1 = { a: 1, b: 2 };
+        const obj2 = { a: 1, b: 3 };
+        expect(compareObjectTypes(obj1, obj2)).toBe(false);
+      });
+    });
+
+    describe('Deep object comparisons', () => {
+      it('should return true for deeply nested identical objects', () => {
+        const obj1 = { a: { b: { c: 1 } } };
+        const obj2 = { a: { b: { c: 1 } } };
+        expect(compareObjectTypes(obj1, obj2)).toBe(true);
+      });
+
+      it('should return false for deeply nested objects with different values', () => {
+        const obj1 = { a: { b: { c: 1 } } };
+        const obj2 = { a: { b: { c: 2 } } };
+        expect(compareObjectTypes(obj1, obj2)).toBe(false);
+      });
+    });
+
+    describe('Array comparisons', () => {
+      it('should return true for identical arrays', () => {
+        const obj1 = [1, 2, 3];
+        const obj2 = [1, 2, 3];
+        expect(compareObjectTypes(obj1, obj2)).toBe(true);
+      });
+
+      it('should return false for arrays with different elements', () => {
+        const obj1 = [1, 2, 3];
+        const obj2 = [1, 2, 4];
+        expect(compareObjectTypes(obj1, obj2)).toBe(false);
+      });
+
+      it('should handle mixed structures of arrays and objects', () => {
+        const obj1 = { a: [1, { b: 2 }], c: 3 };
+        const obj2 = { a: [1, { b: 2 }], c: 3 };
+        expect(compareObjectTypes(obj1, obj2)).toBe(true);
+      });
+    });
+
+    describe('Edge cases', () => {
+      it('should return false for mismatched data types', () => {
+        const obj1 = { a: 1 };
+        const obj2 = 'string';
+        expect(compareObjectTypes(obj1, obj2)).toBe(false);
+      });
+
+      it('should return true for empty objects', () => {
+        const obj1 = {};
+        const obj2 = {};
+        expect(compareObjectTypes(obj1, obj2)).toBe(true);
+      });
+
+      it('should return false for objects with different key lengths', () => {
+        const obj1 = { a: 1 };
+        const obj2 = { a: 1, b: 2 };
+        expect(compareObjectTypes(obj1, obj2)).toBe(false);
+      });
+    });
+  });
 });