Skip to content

PYTHON-5382 - Add a test with min dependencies #2410

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Jun 27, 2025
Merged

PYTHON-5382 - Add a test with min dependencies #2410

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e84fb43
PYTHON-5382 - Add a test with min dependencies
NoahStapp Jun 26, 2025
88c0be9
test
NoahStapp Jun 26, 2025
e7a8ce3
test
NoahStapp Jun 26, 2025
2bae838
test
NoahStapp Jun 26, 2025
05d0398
test
NoahStapp Jun 26, 2025
0fab8f7
test
NoahStapp Jun 26, 2025
1241795
uv sync pulls in other dependencies
NoahStapp Jun 26, 2025
5c0fab6
Fix uv resolution + use SRV tests
NoahStapp Jun 26, 2025
d53eee5
Update test_import_dns_resolver for dnspython 1.X
NoahStapp Jun 27, 2025
53626b7
Merge branch 'master' into PYTHON-5382
NoahStapp Jun 27, 2025
281c72a
Read-only Github permission for workflow
NoahStapp Jun 27, 2025
80667e9
Run dns tests too and async
NoahStapp Jun 27, 2025
a078102
Remove dns test for compat reasons
NoahStapp Jun 27, 2025
7d10753
Add test_minimum_for_async
NoahStapp Jun 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .github/workflows/test-python.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,3 +220,28 @@ jobs:
which python
pip install -e ".[test]"
PYMONGO_MUST_CONNECT=1 pytest -v -k client_context

test_minimum:
permissions:
contents: read
runs-on: ubuntu-latest
name: Test using minimum dependencies and supported Python
steps:
- uses: actions/checkout@v4
with:
persist-credentials: false
- name: Install uv
uses: astral-sh/setup-uv@f0ec1fc3b38f5e7cd731bb6ce540c5af426746bb # v5
with:
python-version: '3.9'
- name: Start MongoDB
uses: supercharge/mongodb-github-action@90004df786821b6308fb02299e5835d0dae05d0d # 1.12.0
with:
mongodb-version: 6.0
- name: Run connect test
shell: bash
run: |
uv venv
source .venv/bin/activate
uv pip install -e ".[test]" --resolution=lowest-direct
pytest -v test/test_srv_polling.py
Copy link
Member

Choose a reason for hiding this comment

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

Sorry I just noticed this was test_srv_polling but test_dns would probably be a better choice. Or both? And should we do both sync and async?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Almost all of the test_dns tests require a sharded or replica cluster. We can add it here anyway, but it'll only add a handful of tests. And good catch, we should run sync and async tests here.

Copy link
Member

Choose a reason for hiding this comment

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

Ah okay then, could you add a comment explaining why we use test_srv_polling only and not test_dns?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like test_dns's mocking is not compatible with dnspython 1.0. Perhaps it's not worth it to add in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And unfortunately there's no resolver.query method for the async resolver. I'll add a comment that the async test_dns is not compatible with dnspython 1.x, which is consistent with our docs for the async API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding a separate test for dnspython 2.0 for async support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ended up having to use dnspython 2.1.0, as 2.0.0 does not include the lifetime kwarg we pass to our resolve calls, but only for the async API.

Copy link
Member

Choose a reason for hiding this comment

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

Does that mean we have to update our docs and error message to say >=2.1 is required for async?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'll file a follow-up ticket for those changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://jira.mongodb.org/browse/PYTHON-5426

2 changes: 1 addition & 1 deletion test/asynchronous/test_srv_polling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ def test_import_dns_resolver(self):
# Regression test for PYTHON-4407
import dns.resolver

self.assertTrue(hasattr(dns.resolver, "resolve"))
self.assertTrue(hasattr(dns.resolver, "resolve") or hasattr(dns.resolver, "query"))


if __name__ == "__main__":
Expand Down
2 changes: 1 addition & 1 deletion test/test_srv_polling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ def test_import_dns_resolver(self):
# Regression test for PYTHON-4407
import dns.resolver

self.assertTrue(hasattr(dns.resolver, "resolve"))
self.assertTrue(hasattr(dns.resolver, "resolve") or hasattr(dns.resolver, "query"))


if __name__ == "__main__":
Expand Down
Loading