Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HarnessPredicate doesn't correctly apply ancestor selectors to compound component selectors #22475

Closed
jelbourn opened this issue Apr 13, 2021 · 1 comment · Fixed by #22476
Closed

HarnessPredicate doesn't correctly apply ancestor selectors to compound component selectors #22475

jelbourn opened this issue Apr 13, 2021 · 1 comment · Fixed by #22476
Assignees
@crisbeto
Labels
area: cdk/testing G This is is related to a Google internal issue help wanted The team would appreciate a PR from the community to address this issue P2 The issue is important to a large percentage of users, with a workaround

Comments

@jelbourn
Copy link
Member

Note(@jelbourn): Copied from an internal bug report

HarnessPredicate.getSelector returns the wrong result in certain cases. E.g., MatButtonHarness and other CDK harnesses are defined as matching one of several possible selectors:

  static hostSelector = [
    '[mat-button]',
    '[mat-raised-button]',
    '[mat-flat-button]',
    '[mat-icon-button]',
    '[mat-stroked-button]',
    '[mat-fab]',
    '[mat-mini-fab]',
  ].join(',');

I would expect getHarness(MatButtonHarness.with({ancestor: '.foo'})) to always find a MatButton that has a '.foo' ancestor. However, the current implementation of https://source.corp.google.com/search?q=HarnessPredicate.getSelector returns:

.foo [mat-button],[mat-raised-button],[mat-flat-button],[mat-icon-button],[mat-stroked-button],[mat-fab],[mat-mini-fab]

which only applies the ancestor constraint to the first possibility.
@jelbourn jelbourn added P2 The issue is important to a large percentage of users, with a workaround help wanted The team would appreciate a PR from the community to address this issue G This is is related to a Google internal issue area: cdk/testing labels Apr 13, 2021
@crisbeto crisbeto self-assigned this Apr 14, 2021
crisbeto added a commit to crisbeto/material2 that referenced this issue Apr 14, 2021
@crisbeto
fix(cdk/testing): incorrectly handling ancestor of compound selector
13d0089 
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 angular#22475.
@crisbeto crisbeto mentioned this issue Apr 14, 2021
Merged
crisbeto added a commit to crisbeto/material2 that referenced this issue Apr 14, 2021
@crisbeto
fix(cdk/testing): incorrectly handling ancestor of compound selector
2ae897c 
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 angular#22475.
crisbeto added a commit to crisbeto/material2 that referenced this issue Apr 14, 2021
@crisbeto
fix(cdk/testing): incorrectly handling ancestor of compound selector
d46e47b 
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 angular#22475.
mmalerba pushed a commit that referenced this issue May 7, 2021
@crisbeto
fix(cdk/testing): incorrectly handling ancestor of compound selector (#…
032feef 
…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.
mmalerba pushed a commit that referenced this issue May 7, 2021
@crisbeto @mmalerba
fix(cdk/testing): incorrectly handling ancestor of compound selector (#…
f053bef 
…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.

(cherry picked from commit 032feef)
mmalerba pushed a commit that referenced this issue May 7, 2021
@crisbeto @mmalerba
fix(cdk/testing): incorrectly handling ancestor of compound selector (#…
ea84e7d 
…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.

(cherry picked from commit 032feef)
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@crisbeto crisbeto

Labels
area: cdk/testing G This is is related to a Google internal issue help wanted The team would appreciate a PR from the community to address this issue P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(cdk/testing): incorrectly handling ancestor of compound selector crisbeto/material2
2 participants
@jelbourn @crisbeto