Skip to content

upgrade python and added unittests#50

Merged
rsaha-qlik merged 12 commits intometadatafrom
python_update_add_unittests
Apr 20, 2026
Merged

upgrade python and added unittests#50
rsaha-qlik merged 12 commits intometadatafrom
python_update_add_unittests

Conversation

@mukeshbhatt18gl
Copy link
Copy Markdown

@mukeshbhatt18gl mukeshbhatt18gl commented Mar 24, 2026

Description of change

This PR upgrades the project’s Python runtime/dependency baseline and introduces a new unittest-based test suite to validate discovery, sync behavior, bookmarks/start-date handling, pagination, and field-selection rules.

SAC-28706

Manual QA steps

Risks

Rollback steps

  • revert this branch

AI generated code

https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code

  • this PR has been written with the help of GitHub Copilot or another generative AI tool

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR upgrades the project’s Python runtime/dependency baseline and introduces a new unittest-based test suite to validate discovery, sync behavior, bookmarks/start-date handling, pagination, and field-selection rules.

Changes:

  • Upgrade CI virtualenv Python version to 3.12 and add CI steps to run unit + broader test discovery.
  • Bump core dependency pins (backoff/requests/pendulum/singer-python).
  • Add a set of unit tests under tests/unittests/ plus additional tests under tests/ covering sync/discovery/state behaviors.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/unittests/test_sync_selection_unit.py Unit tests for stream selection/custom stream detection and export-row transformation.
tests/unittests/test_sync_helpers_unit.py Unit tests for retry interval logic and bookmark helpers.
tests/unittests/test_schema_unit.py Unit tests for schema helpers (naming, pk selection, field typing).
tests/unittests/test_discover_unit.py Unit tests for discovery metadata/keys behavior via patched schema sources.
tests/test_sync.py Higher-level sync tests with mocked client + Singer output functions.
tests/test_start_date.py Tests for start-date vs bookmark precedence for static + bulk sync paths.
tests/test_pagination.py Tests static endpoint pagination behavior and bookmark writing.
tests/test_discovery_metadata.py Tests forced replication method metadata for known streams.
tests/test_discovery_enhanced.py Broader discovery tests validating catalog structure/metadata/schemas.
tests/test_bookmark.py Tests bookmark writers + static sync bookmark usage.
tests/test_automatic_fields.py Tests that automatic fields are exported even if unselected.
tests/test_all_fields.py Tests selected-field export behavior and 250-field limit enforcement.
tests/base.py Shared base test utilities/expected metadata.
setup.py Updates pinned dependency versions.
.circleci/config.yml Switch CI to Python 3.12, update schema validation path, add unittest execution steps.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_sync.py Outdated
Comment thread tests/test_sync.py Outdated
Comment thread tests/unittests/test_sync_helpers_unit.py Outdated
Comment thread .circleci/config.yml Outdated
mukeshbhatt18gl and others added 4 commits March 24, 2026 18:16
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread tests/test_discovery_enhanced.py Outdated
Comment thread tests/test_sync.py Outdated
Comment thread tests/base.py Outdated
Comment thread tests/base.py Outdated
Comment thread setup.py
Comment thread tests/test_sync.py
Comment thread tests/test_all_fields.py Outdated
Comment thread tests/test_bookmark.py
Comment thread tap_eloqua/sync.py Outdated
Comment thread tap_eloqua/sync.py
Comment on lines 262 to 263
if status == 'success':
break
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This not required anymore since we moved this condition above. You may also need to update the elif conditions

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ideally this tap side change doesn't fall into scope of this PR, also we don't have test account to test it thoroughly, therefore I would not mind if we remove these changes to avoid any disruption on the prod connections.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Removed out of scope changes

Comment thread setup.py
Comment thread tests/base.py
Comment thread tap_eloqua/sync.py Outdated
Comment on lines +25 to +30
"""Convert a date string or pendulum DateTime to the format expected by Eloqua
API filter expressions (``YYYY-MM-DD HH:MM:SS``).

Using an explicit ``.format()`` call instead of the deprecated
``.to_datetime_string()`` guards against Pendulum 2→3 behavioural differences.
"""
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
"""Convert a date string or pendulum DateTime to the format expected by Eloqua
API filter expressions (``YYYY-MM-DD HH:MM:SS``).
Using an explicit ``.format()`` call instead of the deprecated
``.to_datetime_string()`` guards against Pendulum 23 behavioural differences.
"""
"""Convert a date string or pendulum DateTime to the format expected by Eloqua
API filter expressions (``YYYY-MM-DD HH:MM:SS``).
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also I observed that .to_datetime_string() still works

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

roll back the change.

@singer-io singer-io deleted a comment from rsaha-qlik Apr 16, 2026
@rsaha-qlik rsaha-qlik merged commit c2094f7 into metadata Apr 20, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants