Cypress testing suite for hidden features / bugs #543
Conversation
|
[diff-counting] Significant lines: 269. |
|
Visit the preview URL for this PR (updated for commit 575bfe7): https://cornelldti-courseplan-dev--pr543-hidden-feature-unit-swbuk23f.web.app (expires Tue, 16 Nov 2021 22:12:44 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
willespencer
changed the title
| // Test to confirm that only one "+New Semester" button shows up at once | ||
| // Due to front-end styling issues, there are two different buttons in different components | ||
| // Only one should be visible at a time (from semesterView if there are 0 semesters, from semester otherwise) | ||
| it('Test that only one semester button shows', () => { |
There was a problem hiding this comment.
Is it possible to have this test be perpetual (i.e. throughout the other tests, ensure the invariant holds) rather than for one specific set of steps?
There was a problem hiding this comment.
Yes! I think I can add an afterEach hook to run it after every test that we have. It will require reworking things a bit, so the test only checks for buttons and does not actually click anything, and also needs to ensure that the site is in a state where this can be checked. Adding it to my other item!
There was a problem hiding this comment.
and also needs to ensure that the site is in a state where this can be checked.
I think this can just be done by making the invariant "there is max 1 button" instead of "there is exactly 1 button". So if no buttons exist, the test passes.
"There is exactly 1 button" is not an invariant and not related to the bug itself so it can be another test if we want.
| cy.visit('localhost:8080/login'); | ||
| cy.login(Cypress.env('TEST_UID')); | ||
|
|
||
| const TEST_EMAIL = '[email protected]'; |
There was a problem hiding this comment.
😍
Should we refactor this Firestore reset and call it for every integration test?
Or at least store TEST_EMAIL in a constants file?
There was a problem hiding this comment.
Going to put TEST_EMAIL in a constants file now!
There was a problem hiding this comment.
I however do not think we want to do the Firestore reset in every integration test because then we have to go through Onboarding/walkthrough every time. It might make sense though to do a more simple data reset (instead of deleting data, set to only required/basic data) so that tests are not reliant on each other. Making an item for this!
|
^ also went through the entire test plan and everything worked as expected:
|
Summary
This pull request adds Cypress front-end tests to test a number of features & bugs that can pop up without devs noticing, especially ones that have happened in the past and could be repeated.
Remaining TODOs:
Test Plan
Confirm that the CI tests pass - this means that all the existing and newly written Cypress tests also pass
Additionally, change the URL used for accessing CUReviews data locally and confirm that Cypress test would fail were CUReviews API to break or change routes.
Notes
Resolving the Firestore permissions error took a long time - but the issue ended up being related to the service account secret on GitHub. @SamChou19815 suspects that it was in multiline form which could have been causing the issue of insufficient permissions on the CI. Resetting it fixed the issue, for both using Firestore calls in Cypress, and also when writing separate scripts to edit user data before running cypress (see individual commits).
This new spec file will delete the semester and onboarding collections for the test email, which could affect other tests if this new file does not run completely and these documents are not recreated.