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.

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

fix: validate selectorPriority configuration #18573

Merged
merged 5 commits into from
Oct 21, 2021

Conversation

mjhenkes
Copy link
Member

@mjhenkes mjhenkes commented Oct 20, 2021

User facing changelog

This introduces validation for the selectorPriority configuration option. Previously any values were accepted, but any values other than data-*, id, class, tag, attribute, or nth-child would cause the selector playground to error out in the cypress app.

Additional details

We use a fork of the unique-selector package to generate the selectors for the playground. The unique-selector package only supports the following 'priorities':

  • data-*
  • id
  • class
  • attributes
  • nth-child
  • tag

Adding the name priority causes the getAllSelectors function to blow up because it doesn't have a function to create a selector for that attribute.

How has the user experience changed?

The app will now display an exception if the selectorPriority configuration has been setup with unsupported values. This is preferable to having the selector playground cease functioning.

PR Tasks

  • Have tests been added/updated?
  • Has the original issue or this PR been tagged with a release in ZenHub?

@mjhenkes mjhenkes requested a review from a team as a code owner October 20, 2021 17:50
@mjhenkes mjhenkes requested review from BlueWinds and emilyrohrbough and removed request for a team October 20, 2021 17:50
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 20, 2021

Thanks for taking the time to open a PR!

@CLAassistant
Copy link

CLAassistant commented Oct 20, 2021

CLA assistant check
All committers have signed the CLA.

@cypress
Copy link

cypress bot commented Oct 20, 2021



Test summary

4208 0 50 2Flakiness 2


Run details

Project cypress
Status Passed
Commit 1ed4e57
Started Oct 21, 2021 3:30 PM
Ended Oct 21, 2021 3:41 PM
Duration 11:09 💡
OS Linux Debian - 10.9
Browser Electron 93

View run in Cypress Dashboard ➡️


Flakiness

cypress/proxy-logging-spec.ts Flakiness
1 Proxy Logging > request logging > xhr log has response body/status code
e2e/redirects_spec.js Flakiness
1 redirection > meta > binds to the new page after a timeout

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

.and.include('`Cypress.SelectorPlayground.defaults()` called with invalid `selectorPriority` property. It must be one of: `data-*`, `id`, `class`, `tag`, `attributes`, `nth-child`. You passed: `name`')
})

it('throws if selector:playground:priority if selectorPriority contains an unsupported priority that contains a valid priority', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these test names flipped?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to say 'idIsNotValid' contains the substring 'id' to demonstrate we're testing the full word and not just starts with. I'm open to better ways of saying that. 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahhh maybe

throws selector:playground:priority if selectorPriority has an unsupported priority that contains a substring of a valid priority

emilyrohrbough
emilyrohrbough previously approved these changes Oct 20, 2021
Copy link
Contributor

@chrisbreiding chrisbreiding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few nitpicks. Otherwise looks great.

@mjhenkes mjhenkes merged commit 5f94cad into develop Oct 21, 2021
@mjhenkes mjhenkes deleted the fix-7745-validate-selector-priority-config branch October 21, 2021 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Modifying the Cypress.SelectorPlayground.defaults - selectorPriority breaks the Selector Playground
4 participants