-
Notifications
You must be signed in to change notification settings - Fork 16
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
Optimize tests #1027
Optimize tests #1027
Conversation
Your Render PR Server URL is https://files-ui-stage-pr-1027.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-c2dri4j2v8pd0ivpilug. |
describe("Settings", () => { | ||
it("can navigate to the settings profile page", () => { | ||
cy.web3Login() | ||
cy.get("[data-cy=settings-nav]").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.
Is there perhaps a way we could make these markers constant or avoid needing magic strings?
CC: @FSM1 @tanmoyAtb
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 sure if I misunderstood what you mean but those data-cy are based on Cypress best practices, to make sure tests fail as little as possible, even when changing the copy, or the classes https://docs.cypress.io/guides/references/best-practices#Selecting-Elements
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 mean we could have a bunch of string constants, but I dont know if there is much benefit to be had yet. If we do start feeling overwhelmed by this, its easy enough to Ctrl+f data-cy and fix it later
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.
Ah now I get it. This is not something that should ever change, so I don't think we would get any benefit from having a list of strings there indeed.
Co-authored-by: Michael Yankelev <12774278+FSM1@users.noreply.github.com>
* Fix upload (#1010) * Bulk operations for Bin (#1017) * Set the base for Cypress tests and automation (#1016) * Update Send Feedback Link (#1024) * only enable logging in non-mainnet env (#1020) * Optimize tests (#1027) * fix (#1030) * File browser context provider (#1026) * work on selections (#1029) * Remove release drafter for now (#1038) * Update Readme for tests (#1036) * Delete release_drafter.yml (#1039) * Bulk DND Move files (#1028) * Test file upload (#1035) * Selection removal preventions (#1037) * Theme selection UI fix (#1033) * Update API client (#1032) * make it light and the files blue (#1049) * React-PDF Worker fixes (#1052) * Store screenshots and video of failing tests (#1054) * fix test classes and add rename test (#1055) * Hide date for folders (#1060) * s/testId/testid (#1064) * folder path in route fixed (#1066) * Resolve File Browser race condition (#1069)
it
in 2 exactly similar spec files. The first takes 30s, the second 15s 🚀 because it's leveraging what was stored in the first.REACT_APP_TEST
variable to disable the shared transfer that may or may not be present and that Cypress tells that it's really bad to do conditional testing based on the UI (also it's hard). And because there's no way to know because it comes from IPFS, I thought we should first test all the other things, and by the time we're good, we'll have notifications rather than an aggressive popup :D