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

Changed test environment (nox -s fides_env) to run fides deploy for local testing #3017

Merged
merged 37 commits into from
Apr 12, 2023

Conversation

NevilleS
Copy link
Contributor

@NevilleS NevilleS commented Apr 10, 2023

Closes #3039

Code Changes

  • Rewrite the fides_env nox session to call fides deploy up instead of docker compose up and running custom Python scripts to setup test data
  • Update sample project privacy center with global privacy control feature
  • Remove the nox -s "build(sample)" codepath and use :local tags for fides deploy instead
  • Remove docker-compose.test_env.yml, src/data/fides/test_env/***, and other test_env-specific code
  • Improve check_webserver_required_config_values to reuse existing config code (instead of recreating the load)
  • Cleanup the sample project docker compose file, TOML, etc.
  • Add new options to fides deploy up: --env-file and --image, to give some customization options
  • Change redis.enabled to default to True (unrelated, but I just noticed this!)
  • Add a unit test for loading default configuration
  • Add a "build-time" report so we can see where the time goes in fides_env...
  • Update test environment docs
  • Optimize pip install (if possible)
  • Add fides_env(dev) with hot-reloading (if possible)
  • Update test environment to use new "system config" APIs to configure storage & messaging provider
  • Add more E2E tests: sample app, access request results, erasure request

Steps to Confirm

  • Run nox -s "fides_env(test)" and ensure everything we need for manual testing is there
  • Run Cypress E2E smoke tests

Pre-Merge Checklist

Description Of Changes

This overhauls the local test environment by eliminating all the “test_env” specific code and instead using fides deploy to spin up a fully-featured test project. I’m expecting this will remove a lot of duplication and make it easier to test new features like a customer would, but it does have some rough edges that I’d like some help with…

@cypress
Copy link

cypress bot commented Apr 10, 2023

Passing run #1292 ↗︎

0 3 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge 8dcb946 into 341f3a9...
Project: fides Commit: de6a39253c ℹ️
Status: Passed Duration: 00:37 💡
Started: Apr 12, 2023 12:25 PM Ended: Apr 12, 2023 12:26 PM

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

@codecov
Copy link

codecov bot commented Apr 10, 2023

Codecov Report

Patch coverage: 77.77% and project coverage change: +0.48 🎉

Comparison is base (2aa7858) 86.92% compared to head (10b24db) 87.41%.

❗ Current head 10b24db differs from pull request most recent head 8dcb946. Consider uploading reports for the commit 8dcb946 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3017      +/-   ##
==========================================
+ Coverage   86.92%   87.41%   +0.48%     
==========================================
  Files         303      306       +3     
  Lines       17322    17534     +212     
  Branches     2230     2255      +25     
==========================================
+ Hits        15057    15327     +270     
+ Misses       1849     1793      -56     
+ Partials      416      414       -2     
Impacted Files Coverage Δ
src/fides/api/main.py 79.01% <0.00%> (-0.13%) ⬇️
src/fides/core/config/redis_settings.py 82.92% <ø> (ø)
src/fides/core/deploy.py 38.58% <16.66%> (ø)
src/fides/cli/commands/util.py 81.03% <100.00%> (+1.40%) ⬆️
src/fides/core/config/__init__.py 96.15% <100.00%> (+0.64%) ⬆️
src/fides/core/config/helpers.py 100.00% <100.00%> (+2.35%) ⬆️

... and 27 files with indirect coverage changes

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ThomasLaPiana
Copy link
Contributor

@NevilleS I'm working through this now btw! I'm gonna commit directly

@NevilleS
Copy link
Contributor Author

Sounds good, the more the merrier!

Right now I feel like build times are the biggest issue, but we also need a good solution for a "dev" environment that hot-reloads the python server and (ideally) the admin UI.

Do you have some ideas for these?

print("Teardown complete")
def teardown(session: nox.Session, volumes: bool = False, images: bool = False) -> None:
"""Tear down all docker environments."""
for compose_file in COMPOSE_FILE_LIST:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, I prefer this. Good idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

it also showed me that one of those compose files doesn't seem to be valid, I'll double check on it

noxfiles/utils_nox.py Outdated Show resolved Hide resolved
@ThomasLaPiana
Copy link
Contributor

Sounds good, the more the merrier!

Right now I feel like build times are the biggest issue, but we also need a good solution for a "dev" environment that hot-reloads the python server and (ideally) the admin UI.

Do you have some ideas for these?

I'm giving the hot-reloading stuff a think...it's possible (as all are things with the power of code) but pondering what the best way might be

@NevilleS
Copy link
Contributor Author

Sounds good, the more the merrier!
Right now I feel like build times are the biggest issue, but we also need a good solution for a "dev" environment that hot-reloads the python server and (ideally) the admin UI.
Do you have some ideas for these?

I'm giving the hot-reloading stuff a think...it's possible (as all are things with the power of code) but pondering what the best way might be

I did check with @pattisdr who is the biggest user of that, and she's OK if we deprecate that, since there are other nox sessions that can do most of the same stuff. So I'd lean towards just pulling that out so we focus on making this work really well and be as reusable as possible in plus

@NevilleS NevilleS marked this pull request as ready for review April 11, 2023 23:19
@ThomasLaPiana ThomasLaPiana self-requested a review April 12, 2023 00:33
@ThomasLaPiana
Copy link
Contributor

@NevilleS I approve this, but I can't approve it because I'm an author now too

if we're both in agreement and 🤝 we can override and ship it

@NevilleS
Copy link
Contributor Author

Needs a CHANGELOG entry and then I say ship it. I'll carve out the follow-up work I want to continue doing separately (add to fidesplus, use this to add more tests, etc.)

@NevilleS NevilleS changed the title Merge test environment with fides deploy sample project Changed test environment (nox -s fides_env) to run fides deploy for local testing Apr 12, 2023
@NevilleS NevilleS merged commit da40572 into main Apr 12, 2023
@NevilleS NevilleS deleted the ns-new-test-env branch April 12, 2023 12:35
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.

Update fides test environment to use fides deploy for testing (privacy center, connectors, sample app, etc.)
2 participants