From c29f57324361f352599db433e7a4b501658a0030 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 8 Nov 2024 10:25:05 -0800 Subject: [PATCH] fix(aria snapshot): assorted fixes (#33512) --- .../src/server/injected/ariaSnapshot.ts | 6 +- .../src/server/injected/yaml.ts | 4 + .../src/utils/isomorphic/ariaSnapshot.ts | 25 ++++-- tests/page/page-aria-snapshot.spec.ts | 33 ++++++++ tests/page/to-match-aria-snapshot.spec.ts | 79 ++++++++++++++++++- .../update-aria-snapshot.spec.ts | 55 ++++++++----- 6 files changed, 169 insertions(+), 33 deletions(-) diff --git a/packages/playwright-core/src/server/injected/ariaSnapshot.ts b/packages/playwright-core/src/server/injected/ariaSnapshot.ts index 8aca49e973..d1aa9ce637 100644 --- a/packages/playwright-core/src/server/injected/ariaSnapshot.ts +++ b/packages/playwright-core/src/server/injected/ariaSnapshot.ts @@ -292,7 +292,7 @@ export function renderAriaTree(ariaNode: AriaNode, options?: { mode?: 'raw' | 'r if (typeof ariaNode === 'string') { if (parentAriaNode && !includeText(parentAriaNode, ariaNode)) return; - const text = renderString(ariaNode); + const text = yamlEscapeValueIfNeeded(renderString(ariaNode)); if (text) lines.push(indent + '- text: ' + text); return; @@ -399,8 +399,8 @@ function textContributesInfo(node: AriaNode, text: string): boolean { if (node.name.length > text.length) return false; - // Figure out if text adds any value. - const substr = longestCommonSubstring(text, node.name); + // Figure out if text adds any value. "longestCommonSubstring" is expensive, so limit strings length. + const substr = (text.length <= 200 && node.name.length <= 200) ? longestCommonSubstring(text, node.name) : ''; let filtered = text; while (substr && filtered.includes(substr)) filtered = filtered.replace(substr, ''); diff --git a/packages/playwright-core/src/server/injected/yaml.ts b/packages/playwright-core/src/server/injected/yaml.ts index 58ebd9d788..977591c1cc 100644 --- a/packages/playwright-core/src/server/injected/yaml.ts +++ b/packages/playwright-core/src/server/injected/yaml.ts @@ -86,6 +86,10 @@ function yamlStringNeedsQuotes(str: string): boolean { if (/^[>|]/.test(str)) return true; + // Strings starting with quotes need quotes + if (/^["']/.test(str)) + return true; + // Strings containing special characters that could cause ambiguity if (/[{}`]/.test(str)) return true; diff --git a/packages/playwright-core/src/utils/isomorphic/ariaSnapshot.ts b/packages/playwright-core/src/utils/isomorphic/ariaSnapshot.ts index be7cd134f6..aae5fb3e03 100644 --- a/packages/playwright-core/src/utils/isomorphic/ariaSnapshot.ts +++ b/packages/playwright-core/src/utils/isomorphic/ariaSnapshot.ts @@ -138,14 +138,18 @@ class KeyParser { return this._pos >= this._length; } + private _isWhitespace() { + return !this._eof() && /\s/.test(this._peek()); + } + private _skipWhitespace() { - while (!this._eof() && /\s/.test(this._peek())) + while (this._isWhitespace()) this._pos++; } - private _readIdentifier(): string { + private _readIdentifier(type: 'role' | 'attribute'): string { if (this._eof()) - this._throwError('Unexpected end of input when expecting identifier'); + this._throwError(`Unexpected end of input when expecting ${type}`); const start = this._pos; while (!this._eof() && /[a-zA-Z]/.test(this._peek())) this._pos++; @@ -178,6 +182,7 @@ class KeyParser { private _readRegex(): string { let result = ''; let escaped = false; + let insideClass = false; while (!this._eof()) { const ch = this._next(); if (escaped) { @@ -186,8 +191,14 @@ class KeyParser { } else if (ch === '\\') { escaped = true; result += ch; - } else if (ch === '/') { + } else if (ch === '/' && !insideClass) { return result; + } else if (ch === '[') { + insideClass = true; + result += ch; + } else if (ch === ']' && insideClass) { + result += ch; + insideClass = false; } else { result += ch; } @@ -218,14 +229,14 @@ class KeyParser { this._next(); this._skipWhitespace(); errorPos = this._pos; - const flagName = this._readIdentifier(); + const flagName = this._readIdentifier('attribute'); this._skipWhitespace(); let flagValue = ''; if (this._peek() === '=') { this._next(); this._skipWhitespace(); errorPos = this._pos; - while (this._peek() !== ']' && !this._eof()) + while (this._peek() !== ']' && !this._isWhitespace() && !this._eof()) flagValue += this._next(); } this._skipWhitespace(); @@ -243,7 +254,7 @@ class KeyParser { _parse(): AriaTemplateNode { this._skipWhitespace(); - const role = this._readIdentifier() as AriaTemplateRoleNode['role']; + const role = this._readIdentifier('role') as AriaTemplateRoleNode['role']; this._skipWhitespace(); const name = this._readStringOrRegex() || ''; const result: AriaTemplateRoleNode = { kind: 'role', role, name }; diff --git a/tests/page/page-aria-snapshot.spec.ts b/tests/page/page-aria-snapshot.spec.ts index e9156ebad8..22ae280b4d 100644 --- a/tests/page/page-aria-snapshot.spec.ts +++ b/tests/page/page-aria-snapshot.spec.ts @@ -459,3 +459,36 @@ it('should be ok with circular ownership', async ({ page }) => { - region: Hello `); }); + +it('should escape yaml text in text nodes', async ({ page }) => { + await page.setContent(` +
+ one: link1 "two link2 'three link3 \`four +
+ `); + + await checkAndMatchSnapshot(page.locator('body'), ` + - group: + - text: "one:" + - link "link1" + - text: "\\\"two" + - link "link2" + - text: "'three" + - link "link3" + - text: "\`four" + `); +}); + +it.fixme('should handle long strings', async ({ page }) => { + await page.setContent(` + +
${'a'.repeat(100000)}
+ + `); + + const trimmed = 'a'.repeat(1000); + await checkAndMatchSnapshot(page.locator('body'), ` + - link "${trimmed}": + - region: "${trimmed}" + `); +}); diff --git a/tests/page/to-match-aria-snapshot.spec.ts b/tests/page/to-match-aria-snapshot.spec.ts index 0cc9a72c46..40709909ce 100644 --- a/tests/page/to-match-aria-snapshot.spec.ts +++ b/tests/page/to-match-aria-snapshot.spec.ts @@ -76,10 +76,30 @@ test('should match complex', async ({ page }) => { }); test('should match regex', async ({ page }) => { - await page.setContent(`

Issues 12

`); - await expect(page.locator('body')).toMatchAriaSnapshot(` - - heading ${/Issues \d+/} - `); + { + await page.setContent(`

Issues 12

`); + await expect(page.locator('body')).toMatchAriaSnapshot(` + - heading ${/Issues \d+/} + `); + } + { + await page.setContent(`

Issues 1/2

`); + await expect(page.locator('body')).toMatchAriaSnapshot(` + - heading ${/Issues 1[/]2/} + `); + } + { + await page.setContent(`

Issues 1[

`); + await expect(page.locator('body')).toMatchAriaSnapshot(` + - heading ${/Issues 1\[/} + `); + } + { + await page.setContent(`

Issues 1]]2

`); + await expect(page.locator('body')).toMatchAriaSnapshot(` + - heading ${/Issues 1[\]]]2/} + `); + } }); test('should allow text nodes', async ({ page }) => { @@ -472,6 +492,26 @@ test('should unpack escaped names', async ({ page }) => { - 'button "Click '' me"' `); } + + { + await page.setContent(` +

heading "name" [level=1]

+ `); + await expect(page.locator('body')).toMatchAriaSnapshot(` + - heading "heading \\"name\\" [level=1]" [level=1] + `); + } + + { + await page.setContent(` +

heading \\" [level=2]

+ `); + await expect(page.locator('body')).toMatchAriaSnapshot(` + - | + heading "heading \\\\\\" [level=2]" [ + level = 1 ] + `); + } }); test('should report error in YAML', async ({ page }) => { @@ -599,3 +639,34 @@ test('call log should contain actual snapshot', async ({ page }) => { expect(stripAnsi(error.message)).toContain(`- unexpected value "- heading "todos" [level=1]"`); }); + +test.fixme('should normalize whitespace when matching accessible name', async ({ page }) => { + await page.setContent(` + + `); + await expect(page.locator('body')).toMatchAriaSnapshot(` + - | + button "hello + world" + `); +}); + +test('should parse attributes', async ({ page }) => { + { + await page.setContent(` + + `); + await expect(page.locator('body')).toMatchAriaSnapshot(` + - button [pressed=mixed ] + `); + } + + { + await page.setContent(` +

hello world

+ `); + await expect(page.locator('body')).not.toMatchAriaSnapshot(` + - heading [level = -3 ] + `); + } +}); diff --git a/tests/playwright-test/update-aria-snapshot.spec.ts b/tests/playwright-test/update-aria-snapshot.spec.ts index e8999052a1..e6d76da282 100644 --- a/tests/playwright-test/update-aria-snapshot.spec.ts +++ b/tests/playwright-test/update-aria-snapshot.spec.ts @@ -20,6 +20,10 @@ import { execSync } from 'child_process'; test.describe.configure({ mode: 'parallel' }); +function trimPatch(patch: string) { + return patch.split('\n').map(line => line.trimEnd()).join('\n'); +} + test('should update snapshot with the update-snapshots flag', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ 'a.spec.ts': ` @@ -36,7 +40,7 @@ test('should update snapshot with the update-snapshots flag', async ({ runInline expect(result.exitCode).toBe(0); const patchPath = testInfo.outputPath('test-results/rebaselines.patch'); const data = fs.readFileSync(patchPath, 'utf-8'); - expect(data).toBe(`--- a/a.spec.ts + expect(trimPatch(data)).toBe(`--- a/a.spec.ts +++ b/a.spec.ts @@ -3,7 +3,7 @@ test('test', async ({ page }) => { @@ -46,7 +50,7 @@ test('should update snapshot with the update-snapshots flag', async ({ runInline + - heading "hello" [level=1] \`); }); - + `); expect(stripAnsi(result.output).replace(/\\/g, '/')).toContain(`New baselines created for: @@ -83,7 +87,7 @@ test('should update missing snapshots', async ({ runInlineTest }, testInfo) => { const patchPath = testInfo.outputPath('test-results/rebaselines.patch'); const data = fs.readFileSync(patchPath, 'utf-8'); - expect(data).toBe(`--- a/a.spec.ts + expect(trimPatch(data)).toBe(`--- a/a.spec.ts +++ b/a.spec.ts @@ -2,6 +2,8 @@ import { test, expect } from '@playwright/test'; @@ -94,7 +98,7 @@ test('should update missing snapshots', async ({ runInlineTest }, testInfo) => { + - heading "hello" [level=1] + \`); }); - + `); execSync(`patch -p1 < ${patchPath}`, { cwd: testInfo.outputPath() }); @@ -127,7 +131,7 @@ test('should generate baseline with regex', async ({ runInlineTest }, testInfo) expect(result.exitCode).toBe(0); const patchPath = testInfo.outputPath('test-results/rebaselines.patch'); const data = fs.readFileSync(patchPath, 'utf-8'); - expect(data).toBe(`--- a/a.spec.ts + expect(trimPatch(data)).toBe(`--- a/a.spec.ts +++ b/a.spec.ts @@ -13,6 +13,18 @@
  • /Regex 1/
    • @@ -148,7 +152,7 @@ test('should generate baseline with regex', async ({ runInlineTest }, testInfo) + - listitem: /\\\\/Regex \\\\d+[hmsp]+\\\\// + \`); }); - + `); execSync(`patch -p1 < ${patchPath}`, { cwd: testInfo.outputPath() }); @@ -162,6 +166,10 @@ test('should generate baseline with special characters', async ({ runInlineTest import { test, expect } from '@playwright/test'; test('test', async ({ page }) => { await page.setContent(\`\`); - await expect(page.locator('body')).toMatchAriaSnapshot(\`\`); + await expect(page.locator('body')).toMatchAriaSnapshot(\` + - list: ++ - group: ++ - text: "one:" ++ - link "link1" ++ - text: "\\\\\"two" ++ - link "link2" ++ - text: "'three" ++ - link "link3" ++ - text: "\\\`four" ++ - heading "heading \\\\"name\\\\" [level=1]" [level=1] + - 'button "Click: me"' + - 'button /Click: \\\\d+/' + - button "Click ' me" @@ -202,7 +219,7 @@ test('should generate baseline with special characters', async ({ runInlineTest + - listitem: \"Item {a: b}\" + \`); }); - + `); execSync(`patch -p1 < ${patchPath}`, { cwd: testInfo.outputPath() }); @@ -234,10 +251,10 @@ test('should update missing snapshots in tsx', async ({ runInlineTest }, testInf expect(result.exitCode).toBe(0); const patchPath = testInfo.outputPath('test-results/rebaselines.patch'); const data = fs.readFileSync(patchPath, 'utf-8'); - expect(data).toBe(`--- a/src/button.test.tsx + expect(trimPatch(data)).toBe(`--- a/src/button.test.tsx +++ b/src/button.test.tsx @@ -4,6 +4,8 @@ - + test('pass', async ({ mount }) => { const component = await mount(); - await expect(component).toMatchAriaSnapshot(\`\`); @@ -245,7 +262,7 @@ test('should update missing snapshots in tsx', async ({ runInlineTest }, testInf + - button \"Button\" + \`); }); - + `); execSync(`patch -p1 < ${patchPath}`, { cwd: testInfo.outputPath() }); @@ -296,10 +313,10 @@ test('should update multiple files', async ({ runInlineTest }, testInfo) => { const patchPath = testInfo.outputPath('test-results/rebaselines.patch'); const data = fs.readFileSync(patchPath, 'utf-8'); - expect(data).toBe(`--- a/src/button-1.test.tsx + expect(trimPatch(data)).toBe(`--- a/src/button-1.test.tsx +++ b/src/button-1.test.tsx @@ -4,6 +4,8 @@ - + test('pass 1', async ({ mount }) => { const component = await mount(); - await expect(component).toMatchAriaSnapshot(\`\`); @@ -307,12 +324,12 @@ test('should update multiple files', async ({ runInlineTest }, testInfo) => { + - button \"Button\" + \`); }); - + --- a/src/button-2.test.tsx +++ b/src/button-2.test.tsx @@ -4,6 +4,8 @@ - + test('pass 2', async ({ mount }) => { const component = await mount(); - await expect(component).toMatchAriaSnapshot(\`\`); @@ -320,7 +337,7 @@ test('should update multiple files', async ({ runInlineTest }, testInfo) => { + - button \"Button\" + \`); }); - + `); execSync(`patch -p1 < ${patchPath}`, { cwd: testInfo.outputPath() }); @@ -342,7 +359,7 @@ test('should generate baseline for input values', async ({ runInlineTest }, test expect(result.exitCode).toBe(0); const patchPath = testInfo.outputPath('test-results/rebaselines.patch'); const data = fs.readFileSync(patchPath, 'utf-8'); - expect(data).toBe(`--- a/a.spec.ts + expect(trimPatch(data)).toBe(`--- a/a.spec.ts +++ b/a.spec.ts @@ -2,6 +2,8 @@ import { test, expect } from '@playwright/test'; @@ -353,7 +370,7 @@ test('should generate baseline for input values', async ({ runInlineTest }, test + - textbox: hello world + \`); }); - + `); execSync(`patch -p1 < ${patchPath}`, { cwd: testInfo.outputPath() });