-
Notifications
You must be signed in to change notification settings - Fork 90
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
chore(cypress): Favor firefox in cypress #3121
Conversation
Test summaryRun details
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 |
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.
I merged #3119 and if that lets all the dependency PRs pass, I'd say let's close this?
"cypress:run": "cross-env cypress run", | ||
"cypress:open": "cross-env cypress open", | ||
"cypress:run": "cross-env cypress run --browser firefox", | ||
"cypress:open": "cross-env cypress open --browser firefox", |
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.
That has the drawback that we cannot record our cypress tests anymore 😢 can we close?
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.
No, actually it doesn't, if you look at the build, or click on cypress, you can see that all the videos were recorded
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.
I wouldn't close, is there any justifiable reason to use a different browser than what we recommend our users to use for e2e tests?
cypress/integration/common/report.js
Outdated
@@ -169,7 +169,7 @@ Then('each list item links to the post page', () => { | |||
Then('I can visit the post page', () => { | |||
cy.contains(annoyingUserWhoMutedModeratorTitle).click() | |||
cy.location('pathname').should('contain', '/post') | |||
.get('h3').should('contain', annoyingUserWhoMutedModeratorTitle) | |||
.get('.ds-card-content .hyphenate-text').should('contain', annoyingUserWhoMutedModeratorTitle) |
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.
Code creep?
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.
Could be, yes.
It failed locally, but that was in headless. Also, I was reading cypress docs and they suggest to never use tags as selectors.
We could open another PR to clean up our cypress code though
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.
👍 let's keep fingers crossed.
482001d
to
073d986
Compare
- we have recommended our users to use firefox, so why should we test in Electron?
073d986
to
2516da5
Compare
@@ -169,7 +169,7 @@ Then('each list item links to the post page', () => { | |||
Then('I can visit the post page', () => { | |||
cy.contains(annoyingUserWhoMutedModeratorTitle).click() | |||
cy.location('pathname').should('contain', '/post') | |||
.get('title').should('contain', annoyingUserWhoMutedModeratorTitle) | |||
.get('.base-card .title').should('contain', annoyingUserWhoMutedModeratorTitle) |
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 related to this PR
directly, but not even sure how this was passing!? the title
doesn't have a .
indicating it's a class 🤷♀️
"baseUrl": "http://localhost:3000", | ||
"env": { | ||
"RETRIES": 2 | ||
} |
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.
Also, not related to firefox
as a test runner, but unless I'm missing something, does anyone think retries
are actually occurring? hopefully, we can see some evidence that this works
Test summaryRun details
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 |
🍰 Pullrequest
Issues
Todo