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

Cypress end to end smoke test #2241

Merged
merged 25 commits into from
Feb 10, 2023
Merged

Cypress end to end smoke test #2241

merged 25 commits into from
Feb 10, 2023

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Jan 13, 2023

A good chunk of #1293

Code Changes

  • New folder cypress-e2e within clients with Cypress 12 set up
  • Test for logging in
  • Test for access request
  • Test for confirming mongo and postgres connectors are configured

Steps to Confirm

  • nox -s test_env
  • In the new folder cypress-e2e, install dependencies via npm i
  • npm run cy:run to run all the cypress tests
  • Alternatively, use npm run cy:open to watch the tests run in their UI

Pre-Merge Checklist

Description Of Changes

This is the first chunk of #1293. The new shiny thing is cy.origin which was introduced in Cypress 12 and makes it possible for us to write tests that go between different origins (i.e. privacy center and admin ui) without cross origin errors.

The original requirement wanted one single test to test all the things, though I thought it made sense to break them up and wasn't sure if there was a benefit to them all being in one test. (mostly checking mongo + postgres seemed independent of approving an access request)

I'd like to get this reviewed and discussed first, and then I can break out the remaining work in #1293 to separate tickets.

One thing I'd like to discuss is if it is okay that we are running these tests against the development UI servers (nox -s test_env spins up localhost:3000 and localhost:3001 for the web apps). I think ideally we'd test against localhost:8080 (the built frontend for admin-ui at least) to more mimic the production environment. But then we'd likely have to change some configurations with test_env, perhaps to not volume mount our local built front end?

Also, do we want this running in CI?

@allisonking allisonking marked this pull request as ready for review January 13, 2023 21:44
@NevilleS
Copy link
Contributor

Thanks for doing this - on first blush this is precisely what we need, so what I want to do now is figure out how to structure this best for maintainability.

I'll give it some more time this week. I tried running locally and the tests failed because of timeouts... because it was having to build the admin UI pages first before it could navigate to them 🙃. A production build of the Admin UI feels more appropriate for Cypress E2E...

Copy link
Contributor

@ssangervasi ssangervasi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚬 Most of my comments could be spun out as enhancement tickets, maybe TODO once we have more cases in mind.

@codecov
Copy link

codecov bot commented Jan 27, 2023

Codecov Report

Base: 88.65% // Head: 88.66% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (ac863b6) compared to base (e50015c).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2241   +/-   ##
=======================================
  Coverage   88.65%   88.66%           
=======================================
  Files         331      331           
  Lines       16099    16099           
  Branches     4469     4469           
=======================================
+ Hits        14273    14274    +1     
+ Misses       1669     1668    -1     
  Partials      157      157           
Impacted Files Coverage Δ
src/fides/api/main.py 86.75% <0.00%> (+0.66%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@allisonking allisonking force-pushed the aking/1293/cypress-test-env branch 3 times, most recently from 79830a0 to 1d9541c Compare February 6, 2023 19:47
@cypress
Copy link

cypress bot commented Feb 9, 2023

Passing run #20 ↗︎

0 3 0 0 Flakiness 0

Details:

Merge ac863b6 into e50015c...
Project: fides Commit: 0989c4817a ℹ️
Status: Passed Duration: 00:36 💡
Started: Feb 10, 2023 6:17 PM Ended: Feb 10, 2023 6:18 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@allisonking
Copy link
Contributor Author

privacy center check passed locally. the CI test seems to be failing across multiple branches, will need to look into it separately...

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.

3 participants