SAC-29934: Refactored and fixed integration tests to support new streams #43
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…into SAC-29934-add-integration-tests
There was a problem hiding this comment.
Pull request overview
This PR refactors and fixes integration tests for the tap-trello connector to support new streams including board-related streams (board_memberships, board_custom_fields, board_labels), card-related streams (card_attachments, card_custom_field_items), organization-related streams (organizations, organization_actions, organization_members, organization_memberships), and members. The changes ensure tests properly handle streams that cannot be directly created and implement proper parent stream tracking to avoid mixing different parent types.
Key changes:
- Introduced a base test class (TrelloBaseTest) to centralize common test setup and stream definitions
- Enhanced test utilities to support new streams and handle uncreatable streams gracefully
- Fixed parent object tracking to reset when switching between different parent stream types
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/trello_utils.py | Extended utilities to support new streams, added tracking for parent stream context, enhanced error handling for object creation/fetching |
| tests/test_trello_start_date.py | Refactored to use TrelloBaseTest, added handling for uncreatable streams |
| tests/test_trello_pagination.py | Refactored to use TrelloBaseTest and removed duplicate method definitions |
| tests/test_trello_discovery.py | Refactored to use TrelloBaseTest and removed duplicate boilerplate |
| tests/test_trello_custom_fields.py | Refactored to use TrelloBaseTest, added untestable streams handling |
| tests/test_trello_bookmarks_qa.py | Enhanced bookmark validation to handle different bookmark field names, improved robustness with null checks |
| tests/test_trello_bookmark_states.py | Refactored to use TrelloBaseTest and improved bookmark field validation |
| tests/test_trello_auto_fields.py | Refactored to use TrelloBaseTest, enhanced validation for deduplicated streams |
| tests/base.py | New base class providing common test setup and stream definitions for all Trello tests |
| tap_trello/streams/organization_actions.py | Implemented date filtering for incremental replication using bookmark and start_date |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vishalp-dev
reviewed
Jan 6, 2026
vishalp-dev
reviewed
Jan 6, 2026
vishalp-dev
reviewed
Jan 6, 2026
vishalp-dev
reviewed
Jan 6, 2026
vishalp-dev
reviewed
Jan 6, 2026
vishalp-dev
reviewed
Jan 6, 2026
Member
vishalp-dev
left a comment
vishalp-dev
left a comment
There was a problem hiding this comment.
Added Review comments, Also please respond on the copilot comments
vishalp-dev
reviewed
Jan 6, 2026
vishalp-dev
reviewed
Jan 6, 2026
vishalp-dev
reviewed
Jan 6, 2026
vishalp-dev
reviewed
Jan 6, 2026
vishalp-dev
reviewed
Jan 6, 2026
vishalp-dev
reviewed
Jan 6, 2026
vishalp-dev
approved these changes
Jan 7, 2026
MuralidharT03
added a commit
that referenced
this pull request
Jan 7, 2026
* Add sync logic for the tap * Updated sync logic * Fixed naming convention * Fixed pylint errors and refactored duplicate code * Add unittests for the tap (#42) * Addressed review comments * Refactored code to address review comments * Removed non functional methods * SAC-29934: Refactored and fixed integration tests to support new streams (#43) * Refactored and fixed integration tests to support new streams * Excluded organization_actions stream as it doesn't have enough data * Added organization_actions stream to untestable_streams * Fixed bookmarks assertion * Fixed duplicate records assertion error * Fix automatic fields for child streams by calling modify_object in FullTableStream * Fix package installation by using find_packages() to include streams submodule * Fixed Field mismatch errors * Addressed review comments * Fix API rate limits * Fix assertion error for card_custom_field_items stream
MuralidharT03
added a commit
that referenced
this pull request
Jan 7, 2026
* Updated schema.json files for streams * Added parent id field in child schemas * Removed trailing newlines * Updated libraries, gitignore and ReadME documentation * Added missing newline at end of file * Enforce non-nullable primary key properties * SAC-29835: Add sync logic for the tap (#41) * Add sync logic for the tap * Updated sync logic * Fixed naming convention * Fixed pylint errors and refactored duplicate code * Add unittests for the tap (#42) * Addressed review comments * Refactored code to address review comments * Removed non functional methods * SAC-29934: Refactored and fixed integration tests to support new streams (#43) * Refactored and fixed integration tests to support new streams * Excluded organization_actions stream as it doesn't have enough data * Added organization_actions stream to untestable_streams * Fixed bookmarks assertion * Fixed duplicate records assertion error * Fix automatic fields for child streams by calling modify_object in FullTableStream * Fix package installation by using find_packages() to include streams submodule * Fixed Field mismatch errors * Addressed review comments * Fix API rate limits * Fix assertion error for card_custom_field_items stream
rdeshmukh15
added a commit
that referenced
this pull request
Jan 22, 2026
* SAC-29830: Initial Commit with Singer tap generated code * Updated schema.json files for streams (#40) * Updated schema.json files for streams * Added parent id field in child schemas * Removed trailing newlines * Updated libraries, gitignore and ReadME documentation * Added missing newline at end of file * Enforce non-nullable primary key properties * SAC-29830: Create schema (#44) * Updated schema.json files for streams * Added parent id field in child schemas * Removed trailing newlines * Updated libraries, gitignore and ReadME documentation * Added missing newline at end of file * Enforce non-nullable primary key properties * SAC-29835: Add sync logic for the tap (#41) * Add sync logic for the tap * Updated sync logic * Fixed naming convention * Fixed pylint errors and refactored duplicate code * Add unittests for the tap (#42) * Addressed review comments * Refactored code to address review comments * Removed non functional methods * SAC-29934: Refactored and fixed integration tests to support new streams (#43) * Refactored and fixed integration tests to support new streams * Excluded organization_actions stream as it doesn't have enough data * Added organization_actions stream to untestable_streams * Fixed bookmarks assertion * Fixed duplicate records assertion error * Fix automatic fields for child streams by calling modify_object in FullTableStream * Fix package installation by using find_packages() to include streams submodule * Fixed Field mismatch errors * Addressed review comments * Fix API rate limits * Fix assertion error for card_custom_field_items stream * Fixed spelling mistakes * Remove underscore prefixes from internal methods in DateWindowPaginated mixin * Replace OAuth1 authentication with API key and token authentication (#46) * Replace OAuth1 authentication with API key and token authentication * Addressed copilot comments * Updated ReadMe document and removed authenticate method * Update config.yml to fetch env vars from tap_tester_sandbox * Reduced page limit for actions and cards * Fix custom fields test to compare IDs instead of exact dict values * Refactored tests to use base suite and deleted legacy tests (#47) * Refactored tests to use base suite and deleted legacy tests * Fix bookmark and start-date test cases * Remove interrupted-sync test case * Update bookmark test * Resolve PR comments --------- Co-authored-by: satyendra101 <[email protected]> --------- Co-authored-by: Rutuja Deshmukh <[email protected]> Co-authored-by: satyendra101 <[email protected]> --------- Co-authored-by: Rutuja Deshmukh <[email protected]> Co-authored-by: satyendra101 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
Refactored and fixed integration tests for the tap trello.
Below are the test cases verified
Jira Link: SAC-29835
Manual QA steps
Risks
Rollback steps
AI generated code
https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code