From 02fc89cd75cf6bf0f26854fb96a1d585c3d1c5a7 Mon Sep 17 00:00:00 2001 From: Shobhit Kumar Date: Tue, 25 Feb 2025 14:15:34 +0530 Subject: [PATCH 1/7] Modified getRegionsByElements to ignore class --- percy/providers/genericProvider.js | 5 ++-- test/percy/providers/genericProvider.test.mjs | 29 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/percy/providers/genericProvider.js b/percy/providers/genericProvider.js index 4d4bacb..0f9cc67 100644 --- a/percy/providers/genericProvider.js +++ b/percy/providers/genericProvider.js @@ -245,9 +245,8 @@ class GenericProvider { async getRegionsByElements(elementsArray, elements) { for (let index = 0; index < elements.length; index++) { try { - const type = await elements[index].getAttribute('class'); - const selector = `element: ${index} ${type}`; - + const selector = `element: ${index}`; + const ignoredRegion = await this.getRegionObject(selector, elements[index]); elementsArray.push(ignoredRegion); } catch (e) { diff --git a/test/percy/providers/genericProvider.test.mjs b/test/percy/providers/genericProvider.test.mjs index 416f352..54522b0 100644 --- a/test/percy/providers/genericProvider.test.mjs +++ b/test/percy/providers/genericProvider.test.mjs @@ -180,6 +180,35 @@ describe('GenericProvider', () => { // expect(getRegionObjectSpy).not.toHaveBeenCalled(); expect(elementsArray).toEqual([]); }); + + it('should add regions for valid elements', async () => { + const elementsArray = []; + const mockElements = [ + { + getRect: jasmine.createSpy().and.resolveTo({ x: 10, y: 20, width: 100, height: 50 }) + }, + { + getRect: jasmine.createSpy().and.resolveTo({ x: 30, y: 40, width: 200, height: 60 }) + } + ]; + + await provider.getRegionsByElements.call({ driver, getRegionObject: getRegionObjectSpy }, elementsArray, mockElements); + + expect(getRegionObjectSpy).toHaveBeenCalledTimes(2); + expect(getRegionObjectSpy.calls.argsFor(0)[0]).toBe('element: 0'); + expect(getRegionObjectSpy.calls.argsFor(1)[0]).toBe('element: 1'); + expect(elementsArray).toEqual([{}, {}]); + }); + + it('should handle empty elements array', async () => { + const elementsArray = []; + const mockElements = []; + + await provider.getRegionsByElements.call({ driver, getRegionObject: getRegionObjectSpy }, elementsArray, mockElements); + + expect(getRegionObjectSpy).not.toHaveBeenCalled(); + expect(elementsArray).toEqual([]); + }); }); describe('getRegionsByLocation function', () => { From 6edf68d25184e2e47d39c52db0d9d9c7d8d09bdf Mon Sep 17 00:00:00 2001 From: Shobhit Kumar Date: Tue, 25 Feb 2025 14:16:22 +0530 Subject: [PATCH 2/7] Fixed lint --- percy/providers/genericProvider.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/percy/providers/genericProvider.js b/percy/providers/genericProvider.js index 0f9cc67..82bf7f5 100644 --- a/percy/providers/genericProvider.js +++ b/percy/providers/genericProvider.js @@ -246,7 +246,7 @@ class GenericProvider { for (let index = 0; index < elements.length; index++) { try { const selector = `element: ${index}`; - + const ignoredRegion = await this.getRegionObject(selector, elements[index]); elementsArray.push(ignoredRegion); } catch (e) { From 1e5a10f19333f51893d71031f4885f5db8953c82 Mon Sep 17 00:00:00 2001 From: Shobhit Kumar Date: Tue, 25 Feb 2025 17:21:38 +0530 Subject: [PATCH 3/7] Bifuracted code to handle android and ios selectors seperately --- percy/providers/genericProvider.js | 21 +- test/percy/providers/genericProvider.test.mjs | 219 +++++++++++++++--- 2 files changed, 211 insertions(+), 29 deletions(-) diff --git a/percy/providers/genericProvider.js b/percy/providers/genericProvider.js index 82bf7f5..124a67d 100644 --- a/percy/providers/genericProvider.js +++ b/percy/providers/genericProvider.js @@ -245,9 +245,24 @@ class GenericProvider { async getRegionsByElements(elementsArray, elements) { for (let index = 0; index < elements.length; index++) { try { - const selector = `element: ${index}`; - - const ignoredRegion = await this.getRegionObject(selector, elements[index]); + let identifier; + const element = elements[index]; + + const capabilities = await this.driver.getCapabilities(); + const platformName = (capabilities.platformName || '').toLowerCase(); + + if (platformName === 'android') { + // Android identifiers + identifier = await element.getAttribute('resource-id') || + await element.getAttribute('class'); + } else if(platformName === 'ios') { + // iOS identifiers + identifier = await element.getAttribute('name') || + await element.getAttribute('type'); + } + + const selector = `element: ${index} ${identifier ? `${identifier}` : ''}`.trim(); + const ignoredRegion = await this.getRegionObject(selector, element); elementsArray.push(ignoredRegion); } catch (e) { log.info(`Correct Mobile Element not passed at index ${index}.`); diff --git a/test/percy/providers/genericProvider.test.mjs b/test/percy/providers/genericProvider.test.mjs index 54522b0..24f286e 100644 --- a/test/percy/providers/genericProvider.test.mjs +++ b/test/percy/providers/genericProvider.test.mjs @@ -152,22 +152,43 @@ describe('GenericProvider', () => { describe('getRegionsByElements', () => { let getRegionObjectSpy; let mockElement; + let mockDriver; beforeEach(() => { getRegionObjectSpy = spyOn(provider, 'getRegionObject').and.resolveTo({}); - mockElement = { - getAttribute: jasmine.createSpy().and.returnValue('some-class') + mockDriver = { + getCapabilities: jasmine.createSpy('getCapabilities') }; }); it('should get regions for each element', async () => { + // Set up mock driver with platform capability + mockDriver.getCapabilities.and.resolveTo({ platformName: 'android' }); + const elementsArray = []; + mockElement = { + getAttribute: jasmine.createSpy('getAttribute') + .and.callFake(attr => { + if (attr === 'resource-id') return 'test_id'; + if (attr === 'class') return 'android.widget.Button'; + return null; + }) + }; const elements = [mockElement, mockElement, mockElement]; - await provider.getRegionsByElements.call({ driver, getRegionObject: getRegionObjectSpy }, elementsArray, elements); + await provider.getRegionsByElements.call( + { driver: mockDriver, getRegionObject: getRegionObjectSpy }, + elementsArray, + elements + ); expect(getRegionObjectSpy).toHaveBeenCalledTimes(3); expect(elementsArray).toEqual([{}, {}, {}]); + + // Verify each call had correct selector + for (let i = 0; i < 3; i++) { + expect(getRegionObjectSpy.calls.argsFor(i)[0]).toBe(`element: ${i} test_id`); + } }); it('should ignore when error', async () => { @@ -181,33 +202,179 @@ describe('GenericProvider', () => { expect(elementsArray).toEqual([]); }); - it('should add regions for valid elements', async () => { - const elementsArray = []; - const mockElements = [ - { - getRect: jasmine.createSpy().and.resolveTo({ x: 10, y: 20, width: 100, height: 50 }) - }, - { - getRect: jasmine.createSpy().and.resolveTo({ x: 30, y: 40, width: 200, height: 60 }) - } - ]; + describe('Platform specific tests', () => { + it('should handle unknown platform', async () => { + mockDriver.getCapabilities.and.resolveTo({ platformName: 'unknown' }); + const elementsArray = []; + mockElement = { + getAttribute: jasmine.createSpy('getAttribute') + .and.returnValue('some-value') + }; + + await provider.getRegionsByElements.call( + { driver: mockDriver, getRegionObject: getRegionObjectSpy }, + elementsArray, + [mockElement] + ); + + expect(getRegionObjectSpy).toHaveBeenCalledTimes(1); + expect(getRegionObjectSpy.calls.argsFor(0)[0]).toBe('element: 0'); + expect(elementsArray).toEqual([{}]); + }); - await provider.getRegionsByElements.call({ driver, getRegionObject: getRegionObjectSpy }, elementsArray, mockElements); + it('should handle missing platformName in capabilities', async () => { + mockDriver.getCapabilities.and.resolveTo({}); + const elementsArray = []; + mockElement = { + getAttribute: jasmine.createSpy('getAttribute') + .and.returnValue('some-value') + }; + + await provider.getRegionsByElements.call( + { driver: mockDriver, getRegionObject: getRegionObjectSpy }, + elementsArray, + [mockElement] + ); + + expect(getRegionObjectSpy).toHaveBeenCalledTimes(1); + expect(getRegionObjectSpy.calls.argsFor(0)[0]).toBe('element: 0'); + expect(elementsArray).toEqual([{}]); + }); + describe('Android', () => { + beforeEach(() => { + mockDriver.getCapabilities.and.resolveTo({ platformName: 'android' }); + }); + + it('should use resource-id as primary identifier', async () => { + const elementsArray = []; + mockElement = { + getAttribute: jasmine.createSpy('getAttribute') + .and.callFake(attr => { + if (attr === 'resource-id') return 'test_resource_id'; + if (attr === 'class') return 'android.widget.Button'; + return null; + }) + }; + + await provider.getRegionsByElements.call( + { driver: mockDriver, getRegionObject: getRegionObjectSpy }, + elementsArray, + [mockElement] + ); + + expect(getRegionObjectSpy).toHaveBeenCalledTimes(1); + expect(getRegionObjectSpy.calls.argsFor(0)[0]).toBe('element: 0 test_resource_id'); + expect(elementsArray).toEqual([{}]); + }); + + it('should fallback to class when resource-id is not available', async () => { + const elementsArray = []; + mockElement = { + getAttribute: jasmine.createSpy('getAttribute') + .and.callFake(attr => { + if (attr === 'resource-id') return null; + if (attr === 'class') return 'android.widget.Button'; + return null; + }) + }; + + await provider.getRegionsByElements.call( + { driver: mockDriver, getRegionObject: getRegionObjectSpy }, + elementsArray, + [mockElement] + ); + + expect(getRegionObjectSpy).toHaveBeenCalledTimes(1); + expect(getRegionObjectSpy.calls.argsFor(0)[0]).toBe('element: 0 android.widget.Button'); + expect(elementsArray).toEqual([{}]); + }); + }); - expect(getRegionObjectSpy).toHaveBeenCalledTimes(2); - expect(getRegionObjectSpy.calls.argsFor(0)[0]).toBe('element: 0'); - expect(getRegionObjectSpy.calls.argsFor(1)[0]).toBe('element: 1'); - expect(elementsArray).toEqual([{}, {}]); + describe('iOS', () => { + beforeEach(() => { + mockDriver.getCapabilities.and.resolveTo({ platformName: 'ios' }); + }); + + it('should use name as primary identifier', async () => { + const elementsArray = []; + mockElement = { + getAttribute: jasmine.createSpy('getAttribute') + .and.callFake(attr => { + if (attr === 'name') return 'TestButton'; + if (attr === 'type') return 'XCUIElementTypeButton'; + return null; + }) + }; + + await provider.getRegionsByElements.call( + { driver: mockDriver, getRegionObject: getRegionObjectSpy }, + elementsArray, + [mockElement] + ); + + expect(getRegionObjectSpy).toHaveBeenCalledTimes(1); + expect(getRegionObjectSpy.calls.argsFor(0)[0]).toBe('element: 0 TestButton'); + expect(elementsArray).toEqual([{}]); + }); + + it('should fallback to type when name is not available', async () => { + const elementsArray = []; + mockElement = { + getAttribute: jasmine.createSpy('getAttribute') + .and.callFake(attr => { + if (attr === 'name') return null; + if (attr === 'type') return 'XCUIElementTypeButton'; + return null; + }) + }; + + await provider.getRegionsByElements.call( + { driver: mockDriver, getRegionObject: getRegionObjectSpy }, + elementsArray, + [mockElement] + ); + + expect(getRegionObjectSpy).toHaveBeenCalledTimes(1); + expect(getRegionObjectSpy.calls.argsFor(0)[0]).toBe('element: 0 XCUIElementTypeButton'); + expect(elementsArray).toEqual([{}]); + }); + }); }); - it('should handle empty elements array', async () => { - const elementsArray = []; - const mockElements = []; - - await provider.getRegionsByElements.call({ driver, getRegionObject: getRegionObjectSpy }, elementsArray, mockElements); - - expect(getRegionObjectSpy).not.toHaveBeenCalled(); - expect(elementsArray).toEqual([]); + describe('Error handling', () => { + it('should handle elements with no identifiers', async () => { + mockDriver.getCapabilities.and.resolveTo({ platformName: 'android' }); + const elementsArray = []; + mockElement = { + getAttribute: jasmine.createSpy('getAttribute').and.returnValue(null) + }; + + await provider.getRegionsByElements.call( + { driver: mockDriver, getRegionObject: getRegionObjectSpy }, + elementsArray, + [mockElement] + ); + + expect(getRegionObjectSpy).toHaveBeenCalledTimes(1); + expect(getRegionObjectSpy.calls.argsFor(0)[0]).toBe('element: 0'); + expect(elementsArray).toEqual([{}]); + }); + + it('should handle capability errors', async () => { + mockDriver.getCapabilities.and.rejectWith(new Error('Capabilities not found')); + const elementsArray = []; + mockElement = { + getAttribute: jasmine.createSpy('getAttribute') + }; + + await provider.getRegionsByElements.call( + { driver: mockDriver, getRegionObject: getRegionObjectSpy }, + elementsArray, + [mockElement] + ); + + expect(elementsArray).toEqual([]); + }); }); }); From d6c63d4b7a4b741dfbdfe2984758a0296752e1bf Mon Sep 17 00:00:00 2001 From: Shobhit Kumar Date: Tue, 25 Feb 2025 17:24:07 +0530 Subject: [PATCH 4/7] Fixed lint --- percy/providers/genericProvider.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/percy/providers/genericProvider.js b/percy/providers/genericProvider.js index 124a67d..4df033b 100644 --- a/percy/providers/genericProvider.js +++ b/percy/providers/genericProvider.js @@ -247,20 +247,20 @@ class GenericProvider { try { let identifier; const element = elements[index]; - + const capabilities = await this.driver.getCapabilities(); const platformName = (capabilities.platformName || '').toLowerCase(); - + if (platformName === 'android') { // Android identifiers - identifier = await element.getAttribute('resource-id') || + identifier = await element.getAttribute('resource-id') || await element.getAttribute('class'); - } else if(platformName === 'ios') { + } else if (platformName === 'ios') { // iOS identifiers - identifier = await element.getAttribute('name') || + identifier = await element.getAttribute('name') || await element.getAttribute('type'); } - + const selector = `element: ${index} ${identifier ? `${identifier}` : ''}`.trim(); const ignoredRegion = await this.getRegionObject(selector, element); elementsArray.push(ignoredRegion); From b370a232c9f634eb4f6c0ff232023e650547319c Mon Sep 17 00:00:00 2001 From: Shobhit Kumar Date: Thu, 27 Feb 2025 17:31:23 +0530 Subject: [PATCH 5/7] changes platformName to PLATFORM_NAME --- percy/providers/genericProvider.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/percy/providers/genericProvider.js b/percy/providers/genericProvider.js index 4df033b..cdcae6c 100644 --- a/percy/providers/genericProvider.js +++ b/percy/providers/genericProvider.js @@ -249,13 +249,13 @@ class GenericProvider { const element = elements[index]; const capabilities = await this.driver.getCapabilities(); - const platformName = (capabilities.platformName || '').toLowerCase(); + const PLATFORM_NAME = (capabilities.platformName || '').toLowerCase(); - if (platformName === 'android') { + if (PLATFORM_NAME === 'android') { // Android identifiers identifier = await element.getAttribute('resource-id') || await element.getAttribute('class'); - } else if (platformName === 'ios') { + } else if (PLATFORM_NAME === 'ios') { // iOS identifiers identifier = await element.getAttribute('name') || await element.getAttribute('type'); From 8e72a399781d2975f9039263f907b9749e2885cb Mon Sep 17 00:00:00 2001 From: Shobhit Kumar Date: Thu, 27 Feb 2025 17:48:46 +0530 Subject: [PATCH 6/7] removed fallback for platformName --- percy/providers/genericProvider.js | 2 +- test/percy/providers/genericProvider.test.mjs | 18 ------------------ 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/percy/providers/genericProvider.js b/percy/providers/genericProvider.js index cdcae6c..f38374e 100644 --- a/percy/providers/genericProvider.js +++ b/percy/providers/genericProvider.js @@ -249,7 +249,7 @@ class GenericProvider { const element = elements[index]; const capabilities = await this.driver.getCapabilities(); - const PLATFORM_NAME = (capabilities.platformName || '').toLowerCase(); + const PLATFORM_NAME = capabilities.platformName.toLowerCase(); if (PLATFORM_NAME === 'android') { // Android identifiers diff --git a/test/percy/providers/genericProvider.test.mjs b/test/percy/providers/genericProvider.test.mjs index 24f286e..982fcde 100644 --- a/test/percy/providers/genericProvider.test.mjs +++ b/test/percy/providers/genericProvider.test.mjs @@ -222,24 +222,6 @@ describe('GenericProvider', () => { expect(elementsArray).toEqual([{}]); }); - it('should handle missing platformName in capabilities', async () => { - mockDriver.getCapabilities.and.resolveTo({}); - const elementsArray = []; - mockElement = { - getAttribute: jasmine.createSpy('getAttribute') - .and.returnValue('some-value') - }; - - await provider.getRegionsByElements.call( - { driver: mockDriver, getRegionObject: getRegionObjectSpy }, - elementsArray, - [mockElement] - ); - - expect(getRegionObjectSpy).toHaveBeenCalledTimes(1); - expect(getRegionObjectSpy.calls.argsFor(0)[0]).toBe('element: 0'); - expect(elementsArray).toEqual([{}]); - }); describe('Android', () => { beforeEach(() => { mockDriver.getCapabilities.and.resolveTo({ platformName: 'android' }); From 51d856b01bb965ecdd886b66e80b5dccbb8f6495 Mon Sep 17 00:00:00 2001 From: Shobhit Kumar Date: Thu, 27 Feb 2025 17:51:54 +0530 Subject: [PATCH 7/7] Fixed lint --- percy/providers/genericProvider.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/percy/providers/genericProvider.js b/percy/providers/genericProvider.js index f38374e..7a7d54e 100644 --- a/percy/providers/genericProvider.js +++ b/percy/providers/genericProvider.js @@ -249,13 +249,13 @@ class GenericProvider { const element = elements[index]; const capabilities = await this.driver.getCapabilities(); - const PLATFORM_NAME = capabilities.platformName.toLowerCase(); + const platformName = capabilities.platformName.toLowerCase(); - if (PLATFORM_NAME === 'android') { + if (platformName === 'android') { // Android identifiers identifier = await element.getAttribute('resource-id') || await element.getAttribute('class'); - } else if (PLATFORM_NAME === 'ios') { + } else if (platformName === 'ios') { // iOS identifiers identifier = await element.getAttribute('name') || await element.getAttribute('type');