Skip to content

Commit 032feef

Browse files
authored
fix(cdk/testing): incorrectly handling ancestor of compound selector (#22476)
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 angular/angular#38716. Fixes #22475.
1 parent 8dcc94d commit 032feef

File tree

6 files changed

+123
-3
lines changed

6 files changed

+123
-3
lines changed

src/cdk/testing/component-harness.ts

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -520,9 +520,25 @@ export class HarnessPredicate<T extends ComponentHarness> {
520520

521521
/** Gets the selector used to find candidate elements. */
522522
getSelector() {
523-
return this._ancestor.split(',')
524-
.map(part => `${part.trim()} ${this.harnessType.hostSelector}`.trim())
525-
.join(',');
523+
// We don't have to go through the extra trouble if there are no ancestors.
524+
if (!this._ancestor) {
525+
return (this.harnessType.hostSelector || '').trim();
526+
}
527+
528+
const [ancestors, ancestorPlaceholders] = _splitAndEscapeSelector(this._ancestor);
529+
const [selectors, selectorPlaceholders] =
530+
_splitAndEscapeSelector(this.harnessType.hostSelector || '');
531+
const result: string[] = [];
532+
533+
// We have to add the ancestor to each part of the host compound selector, otherwise we can get
534+
// incorrect results. E.g. `.ancestor .a, .ancestor .b` vs `.ancestor .a, .b`.
535+
ancestors.forEach(escapedAncestor => {
536+
const ancestor = _restoreSelector(escapedAncestor, ancestorPlaceholders);
537+
return selectors.forEach(escapedSelector =>
538+
result.push(`${ancestor} ${_restoreSelector(escapedSelector, selectorPlaceholders)}`));
539+
});
540+
541+
return result.join(', ');
526542
}
527543

528544
/** Adds base options common to all harness types. */
@@ -560,3 +576,34 @@ function _valueAsString(value: unknown) {
560576
return '{...}';
561577
}
562578
}
579+
580+
/**
581+
* Splits up a compound selector into its parts and escapes any quoted content. The quoted content
582+
* has to be escaped, because it can contain commas which will throw throw us off when trying to
583+
* split it.
584+
* @param selector Selector to be split.
585+
* @returns The escaped string where any quoted content is replaced with a placeholder. E.g.
586+
* `[foo="bar"]` turns into `[foo=__cdkPlaceholder-0__]`. Use `_restoreSelector` to restore
587+
* the placeholders.
588+
*/
589+
function _splitAndEscapeSelector(selector: string): [parts: string[], placeholders: string[]] {
590+
const placeholders: string[] = [];
591+
592+
// Note that the regex doesn't account for nested quotes so something like `"ab'cd'e"` will be
593+
// considered as two blocks. It's a bit of an edge case, but if we find that it's a problem,
594+
// we can make it a bit smarter using a loop. Use this for now since it's more readable and
595+
// compact. More complete implementation:
596+
// https://github.com/angular/angular/blob/bd34bc9e89f18a/packages/compiler/src/shadow_css.ts#L655
597+
const result = selector.replace(/(["'][^["']*["'])/g, (_, keep) => {
598+
const replaceBy = `__cdkPlaceholder-${placeholders.length}__`;
599+
placeholders.push(keep);
600+
return replaceBy;
601+
});
602+
603+
return [result.split(',').map(part => part.trim()), placeholders];
604+
}
605+
606+
/** Restores a selector whose content was escaped in `_splitAndEscapeSelector`. */
607+
function _restoreSelector(selector: string, placeholders: string[]): string {
608+
return selector.replace(/__cdkPlaceholder-(\d+)__/g, (_, index) => placeholders[+index]);
609+
}

src/cdk/testing/tests/cross-environment.spec.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,22 @@ export function crossEnvironmentSpecs(
246246
const subcomps = await harness.directAncestorSelectorSubcomponent();
247247
expect(subcomps.length).toBe(2);
248248
});
249+
250+
it('should handle a compound selector with an ancestor', async () => {
251+
const elements = await harness.compoundSelectorWithAncestor();
252+
253+
expect(await parallel(() => elements.map(element => element.getText()))).toEqual([
254+
'Div inside parent',
255+
'Span inside parent'
256+
]);
257+
});
258+
259+
it('should handle a selector with comma inside attribute with an ancestor', async () => {
260+
const element = await harness.quotedContentSelectorWithAncestor();
261+
262+
expect(element).toBeTruthy();
263+
expect(await element.getText()).toBe('Has comma inside attribute');
264+
});
249265
});
250266

251267
describe('HarnessPredicate', () => {
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {ComponentHarness, HarnessPredicate} from '../../component-harness';
10+
11+
export class CompoundSelectorHarness extends ComponentHarness {
12+
static readonly hostSelector = '.some-div, .some-span';
13+
14+
static with(options = {}) {
15+
return new HarnessPredicate(CompoundSelectorHarness, options);
16+
}
17+
18+
async getText(): Promise<string> {
19+
return (await this.host()).text();
20+
}
21+
}

src/cdk/testing/tests/harnesses/main-component-harness.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
import {ComponentHarness} from '../../component-harness';
1010
import {TestElement, TestKey} from '../../test-element';
11+
import {CompoundSelectorHarness} from './compound-selector-harness';
12+
import {QuotedCommaSelectorHarness} from './quoted-comma-selector-harness';
1113
import {SubComponentHarness, SubComponentSpecialHarness} from './sub-component-harness';
1214

1315
export class WrongComponentHarness extends ComponentHarness {
@@ -81,6 +83,10 @@ export class MainComponentHarness extends ComponentHarness {
8183
this.locatorForAll(SubComponentHarness.with({ancestor: '.other, .subcomponents'}));
8284
readonly directAncestorSelectorSubcomponent =
8385
this.locatorForAll(SubComponentHarness.with({ancestor: '.other >'}));
86+
readonly compoundSelectorWithAncestor =
87+
this.locatorForAll(CompoundSelectorHarness.with({ancestor: '.parent'}));
88+
readonly quotedContentSelectorWithAncestor =
89+
this.locatorFor(QuotedCommaSelectorHarness.with({ancestor: '.quoted-comma-parent'}));
8490

8591
readonly subcomponentHarnessesAndElements =
8692
this.locatorForAll('#counter', SubComponentHarness);
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {ComponentHarness, HarnessPredicate} from '../../component-harness';
10+
11+
export class QuotedCommaSelectorHarness extends ComponentHarness {
12+
static readonly hostSelector = 'div[has-comma="a,b"]';
13+
14+
static with(options = {}) {
15+
return new HarnessPredicate(QuotedCommaSelectorHarness, options);
16+
}
17+
18+
async getText(): Promise<string> {
19+
return (await this.host()).text();
20+
}
21+
}

src/cdk/testing/tests/test-main-component.html

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,15 @@ <h1 style="height: 100px; width: 200px;">Main Component</h1>
5757
<test-sub title="other 1"></test-sub>
5858
<test-sub title="other 2"></test-sub>
5959
</div>
60+
<div class="parent">
61+
<div class="some-div">Div inside parent</div>
62+
<div class="some-span">Span inside parent</div>
63+
</div>
64+
<div class="some-div">Div outside parent</div>
65+
<div class="some-span">Span outside parent</div>
66+
<div class="quoted-comma-parent">
67+
<div has-comma="a,b">Has comma inside attribute</div>
68+
</div>
6069
<div class="task-state-tests">
6170
<button (click)="runTaskOutsideZone()" id="task-state-test-trigger">
6271
Run task outside zone

0 commit comments

Comments
 (0)