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

SaaS connector test refactoring #1795

Merged
merged 16 commits into from
Mar 6, 2023
Merged

Conversation

galvana
Copy link
Contributor

@galvana galvana commented Nov 16, 2022

Closes #1794

Code Changes

  • Adds a new nox command init_saas_connector that creates all the necessary files for a new connector

Steps to Confirm

  • Run nox -s init_saas_connector -- Wahoo and verify the test files are created

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Description Of Changes

First iteration, this should ideally accept a connector template as a parameter so it can copy the files into the right locations and inspect the connector parameters to populate the secrets fixtures.

@galvana galvana linked an issue Nov 16, 2022 that may be closed by this pull request
tests/ops/conftest.py Outdated Show resolved Hide resolved
@galvana galvana changed the title Work in progress SaaS connector test refactoring Nov 16, 2022
@codeclimate
Copy link

codeclimate bot commented Jan 12, 2023

Code Climate has analyzed commit f58a941 and detected 0 issues on this pull request.

View more on Code Climate.

@cypress
Copy link

cypress bot commented Mar 1, 2023

Passing run #614 ↗︎

0 3 0 0 Flakiness 0

Details:

Merge a54957a into 2afc8b6...
Project: fides Commit: 5338aa9e57 ℹ️
Status: Passed Duration: 00:36 💡
Started: Mar 3, 2023 11:27 PM Ended: Mar 3, 2023 11:27 PM

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

@galvana galvana marked this pull request as ready for review March 1, 2023 22:55
@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

Patch coverage has no change and project coverage change: +29.53 🎉

Comparison is base (e5edd4f) 57.05% compared to head (a54957a) 86.59%.

❗ Current head a54957a differs from pull request most recent head 93c73c5. Consider uploading reports for the commit 93c73c5 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1795       +/-   ##
===========================================
+ Coverage   57.05%   86.59%   +29.53%     
===========================================
  Files         291      290        -1     
  Lines       16458    16224      -234     
  Branches     2114     2062       -52     
===========================================
+ Hits         9390    14049     +4659     
+ Misses       6871     1792     -5079     
- Partials      197      383      +186     
Impacted Files Coverage Δ
src/fides/cli/commands/util.py 58.66% <0.00%> (-20.97%) ⬇️
src/fides/api/ctl/database/crud.py 69.44% <0.00%> (-4.41%) ⬇️
src/fides/cli/commands/crud.py 94.11% <0.00%> (-1.13%) ⬇️
src/fides/core/utils.py 86.15% <0.00%> (-0.42%) ⬇️
src/fides/cli/__init__.py 92.00% <0.00%> (-0.16%) ⬇️
src/fides/cli/commands/core.py 92.06% <0.00%> (ø)
src/fides/cli/commands/scan.py 88.46% <0.00%> (ø)
src/fides/cli/commands/user.py 100.00% <0.00%> (ø)
src/fides/cli/commands/export.py 100.00% <0.00%> (ø)
src/fides/cli/commands/generate.py 86.36% <0.00%> (ø)
... and 153 more

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.

Copy link
Contributor

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

i like this a lot! i think the approach here is great, and i'm excited to see it in action. i'm sure there will be enhancements and improvements that we think of as it gets put to use, but this looks like a great first version to get out into the wild. having this scaffolding to start will be a huge help for connector developers.

i left some comments that are relatively minor, but i think they're worth thinking thru. happy to talk this over more if you'd like!

tests/ops/test_helpers/saas_test_utils.py Show resolved Hide resolved
tests/fixtures/saas/yotpo_reviews_fixtures.py Show resolved Hide resolved
noxfiles/utils_nox.py Outdated Show resolved Hide resolved
Copy link
Contributor

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

looks good @galvana, just a quick follow-up to keep us fully covered please? 🙏

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

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

assuming it passes this latest CI run, let's get this in and start using it!

@galvana galvana merged commit 1c39399 into main Mar 6, 2023
@galvana galvana deleted the 1794-saas-connector-test-refactoring branch March 6, 2023 17:30
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.

SaaS connector test refactoring
2 participants