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

persistDashboardFilter.cy.js: fix cypress/no-unnecessary-waiting #3734

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

BacLuc
Copy link
Contributor

@BacLuc BacLuc commented Aug 22, 2023

And also check it in the CI.

  • extract navigation to beforeEach
  • Use the visible gui state for assertions instead of the url. (This is better supported by cypress, and does not depend on the implementation detail how the parameters are stored in the url.)
    -> this also removes the cy.wait
  • Test that the generated url can be opened again and this leads to the same filters. (is more explicit like this instead of the reload)
  • remove now redundant test "should add Category-Filters to query"
  • Reformulate test names that the describe + the test names form english sentences.
  • separate the acts and the asserts
  • wait for the Dashboard to load after the test for firefox

@BacLuc
Copy link
Contributor Author

BacLuc commented Aug 22, 2023

Could not add you to the reviewers @DeNic0la
But your feedback is also welcome

And also check it in the CI.
- extract navigation to beforeEach
- Use the visible gui state for assertions instead of the url.
(This is better supported by cypress, and does not depend on the implementation detail
how the parameters are stored in the url.)
-> this also removes the cy.wait
- Test that the generated url can be opened again and this leads to the same
filters. (is more explicit like this instead of the reload)
- remove now redundant test "should add Category-Filters to query"
- Reformulate test names that the describe + the test names form english sentences.
- separate the acts and the asserts
@DeNic0la
Copy link
Collaborator

Could not add you to the reviewers @DeNic0la But your feedback is also welcome

Honestly, this was the first cypress test i ever wrote. I think you made some good optimisations but i have way to little knowledge about cypress to give feedback about this.

Its an e2e test and IMO the most important thing is that its there and would fail if the feature doesn't work anymore and as far as i can tell it does that.

@manuelmeister manuelmeister added this pull request to the merge queue Sep 12, 2023
Merged via the queue into ecamp:devel with commit 87061ba Sep 12, 2023
22 checks passed
@BacLuc BacLuc deleted the e2e-check-eslint branch December 23, 2023 19:17
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.

5 participants