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

Source Chargebee - Implement integration testing for otherwise untested streams [ITAS] #35509

Merged
merged 113 commits into from
Mar 13, 2024

Conversation

pnilan
Copy link
Contributor

@pnilan pnilan commented Feb 21, 2024

What

  • Implements integration tests for the following streams via http_mocker:
    • addon
    • coupon
    • customer
    • event
    • hosted_page
    • plan
    • site_migration_detail
    • subscription
    • virtual_bank_account
  • Adds additional testing dependency freezegun
  • Fixes error in site_migration_detail semi-incremental implementation. Filters records based on migrated_at.
  • Unpins CDK version to ^0.67.1
  • Increments connector version to 0.4.1

Integration Test Cases

  • One-page read
  • Multi-page read
  • Correctly handles 400 response status
  • Correctly handles 401 response status
  • Correctly handles 429 response status (backoff and retry)
  • Correctly handles 500 status correctly (retries and then receives 200 response)
  • Correctly handles recurring 500 status correctly (retries, throws error)
  • Custom field transformation for subscription, customer, and coupon streams
  • Incremental: Correctly sets state when no initial state is present
  • Incremental: Correctly slices read based on previous state value

…rbytehq/airbyte into pnilan/source-chargebee-schema-update
Copy link
Contributor

@maxi297 maxi297 left a 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

@octavia-squidington-iv octavia-squidington-iv requested a review from a team March 1, 2024 19:34
Copy link
Contributor

@natikgadzhi natikgadzhi left a 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.

@octavia-squidington-iv octavia-squidington-iv requested a review from a team March 3, 2024 17:28
@pnilan pnilan requested a review from maxi297 March 7, 2024 15:30
Copy link
Collaborator

@lazebnyi lazebnyi left a 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.

Copy link
Contributor

@maxi297 maxi297 left a 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"],
Copy link
Contributor

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

@pnilan pnilan merged commit 58b6b80 into master Mar 13, 2024
29 checks passed
@pnilan pnilan deleted the pnilan/source-chargebee-integration-tests branch March 13, 2024 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/chargebee
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants