SAC-28726 - Update python version and add tests#48
Open
MuralidharT03 wants to merge 3 commits intoSAC-28845-add-new-metadatafrom
Open
SAC-28726 - Update python version and add tests#48MuralidharT03 wants to merge 3 commits intoSAC-28845-add-new-metadatafrom
MuralidharT03 wants to merge 3 commits intoSAC-28845-add-new-metadatafrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the tap’s runtime/tooling to a newer Python version and adds a comprehensive set of unit + mock-integration tests to validate transform, discovery, pagination, bookmarking, and start-date behavior.
Changes:
- Bump CI to Python 3.12 and expand CI test execution to run all tests under
tests/. - Add new unit tests for
client,transform, andsyncutility functions, plus mock integration tests for discovery/sync behavior. - Adjust JSON schemas to allow certain nested objects to be nullable.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
setup.py |
Updates pinned dependencies and adds test/dev dependencies. |
.circleci/config.yml |
Switches CI venv to Python 3.12 and runs full test suite with coverage. |
tests/unittests/test_transform.py |
Unit coverage for camelCase conversion and transform helpers. |
tests/unittests/test_sync_utils.py |
Unit coverage for state/bookmark helpers and datetime transform. |
tests/unittests/test_default_start_date.py |
Refactors start-date clamp test to use fixed reference time. |
tests/unittests/test_client.py |
Unit coverage for HTTP error handling and client base URL construction. |
tests/base.py |
Adds shared mock integration test mixin + schema-driven record generator. |
tests/test_discovery.py |
Mock integration coverage for stream discovery + metadata correctness. |
tests/test_pagination.py |
Mock integration coverage for multi-page and empty pagination behavior. |
tests/test_start_date.py |
Mock integration coverage for incremental start-date filtering behavior. |
tests/test_interrupted_sync.py |
Mock integration coverage for resume behavior via bookmarks/state. |
tests/test_bookmark.py |
Mock integration coverage for bookmark updates and isolation across streams. |
tests/test_automatic_fields.py |
Ensures PK/replication keys are always present even with minimal selection. |
tests/test_all_fields.py |
Ensures schema fields appear in emitted records when all fields are selected. |
tap_impact/schemas/company_information.json |
Makes several nested object fields nullable. |
tap_impact/schemas/campaigns.json |
Makes categories nullable. |
Comments suppressed due to low confidence (2)
.circleci/config.yml:35
- The job stores test results at
test_output/report.xml, but the pytest command doesn’t generate any JUnit XML output. As written,store_test_resultswill likely upload nothing. Consider adding--junitxml test_output/report.xml(and creating the directory) or changingstore_test_resultsto point at a directory that pytest actually writes into.
name: 'Unit Tests + Integration Tests'
command: |
source /usr/local/share/virtualenvs/tap-impact/bin/activate
uv pip install pytest coverage parameterized
coverage run -m pytest tests -v
coverage html
when: always
- store_test_results:
path: test_output/report.xml
tests/unittests/test_default_start_date.py:45
- This test re-implements the “3-years-ago clamp” logic locally using a fixed
_FIXED_NOW, but it doesn’t exercise the production code that applies the clamp (it only callsget_bookmark, which doesn’t do any date clamping). To make this test meaningful, consider factoring the clamping logic into a helper function intap_impact.syncand testing that directly, or patchtap_impact.sync.utils.nowand call the function/path that performs the clamp.
_FIXED_NOW = datetime(2024, 6, 15, 12, 0, 0, tzinfo=timezone.utc)
_30_DAYS_AGO = (_FIXED_NOW - timedelta(days=30)).strftime('%Y-%m-%dT%H:%M:%SZ')
_3_YEARS_AGO = (_FIXED_NOW - timedelta(days=3 * 365)).strftime('%Y-%m-%dT%H:%M:%SZ')
class TestBookmarkDateHandling(unittest.TestCase):
def setUp(self):
"""Set up commonly used variables using the fixed reference time."""
self.now = _FIXED_NOW
self.default_date = self.now - timedelta(days=3 * 365)
self.default_date_str = self.default_date.strftime('%Y-%m-%dT%H:%M:%SZ')
@parameterized.expand([
# Test case 1: Start date within 3 years
({}, 'actions', _30_DAYS_AGO, _30_DAYS_AGO),
# Test case 2: Bookmark date within 3 years but start date is older
({"bookmarks": {"actions": _30_DAYS_AGO}}, 'actions', '2014-01-01T00:00:00Z', _30_DAYS_AGO),
# Test case 3: Start date older than 3 years — clamps to default (3 years ago)
({}, 'actions', '2019-01-01T00:00:00Z', _3_YEARS_AGO),
# Test case 4: Bookmark date older than 3 years — clamps to default
({"bookmarks": {"actions": "2020-01-01T00:00:00Z"}}, 'actions', '2019-01-01T00:00:00Z', _3_YEARS_AGO),
])
def test_bookmark_date(self, state, stream_name, start_date, expected_datetime):
"""Test actions stream with various start dates and bookmark handling."""
# Get the actual bookmark datetime returned by the function
last_datetime = get_bookmark(state, stream_name, start_date)
# Convert the returned datetime string to a datetime object
last_datetime_dt = datetime.fromisoformat(last_datetime.replace('Z', '+00:00'))
# Determine if the stream should use the default_date based on the logic
if stream_name in ('actions', 'action_updates') and last_datetime_dt < self.default_date:
last_datetime = self.default_date_str
# Perform the test assertion
self.assertEqual(last_datetime, expected_datetime)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
RushiT0122
approved these changes
Apr 1, 2026
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
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
Updates the tap’s runtime to a newer Python version and adds a comprehensive set of unit + mock-integration tests to validate transform, discovery, pagination, bookmarking, and start-date behavior.
Jira ticket: SAC-28726
Changes:
tests/.client,transform, andsyncutility functions, plus mock integration tests for discovery/sync behavior.Manual QA steps
Risks
Rollback steps
AI generated code
https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code