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

test: component test updates #19925

Merged
merged 11 commits into from
Jan 27, 2022
Merged

Conversation

marktnoonan
Copy link
Contributor

Additional details

In the order GH presents the files, this PR:

  • uses a label to locate the 'spec' input the generator in e2e tests
  • uses label to locate input in SelectCloudProjectModal test
  • adds a little more RecordKey test coverage
  • uses a label to locate the 'spec' input in component tests
  • adds a label to the Specs List search input
  • adds a label to the empty generator input
  • commits missing change for e2eProjectDirs
  • adds more testing to Alert component
  • fixes Alert props - alertClass is confusing and targets the same element as the header, so it wasn't having any effect when used as the tests suggest it should be used.
  • adds coverage to Input component. Would like to make Input also manage the label but for this PR I sidestepped that and fixed labels in consumers instead. The style of labels varies a lot.
  • Unskip and fix a BaseError test for the retry button

PR Tasks

  • Have tests been added/updated?
  • [na] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?
  • [na] Have new configuration options been added to the cypress.schema.json?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jan 26, 2022

Thanks for taking the time to open a PR!

@marktnoonan marktnoonan changed the title remove retry test as retry is not used test: component test updates Jan 26, 2022
@cypress
Copy link

cypress bot commented Jan 26, 2022



Test summary

4356 0 51 0Flakiness 0


Run details

Project cypress
Status Passed
Commit 13792aa
Started Jan 27, 2022 8:55 PM
Ended Jan 27, 2022 9:07 PM
Duration 11:52 💡
OS Linux Debian - 10.10
Browser Electron 94

View run in Cypress Dashboard ➡️


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

lmiller1990
lmiller1990 previously approved these changes Jan 26, 2022
@@ -42,21 +41,24 @@ describe('<SpecsList />', { keystrokeDelay: 0 }, () => {
acc.relative.length < spec.relative.length ? spec : acc
, specs[0])

cy.get(inputSelector).type('garbage 🗑', { delay: 0 })
cy.findByLabelText(defaultMessages.specPage.searchPlaceholder)
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to answer or change anything here right now, but are we sticking w/ i18n in specs or not? there's a ton of discussion around this, it's not entirely clear what we landed on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Last I recall we were "sticking with it" as a practice but not to the point where we would hold up a PR that doesn't do it.

cy.findByLabelText(defaultMessages.specPage.searchPlaceholder)
.as('specsListInput')

cy.get('@specsListInput').type('garbage 🗑', { delay: 0 })
Copy link
Contributor

Choose a reason for hiding this comment

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

Not your change but what does delay: 0 do here? (why do we need it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's just cumulatively saving some time across tests (vs the 10ms default)? We do it several places, @JessicaSachs might know if there's a bigger reason.

elevatebart
elevatebart previously approved these changes Jan 27, 2022
Copy link
Contributor

@elevatebart elevatebart left a comment

Choose a reason for hiding this comment

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

Reapproved after dismissing @lmiller1990 approval

@marktnoonan marktnoonan merged commit 6becbb6 into 10.0-release Jan 27, 2022
@marktnoonan marktnoonan deleted the UNIFY-693-component-tests branch January 27, 2022 21:09
tgriesser added a commit that referenced this pull request Jan 31, 2022
* 10.0-release: (25 commits)
  fix(unify): improve dev server config ergonomics (#19957)
  feat: add spec pattern modal (#19801)
  fix: Windows e2e project scaffolding issues (#19938)
  feat: update @cypress/schematic to use proper e2e config for 10.0.0 (#19827)
  fix: correctly migrate projects with custom integration folder (#19929)
  fix: component spec creation with spec pattern (#19862)
  fix: missed committing yarn.lock after merge conflict
  fix: correct reference branch / commitSha in performance-reporter (#19941)
  feat: update navbar UI per Figma (#19926)
  fix: seed examples files when no e2e directory is created (#19768)
  chore: remove windy lightBlue warning
  test: component test updates (#19925)
  feat: Focus browser from select browser screen and on dashboard login (#19842)
  test: Honeycomb system-test reporter (#19855)
  fix(deps): update dependency engine.io to v5.2.1 [security]
  feat: Retain fileName when working with aliased fixtures and files (#19820)
  Update release-process.md
  Update release-process.md
  Update release-process.md
  Update release-process.md
  ...
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.

3 participants