Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Fixing inconsistent SaaS connector integration tests #473

Conversation

galvana
Copy link
Collaborator

@galvana galvana commented May 5, 2022

Purpose

Fixing inconsistencies with the SaaS connector tests for the following connectors:

  • Hubspot
  • Segment
  • Sentry
  • Stripe

Changes

  • Fixing bug with filter_results that occurred for row data that is returned from SaaS API responses but not annotated in the dataset
  • Waiting until data is available for Hubspot, Segment, and Sentry
  • Updated Stripe access tests since an active subscription on the access identity added a new charge and payment_intent. Cancelled the subscription within Stripe to have consistent data moving forward.

Checklist

  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
  • If docs updated (select one):
    • 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)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes #472

@galvana galvana added the run unsafe ci checks Triggers running of unsafe CI checks label May 5, 2022
@galvana galvana requested a review from PSalant726 May 6, 2022 15:48
src/fidesops/task/filter_results.py Outdated Show resolved Hide resolved
tests/fixtures/saas/hubspot_fixtures.py Outdated Show resolved Hide resolved
tests/fixtures/saas/hubspot_fixtures.py Outdated Show resolved Hide resolved
tests/fixtures/saas/segment_fixtures.py Outdated Show resolved Hide resolved
tests/fixtures/saas/segment_fixtures.py Outdated Show resolved Hide resolved
tests/integration_tests/saas/test_sentry_task.py Outdated Show resolved Hide resolved
tests/integration_tests/saas/test_sentry_task.py Outdated Show resolved Hide resolved
tests/integration_tests/saas/test_sentry_task.py Outdated Show resolved Hide resolved
Copy link
Contributor

@PSalant726 PSalant726 left a comment

Choose a reason for hiding this comment

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

Just some general Python suggestions, but nothing major. Unfortunately I don't have enough context to provide feedback on the content of the changes 😕 . We should get a review from @ethyca/fidesops-team here.

tests/integration_tests/saas/test_sentry_task.py Outdated Show resolved Hide resolved
@@ -137,8 +137,9 @@ def hubspot_erasure_data(connection_config_hubspot, hubspot_erasure_identity_ema
# no need to subscribe contact, since creating a contact auto-subscribes them

# Allows contact to be propagated in Hubspot before calling access / erasure requests
remaining_tries = 5
remaining_tries = 10
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we could encapsulate these things in config vars specific to these tests so they can be tuned easier. Do we have a clear picture of how long a contact takes to propagate through Hubspot, or does it vary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It varies but this is a good future proposal to have the configuration in one place

@seanpreston
Copy link
Contributor

Thanks @galvana these changes are super useful

tests/integration_tests/saas/test_sentry_task.py Outdated Show resolved Hide resolved
src/fidesops/task/filter_results.py Outdated Show resolved Hide resolved
tests/integration_tests/saas/test_sentry_task.py Outdated Show resolved Hide resolved
@galvana galvana added run unsafe ci checks Triggers running of unsafe CI checks and removed run unsafe ci checks Triggers running of unsafe CI checks labels May 9, 2022
@galvana galvana added run unsafe ci checks Triggers running of unsafe CI checks and removed run unsafe ci checks Triggers running of unsafe CI checks labels May 10, 2022
@galvana galvana added run unsafe ci checks Triggers running of unsafe CI checks and removed run unsafe ci checks Triggers running of unsafe CI checks labels May 10, 2022
@galvana galvana added run unsafe ci checks Triggers running of unsafe CI checks and removed run unsafe ci checks Triggers running of unsafe CI checks labels May 10, 2022
@galvana galvana merged commit 417e767 into main May 10, 2022
@galvana galvana deleted the 472-saas-connectors-inconsistent-results-with-saas-connector-integration-tests branch May 10, 2022 17:58
adamsachs pushed a commit to adamsachs/fidesops_forked_test that referenced this pull request May 17, 2022
Fixing inconsistent SaaS connector integration tests
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
Fixing inconsistent SaaS connector integration tests
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
run unsafe ci checks Triggers running of unsafe CI checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SaaS Connectors] Inconsistent results with SaaS connector integration tests
3 participants