From 13d00896a4fb873b4e984d559641efa0bd239c0f Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 14 Apr 2021 07:58:28 +0200 Subject: [PATCH] fix(cdk/testing): incorrectly handling ancestor of compound selector Currently the component harness handles the `ancestor` option by prepending it to the `hostSelector` of the harness. This breaks down if the `hostSelector` is a compound selector, because we end up with something like `.parent .a, .b`, rather than `.parent .a, .parent .b`. These changes resolve the issue and add a bit of extra logic to account for the case where the selector might include commas inside quotes which will break us, because we split compound selectors on the comma. The logic is loosely based on my changes from https://github.com/angular/angular/pull/38716. Fixes #22475. --- src/cdk/testing/component-harness.ts | 53 +++++++++++++++++-- .../testing/tests/cross-environment.spec.ts | 9 ++++ .../harnesses/compound-selector-harness.ts | 21 ++++++++ .../tests/harnesses/main-component-harness.ts | 3 ++ .../testing/tests/test-main-component.html | 6 +++ 5 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 src/cdk/testing/tests/harnesses/compound-selector-harness.ts diff --git a/src/cdk/testing/component-harness.ts b/src/cdk/testing/component-harness.ts index 865b47c037cc..455fd5fdd54b 100644 --- a/src/cdk/testing/component-harness.ts +++ b/src/cdk/testing/component-harness.ts @@ -520,9 +520,25 @@ export class HarnessPredicate { /** Gets the selector used to find candidate elements. */ getSelector() { - return this._ancestor.split(',') - .map(part => `${part.trim()} ${this.harnessType.hostSelector}`.trim()) - .join(','); + // We don't have to go through the extra trouble if there are no ancestors. + if (!this._ancestor) { + return (this.harnessType.hostSelector || '').trim(); + } + + const [ancestors, ancestorPlaceholders] = _splitAndEscapeSelector(this._ancestor); + const [selectors, selectorPlaceholders] = + _splitAndEscapeSelector(this.harnessType.hostSelector || ''); + const result: string[] = []; + + // We have to add the ancestor to each part of the host compound selector, otherwise we can get + // incorrect results. E.g. `.ancestor .a, .ancestor .b` vs `.ancestor .a, .b`. + ancestors.forEach(escapedAncestor => { + const ancestor = _restoreSelector(escapedAncestor, ancestorPlaceholders); + return selectors.forEach(escapedSelector => + result.push(`${ancestor} ${_restoreSelector(escapedSelector, selectorPlaceholders)}`)); + }); + + return result.join(', '); } /** Adds base options common to all harness types. */ @@ -560,3 +576,34 @@ function _valueAsString(value: unknown) { return '{...}'; } } + +/** + * Splits up a compound selector into its parts and escapes any quoted content. The quoted content + * has to be escaped, because it can contain commas which will throw throw us off when trying to + * split it. + * @param selector Selector to be split. + * @returns The escaped string where any quoted content is replaced with a placeholder. E.g. + * `[foo="bar"]` turns into `[foo=__cdkPlaceholder-0__]`. Use `_restoreSelector` to restore + * the placeholders. + */ +function _splitAndEscapeSelector(selector: string): [parts: string[], placeholders: string[]] { + const placeholders: string[] = []; + + // Note that the regex doesn't account for nested quotes so something like `"ab'cd'e"` will be + // considered as two blocks. It's a bit of an edge case, but if we find that it's a problem, + // we can make it a bit smarter using a loop. Use this for now since it's more readable and + // compact. More complete implementation: + // https://github.com/angular/angular/blob/bd34bc9e89f18a/packages/compiler/src/shadow_css.ts#L655 + const result = selector.replace(/(["'][^["']*["'])/g, (_, keep) => { + const replaceBy = `__cdkPlaceholder-${placeholders.length}__`; + placeholders.push(keep); + return replaceBy; + }); + + return [result.split(',').map(part => part.trim()), placeholders]; +} + +/** Restores a selector whose content was escaped in `_splitAndEscapeSelector`. */ +function _restoreSelector(selector: string, placeholders: string[]): string { + return selector.replace(/__cdkPlaceholder-(\d+)__/g, (_, index) => placeholders[+index]); +} diff --git a/src/cdk/testing/tests/cross-environment.spec.ts b/src/cdk/testing/tests/cross-environment.spec.ts index e48e23f4c113..5d10c7bf9cc9 100644 --- a/src/cdk/testing/tests/cross-environment.spec.ts +++ b/src/cdk/testing/tests/cross-environment.spec.ts @@ -246,6 +246,15 @@ export function crossEnvironmentSpecs( const subcomps = await harness.directAncestorSelectorSubcomponent(); expect(subcomps.length).toBe(2); }); + + it('should handle a compound selector with an ancestor', async () => { + const elements = await harness.compoundSelectorWithAncestor(); + + expect(await parallel(() => elements.map(element => element.getText()))).toEqual([ + 'Div inside parent', + 'Span inside parent' + ]); + }); }); describe('HarnessPredicate', () => { diff --git a/src/cdk/testing/tests/harnesses/compound-selector-harness.ts b/src/cdk/testing/tests/harnesses/compound-selector-harness.ts new file mode 100644 index 000000000000..c864c3732852 --- /dev/null +++ b/src/cdk/testing/tests/harnesses/compound-selector-harness.ts @@ -0,0 +1,21 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {ComponentHarness, HarnessPredicate} from '../../component-harness'; + +export class CompoundSelectorHarness extends ComponentHarness { + static readonly hostSelector = '.some-div, .some-span'; + + static with(options = {}) { + return new HarnessPredicate(CompoundSelectorHarness, options); + } + + async getText(): Promise { + return (await this.host()).text(); + } +} diff --git a/src/cdk/testing/tests/harnesses/main-component-harness.ts b/src/cdk/testing/tests/harnesses/main-component-harness.ts index a47d060f396e..7be61c6e1549 100644 --- a/src/cdk/testing/tests/harnesses/main-component-harness.ts +++ b/src/cdk/testing/tests/harnesses/main-component-harness.ts @@ -8,6 +8,7 @@ import {ComponentHarness} from '../../component-harness'; import {TestElement, TestKey} from '../../test-element'; +import {CompoundSelectorHarness} from './compound-selector-harness'; import {SubComponentHarness, SubComponentSpecialHarness} from './sub-component-harness'; export class WrongComponentHarness extends ComponentHarness { @@ -81,6 +82,8 @@ export class MainComponentHarness extends ComponentHarness { this.locatorForAll(SubComponentHarness.with({ancestor: '.other, .subcomponents'})); readonly directAncestorSelectorSubcomponent = this.locatorForAll(SubComponentHarness.with({ancestor: '.other >'})); + readonly compoundSelectorWithAncestor = + this.locatorForAll(CompoundSelectorHarness.with({ancestor: '.parent'})); readonly subcomponentHarnessesAndElements = this.locatorForAll('#counter', SubComponentHarness); diff --git a/src/cdk/testing/tests/test-main-component.html b/src/cdk/testing/tests/test-main-component.html index 67a676f32ec1..d95acf798b43 100644 --- a/src/cdk/testing/tests/test-main-component.html +++ b/src/cdk/testing/tests/test-main-component.html @@ -57,6 +57,12 @@

Main Component

+
+
Div inside parent
+
Span inside parent
+
+
Div outside parent
+
Span outside parent