Skip to content

PYTHON-5724 Add integration test exercising compression logic - #2935

Open
aclark4life wants to merge 8 commits into
mongodb:mainfrom
aclark4life:PYTHON-5724
Open

PYTHON-5724 Add integration test exercising compression logic#2935
aclark4life wants to merge 8 commits into
mongodb:mainfrom
aclark4life:PYTHON-5724

Conversation

@aclark4life

@aclark4life aclark4life commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

PYTHON-5724

Changes in this PR

Adds an integration test that sends real commands with each available wire-protocol compressor enabled, exercising the end-to-end compress/decompress path in compression_support.py.

  • Adds test_compression_commands in test/asynchronous/test_client.py (sync mirror generated by just synchro).

Test Plan

For each available compressor, the test handshakes so the server negotiates it, then asserts the pooled connection received the correct compressor context type:

Compressor Context class compressor_id
snappy SnappyContext 1
zlib ZlibContext 2
zstd ZstdContext 3

It then round-trips an insert/find of compressible data with both directions instrumented:

  • Send path: spies on the connection's compression_context.compress() and asserts it was invoked.
  • Receive path: wraps pymongo.network_layer.decompress (the symbol both the sync and async read paths call) and asserts it was invoked with the expected compressor_id. A plain equality check on the returned document would pass whether or not the reply was compressed, so the spy is what makes this a real assertion about decompression.

If the server negotiates no compression at all, the test reports as skipped rather than passing with nothing exercised. Compressors whose libraries are unavailable are not attempted; zlib is always tried since it is stdlib.

Verified locally against a standalone server: 2 passed, 2 subtests passed (zlib only — snappy/zstd were not installed in that venv, so those branches are covered by the CI variants that install those extras).

Checklist

Checklist for Author

  • Did you update the changelog (if necessary)? N/A — test-only change.
  • Is there test coverage? This PR is test coverage for compression_support.py.
  • Is any followup work tracked in a JIRA ticket? If so, add link(s). N/A.

Checklist for Reviewer

  • Does the title of the PR reference a JIRA Ticket?
  • Do you fully understand the implementation? (Would you be comfortable explaining how this code works to someone else?)
  • Is all relevant documentation (README or docstring) updated?

@aclark4life
aclark4life force-pushed the PYTHON-5724 branch 2 times, most recently from c499d8c to d404c2f Compare July 14, 2026 16:08
@aclark4life
aclark4life requested a review from Copilot July 14, 2026 16:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds new integration coverage to validate wire-protocol compression behavior by running real commands with each negotiated compressor, aiming to exercise the compress/decompress path used by the driver’s networking stack.

Changes:

  • Import concrete compression context classes (Snappy/Zlib/Zstd) into the client test modules for type assertions.
  • Add test_compression_commands (sync + async) to negotiate each available compressor, assert the negotiated context type, and spy on compress() during a CRUD round-trip.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
test/test_client.py Adds a sync integration test that negotiates compressors and verifies compression is used during command execution.
test/asynchronous/test_client.py Adds the async equivalent integration test for compressor negotiation and command execution.

Comment thread test/test_client.py Outdated
Comment thread test/asynchronous/test_client.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread test/asynchronous/test_client.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

test/asynchronous/test_client.py:1908

  • Each subtest creates a new AsyncMongoClient, but it isn’t closed after the assertions. Since this test also temporarily patches a module-level function (network_layer.decompress), leaving prior clients running can introduce cross-talk between subtests (background monitoring/decompression) and make the compressor-id assertions flaky. Closing the client in a finally block keeps subtests isolated even if an assertion fails.
                # Round-trip a command large enough to compress.
                coll = client.pymongo_test.test_compression
                await coll.drop()
                with patch.object(network_layer, "decompress", decompress_spy):
                    await coll.insert_one({"x": "y" * 1024})

Comment thread test/asynchronous/test_client.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@aclark4life
aclark4life marked this pull request as ready for review July 27, 2026 18:26
@aclark4life
aclark4life requested a review from a team as a code owner July 27, 2026 18:26
@aclark4life
aclark4life requested a review from NoahStapp July 27, 2026 18:26
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.

3 participants