test(unify): Settings e2e tests #19324
Conversation
|
Thanks for taking the time to open a PR!
|
|
Test summary
Run details
View run in Cypress Dashboard ➡️ Flakiness
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 |
||||||||||||||||||||||||||
|
|
||
| cy.get('[data-cy="use-well-known-editor"]').should('not.be.checked') | ||
| cy.get('[data-cy="use-custom-editor"]').should('be.checked') | ||
| cy.contains('Choose your editor...').click() |
There was a problem hiding this comment.
We should use the defaultMessages when we can i.e.
import defaultMessages from '@packages/frontend-shared/src/locales/en-US.json'
cy.contains(defaultMessages.settingsPage.editor.noEditorSelectedPlaceholder).click()
There was a problem hiding this comment.
This is something I always struggle with when writing tests - it feels strange to be importing values to test against.
On the one hand, there's less duplication of strings, and if the string changes, then we don't have to update the test. We try to avoid embedding strings in code for good reasons, and some of those reasons apply to embedding strings in tests too.
On the other hand, if the string changes... the behavior has changed and wouldn't we want the test to point that out? If I replaced noEditorSelectedPlaceholder with an empty string in the localization, the test would still pass despite not showing what we expected to the user.
There was a problem hiding this comment.
That's a fair point. Plus, it makes the test a bit harder to read since I don't know exactly what DOM element it's trying to find, I either have to wait for the test to run or dig through the json. I think this is a good point to bring up to the team since we aren't consistent either way
There was a problem hiding this comment.
I agree with @BlueWinds - I don't think it makes sense to assert against the same thing we test against. You could changing the string in en-US.json to some random junk and it would still pass.
There was a problem hiding this comment.
For anyone following along at home, we've started a discussion in Slack about this, currently waiting for Jess to return and give her opinion on the matter before deciding which direction to go.
| cy.get('[data-cy="use-well-known-editor"]').should('not.be.checked') | ||
| cy.get('[data-cy="use-custom-editor"]').should('be.checked') | ||
| cy.contains('Choose your editor...').click() | ||
| cy.get('[data-testid="computer"]').click() |
There was a problem hiding this comment.
I started a discussion in Slack regarding the test attributes in use today: data-cy, data-testid, and data-test. There is a desire to standardize on data-cy given that is what is in our wheelhouse and what we recommend to our users.
The biggest hurdle to that is our usage of testing-library's Cypress command extensions, but we could override the attribute they're looking for to data-cy and continue to use the various ByTestId lookups.
…cycle * 10.0-release: build: remove syncRemoteGraphQL from codegen chore: fix incorrect type from merge build: allow work with local dashboard (#19376) chore: Test example recipes against chrome (#19362) test(unify): Settings e2e tests (#19324) chore(deps): update dependency ssri to 6.0.2 [security] (#19351) fix: spec from story generation, add deps for install (#19352) chore: Fix server unit tests running on mac by using actual tmp dir (#19350) fix: Add more precise types to Cypress.Commands (#19003) fix: Do not screenshot or trigger the failed event when tests are skipped (#19331) fix (#19262) fix: throw when writing to 'read only' properties of `config` (#18896) fix: close chrome when closing electron (#19322) fix: disable automatic request retries (#19161) chore: refactor cy funcs (#19080) chore(deps): update dependency @ffmpeg-installer/ffmpeg to v1.1.0 🌟 (#19300)
User facing changelog
N/A
Additional details
Adds tests around the settings page inside test runner, covering the following functionality:
The following bits are NOT covered:
How has the user experience changed?
N/A
PR Tasks
cypress-documentation?type definitions?cypress.schema.json?