Skip to content

Commit 2745baa

Browse files
committed
fix: previous fix for iOS reduced multiple ignore selectors to one
- add new tests - add new images
1 parent 369e7c3 commit 2745baa

8 files changed

Lines changed: 132 additions & 23 deletions

File tree

packages/image-comparison-core/src/methods/rectangles.test.ts

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ describe('rectangles', () => {
3232

3333
mockBrowserInstance = {
3434
execute: mockExecute,
35-
getElementRect: mockGetElementRect
35+
getElementRect: mockGetElementRect,
36+
$: vi.fn(),
37+
$$: vi.fn(),
3638
} as unknown as WebdriverIO.Browser
3739
})
3840

@@ -772,19 +774,20 @@ describe('rectangles', () => {
772774
desktopOptions.browserInstance = mockBrowserInstance
773775
})
774776

775-
it('should resolve elements via raw BCR on desktop and apply DPR', async () => {
777+
it('should resolve elements via raw BCR on desktop and apply DPR without re-querying', async () => {
776778
const mockElement = { elementId: 'el1', selector: '.nav' } as WebdriverIO.Element
777779
mockExecute.mockResolvedValueOnce({ x: 10, y: 20, width: 200, height: 50 })
778780

779781
const result = await determineWebScreenIgnoreRegions(desktopOptions, [mockElement])
780782

781783
expect(mockExecute).toHaveBeenCalledOnce()
784+
expect(mockBrowserInstance.$).not.toHaveBeenCalled()
782785
expect(result).toEqual([
783786
{ x: 20, y: 40, width: 400, height: 100 },
784787
])
785788
})
786789

787-
it('should add DPR-scaled viewport offset on iOS', async () => {
790+
it('should add DPR-scaled viewport offset on iOS and re-query elements via $$', async () => {
788791
const iosDeviceRectangles = {
789792
...baseDeviceRectangles,
790793
viewport: { y: 94, x: 0, width: 390, height: 650 },
@@ -796,18 +799,59 @@ describe('rectangles', () => {
796799
isIOS: true,
797800
}
798801
const mockElement = { elementId: 'el1', selector: '.hero' } as WebdriverIO.Element
802+
const freshElement = { elementId: 'el1-fresh', selector: '.hero' } as unknown as WebdriverIO.Element
803+
vi.mocked(mockBrowserInstance.$$).mockResolvedValueOnce([freshElement] as any)
799804
mockExecute.mockResolvedValueOnce({ x: 0, y: 100, width: 390, height: 200 })
800805

801806
const result = await determineWebScreenIgnoreRegions(iosOptions, [mockElement])
802807

803-
// iOS viewport offset added in CSS space, then floor/ceil for DPR:
804-
// x: floor((0+0)*3)=0, y: floor((100+94)*3)=582
805-
// right: ceil((0+390)*3)=1170, bottom: ceil((100+200+94)*3)=1182
808+
expect(mockBrowserInstance.$$).toHaveBeenCalledWith('.hero')
809+
expect(mockBrowserInstance.$).not.toHaveBeenCalled()
806810
expect(result).toEqual([
807811
{ x: 0, y: 582, width: 1170, height: 600 },
808812
])
809813
})
810814

815+
it('should correctly resolve multiple elements sharing the same selector on iOS', async () => {
816+
const iosDeviceRectangles = {
817+
...baseDeviceRectangles,
818+
viewport: { y: 94, x: 0, width: 390, height: 650 },
819+
}
820+
const iosOptions = {
821+
...desktopOptions,
822+
devicePixelRatio: 1,
823+
deviceRectangles: iosDeviceRectangles,
824+
isIOS: true,
825+
}
826+
const el1 = { elementId: 'a', selector: '.card' } as WebdriverIO.Element
827+
const el2 = { elementId: 'b', selector: '.card' } as WebdriverIO.Element
828+
const el3 = { elementId: 'c', selector: '.card' } as WebdriverIO.Element
829+
830+
const fresh1 = { elementId: 'f1', selector: '.card' } as unknown as WebdriverIO.Element
831+
const fresh2 = { elementId: 'f2', selector: '.card' } as unknown as WebdriverIO.Element
832+
const fresh3 = { elementId: 'f3', selector: '.card' } as unknown as WebdriverIO.Element
833+
vi.mocked(mockBrowserInstance.$$).mockResolvedValueOnce([fresh1, fresh2, fresh3] as any)
834+
835+
mockExecute
836+
.mockResolvedValueOnce({ x: 0, y: 100, width: 390, height: 50 })
837+
.mockResolvedValueOnce({ x: 0, y: 200, width: 390, height: 50 })
838+
.mockResolvedValueOnce({ x: 0, y: 300, width: 390, height: 50 })
839+
840+
const result = await determineWebScreenIgnoreRegions(iosOptions, [[el1, el2, el3]])
841+
842+
// $$ called once for the shared selector, not $ three times
843+
expect(mockBrowserInstance.$$).toHaveBeenCalledTimes(1)
844+
expect(mockBrowserInstance.$$).toHaveBeenCalledWith('.card')
845+
// execute called with each fresh element
846+
expect(mockExecute).toHaveBeenCalledTimes(3)
847+
// Each region has different y (viewport offset 94 added)
848+
expect(result).toEqual([
849+
{ x: 0, y: 194, width: 390, height: 50 },
850+
{ x: 0, y: 294, width: 390, height: 50 },
851+
{ x: 0, y: 394, width: 390, height: 50 },
852+
])
853+
})
854+
811855
it('should add device-pixel viewport offset on Android native web screenshot', async () => {
812856
const androidDeviceRectangles = {
813857
...baseDeviceRectangles,

packages/image-comparison-core/src/methods/rectangles.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,9 +297,36 @@ export async function determineWebScreenIgnoreRegions(
297297
return { x: rect.x, y: rect.y, width: rect.width, height: rect.height }
298298
}
299299
const regionsFromElements: RectanglesOutput[] = []
300-
for (const element of elements) {
301-
const bcr = await browserInstance.execute(rawBcr, element as any) as RectanglesOutput
302-
regionsFromElements.push(bcr)
300+
if (isIOS && elements.length > 0) {
301+
// iOS Safari invalidates element references when the DOM is mutated
302+
// (e.g. by beforeScreenshot CSS injection). Re-query to get fresh refs.
303+
// Use $$ per unique selector so multiple elements sharing the same
304+
// selector (e.g. from a $$ call) each resolve to the correct match.
305+
const selectorCache = new Map<string, WebdriverIO.Element[]>()
306+
const selectorIndex = new Map<string, number>()
307+
308+
for (const element of elements) {
309+
const selector = element.selector as string
310+
311+
if (!selectorCache.has(selector)) {
312+
const fresh = await browserInstance.$$(selector)
313+
selectorCache.set(selector, fresh as unknown as WebdriverIO.Element[])
314+
selectorIndex.set(selector, 0)
315+
}
316+
317+
const idx = selectorIndex.get(selector)!
318+
const cached = selectorCache.get(selector)!
319+
const el = idx < cached.length ? cached[idx] : element
320+
selectorIndex.set(selector, idx + 1)
321+
322+
const bcr = await browserInstance.execute(rawBcr, el as any) as RectanglesOutput
323+
regionsFromElements.push(bcr)
324+
}
325+
} else {
326+
for (const element of elements) {
327+
const bcr = await browserInstance.execute(rawBcr, element as any) as RectanglesOutput
328+
regionsFromElements.push(bcr)
329+
}
303330
}
304331

305332
return [...regions, ...regionsFromElements]

tests/configs/wdio.local.appium.shared.conf.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,23 @@ export const config: Omit<WebdriverIO.Config, 'capabilities'> = {
77
// ===================
88
// Image compare setup
99
// ===================
10+
port: 4723,
1011
services: [
1112
...sharedConfig.services || [],
12-
[
13-
'appium',
14-
{
15-
// This will use the globally installed version of Appium
16-
command: 'appium',
17-
args: {
18-
// This is needed to tell Appium that we can execute local ADB commands
19-
// and to automatically download the latest version of ChromeDriver
20-
relaxedSecurity: true,
21-
// Write the Appium logs to a file in the root of the directory
22-
log: './logs/appium.log',
23-
},
24-
},
25-
],
13+
// [
14+
// 'appium',
15+
// {
16+
// // This will use the globally installed version of Appium
17+
// command: 'appium',
18+
// args: {
19+
// // This is needed to tell Appium that we can execute local ADB commands
20+
// // and to automatically download the latest version of ChromeDriver
21+
// relaxedSecurity: true,
22+
// // Write the Appium logs to a file in the root of the directory
23+
// log: './logs/appium.log',
24+
// },
25+
// },
26+
// ],
2627
[
2728
'visual',
2829
{
468 KB
Loading
479 KB
Loading
476 KB
Loading
514 KB
Loading

tests/specs/mobile.web.spec.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ describe('@wdio/visual-service mobile web', () => {
6060
it(`should compare a screen with ignore elements successful for '${deviceName}' with ${platformName}:${platformVersion} in ${orientation}-mode`, async function () {
6161
skipTest({ test: this, deviceName, platformName, platformVersion, orientation })
6262

63+
// When running a new set of images then first comment out block 1 and 2. Then run the test.
64+
// Then uncomment block 1, check if they fail with `--store-diffs` as an extra argument.
65+
// If so, then uncomment block 2 and check if pass with the same arguments.
66+
// Block 1
6367
await browser.execute(() => {
6468
document.querySelectorAll('.getStarted_Sjon').forEach(link => {
6569
(link as HTMLElement).style.backgroundColor = 'var(--ifm-font-color-base)'
@@ -70,6 +74,7 @@ describe('@wdio/visual-service mobile web', () => {
7074
// We're accepting 0.05%, which is 500 pixels, to be a max difference
7175
const result = await browser.checkScreen(
7276
'ignoredElementsScreenshot', {
77+
// Block 2
7378
ignore: [
7479
await $$('.getStarted_Sjon'),
7580
],
@@ -211,6 +216,38 @@ const skipRules: SkipRule[] = [
211216
orientations: ['landscape', 'portrait'],
212217
reason: 'Elements not visible in the screenshot, no value in testing',
213218
},
219+
{
220+
titleIncludes: 'ignore elements',
221+
deviceName: 'iPhone 13 mini',
222+
platformName: 'iOS',
223+
platformVersions: ['17.5'],
224+
orientations: ['landscape'],
225+
reason: 'Elements not visible in the screenshot, no value in testing',
226+
},
227+
{
228+
titleIncludes: 'ignore elements',
229+
deviceName: 'iPhone 13 Pro',
230+
platformName: 'iOS',
231+
platformVersions: ['16.0'],
232+
orientations: ['landscape'],
233+
reason: 'Elements not visible in the screenshot, no value in testing',
234+
},
235+
{
236+
titleIncludes: 'ignore elements',
237+
deviceName: 'iPhone 14 Pro',
238+
platformName: 'iOS',
239+
platformVersions: ['17.5'],
240+
orientations: ['landscape'],
241+
reason: 'Elements not visible in the screenshot, no value in testing',
242+
},
243+
{
244+
titleIncludes: 'ignore elements',
245+
deviceName: 'iPhone 15 Pro Max',
246+
platformName: 'iOS',
247+
platformVersions: ['18.0'],
248+
orientations: ['landscape'],
249+
reason: 'Elements not visible in the screenshot, no value in testing',
250+
},
214251
]
215252
function skipTest({ test, deviceName, platformName, platformVersion, orientation }: {
216253
test: Mocha.Context

0 commit comments

Comments
 (0)