-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Source Chargebee - Implement integration testing for otherwise untested streams [ITAS] #35509
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
Conversation
…rbytehq/airbyte into pnilan/source-chargebee-schema-update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple comments (some unrelated to your changes but highlighted by the integration tests
airbyte-integrations/connectors/source-chargebee/integration_tests/configured_catalog.json
Show resolved
Hide resolved
airbyte-integrations/connectors/source-chargebee/source_chargebee/components.py
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-chargebee/source_chargebee/manifest.yaml
Show resolved
Hide resolved
airbyte-integrations/connectors/source-chargebee/source_chargebee/manifest.yaml
Show resolved
Hide resolved
airbyte-integrations/connectors/source-chargebee/source_chargebee/manifest.yaml
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-chargebee/unit_tests/integration/test_addon.py
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-chargebee/unit_tests/integration/test_addon.py
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-chargebee/unit_tests/integration/test_addon.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-chargebee/unit_tests/integration/test_addon.py
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-chargebee/unit_tests/integration/test_coupon.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How many workspaces do we have on Chargebee?
For a mostly low-code connector, the amount of code to have strong test coverage is insanely high. That's not critique of this PR, though — this PR is great.
airbyte-integrations/connectors/source-chargebee/unit_tests/integration/test_addon.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With Maxime's comments LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for your diligence in addressing my comments
@@ -121,11 +121,11 @@ | |||
"supported_sync_modes": ["full_refresh", "incremental"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the configured_catalog show all the streams? There are some I don't see like event
and site_migration_detail
What
addon
coupon
customer
event
hosted_page
plan
site_migration_detail
subscription
virtual_bank_account
freezegun
site_migration_detail
semi-incremental implementation. Filters records based onmigrated_at
.Integration Test Cases
subscription
,customer
, andcoupon
streams