-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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(unify): Specs Page Workflows - E2E/Component #19780
Conversation
Thanks for taking the time to open a PR!
|
Test summaryRun details
View run in Cypress Dashboard ➡️ FlakinessThis 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 |
packages/frontend-shared/src/gql-components/OpenConfigFileInIDE.vue
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,12 @@ | |||
module.exports = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Three new system-test projects were added in this PR. Lots of files, some duplication, but utilizing these gives us as much e2e coverage as possible (as compared to config interceptions).
…into issue-19207-specs-page-ct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one change request on the button that was removed, I think we should come at that a different way. Overall this is super.
packages/frontend-shared/src/gql-components/OpenConfigFileInIDE.vue
Outdated
Show resolved
Hide resolved
packages/frontend-shared/src/gql-components/OpenConfigFileInIDE.vue
Outdated
Show resolved
Hide resolved
…into issue-19207-specs-page-ct # Conflicts: # packages/app/cypress/e2e/index.cy.ts
cy.findByRole('link', { name: defaultMessages.createSpec.viewSpecPatternButton }) | ||
.should('be.visible') | ||
.and('not.be.disabled') | ||
.and('have.attr', 'href', '#/settings?section=project&setting=specPattern') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this href at risk of changing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not in the short term. Medium term, this type of deeplink to the spec pattern will be replaced with a modal the opens and displays the spec pattern directly. But for now, the settings page is explicitly looking for this query param to decide where to scroll, so it's a pretty stable situation and we'd only want to change this URL if that implementation changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marktnoonan is there an issue logged to update to the modal pattern? I wasn't able to find one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tbiethman there was an issue, but recently closed as we believe this change has been dropped from 10.0. But it is still in Figma and I expect it will come around soon enough even if not present during release. It's probably a nicer experience to just peek at the spec pattern instead of leaving the current page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tbiethman I stand corrected, @ZachJW34 has this change completed as part of other spec pattern work in this PR, so whichever merges last can align the test with the new behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marktnoonan I'll keep an eye on that PR. It doesn't change the specs page implementation, at least as it stands. But if the component work is done, it might make sense to re-open that issue and hook this up prior to release.
cy.get('@EmptySpecCard').click() | ||
cy.contains('button', defaultMessages.components.button.cancel).click() | ||
cy.get('[data-cy="create-spec-modal"]').within(() => { | ||
cy.get('[data-cy="card"]').contains(defaultMessages.createSpec.e2e.importEmptySpec.header).click() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cy.get('[data-cy="card"]').contains(defaultMessages.createSpec.e2e.importEmptySpec.header).click() | |
cy.get('[data-cy="card"]') | |
.contains(defaultMessages.createSpec.e2e.importEmptySpec.header) | |
.click() |
* 10.0-release: test: add "cypress-in-cypress" tests for navigating in and out of spec runner (#19845) chore: add test cases for migration (#19905) test: fix launchpad flake caused by unoptimized deps (#19903) feat: launchpad browser select (#19830) fix: increase timeout in cy.contains to avoid flake refactor: Remove connection on specs, increase pagination limits elsewhere (#19881) fix(unify): Fixing launchpad project setup tests (#19886) test(unify): Specs Page Workflows - E2E/Component (#19780) feat: migration rename files (#19807) chore: remove design system warnings (#19851)
This PR adds coverage for the Specs page for both E2E and component testing projects. Some existing tests were modified to align with new tests to reduce duplication, and a few were removed if they have coverage elsewhere. Existing TODOs were cleaned up as much as possible.
User facing changelog
N/A
Additional details
The following reqs are accounted for here: TODO
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?