Skip to content

Conversation

fangnx
Copy link
Member

@fangnx fangnx commented Aug 26, 2025

What

Checklist

  • Contains customer facing changes? Including API/behavior changes
  • Did you add sufficient unit test and/or integration test coverage for this PR?
    • If not, please explain why it is not required

References

JIRA: https://confluentinc.atlassian.net/browse/DGS-21925

Test & Review

Open questions / Follow-ups

@fangnx fangnx requested review from a team and MSeal as code owners August 26, 2025 20:48
@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

@sonarqube-confluent

This comment has been minimized.

@sonarqube-confluent

This comment has been minimized.

@fangnx fangnx changed the title Add unit tests for asyncio producer Add unit tests for asyncio producer + consumer Aug 29, 2025
@sonarqube-confluent

This comment has been minimized.

@sonarqube-confluent

This comment has been minimized.

Copy link
Contributor

@MSeal MSeal left a comment

Choose a reason for hiding this comment

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

Over looks good, I would change the file naming convention to test_aioconsumer.py and test_aioproducer.py OR we should do a follow-up PR to change the test files to not have capital letters for classes past the test_ prefix to follow standard Python convention.


@pytest.mark.asyncio
async def test_constructor_behavior(self, mock_consumer, mock_common, basic_config):
"""Test constructor creates consumer with correct configuration."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Not quite sure what we're testing for -- that _max_workers on executor isn't overwritten by consumer max_workers?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's to cover the logic that we only set max_workers when using the default executor: https://github.com/confluentinc/confluent-kafka-python/blob/async/src/confluent_kafka/aio/_AIOConsumer.py#L24-L33. It's a pretty trivial test but it's nice to have it covered

def sync_produce(*args, **kwargs):
callback = kwargs.get('on_delivery')
if callback:
# Simulate successful delivery - callback would normally come from poll()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice setup

@fangnx
Copy link
Member Author

fangnx commented Sep 3, 2025

Over looks good, I would change the file naming convention to test_aioconsumer.py and test_aioproducer.py OR we should do a follow-up PR to change the test files to not have capital letters for classes past the test_ prefix to follow standard Python convention.

Updated the naming 👍 The repo has many other files not following the convention. I think we can raise a separate PR to clean them up

@sonarqube-confluent

This comment has been minimized.

@sonarqube-confluent
Copy link

Failed

  • 68.00% Coverage on New Code (is less than 80.00%)

Analysis Details

14 Issues

  • Bug 2 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 12 Code Smells

Coverage and Duplications

  • Coverage 68.00% Coverage (66.10% Estimated after merge)
  • Duplications No duplication information (5.30% Estimated after merge)

Project ID: confluent-kafka-python

View in SonarQube

@fangnx fangnx merged commit 1316ce9 into async Sep 3, 2025
2 of 3 checks passed
@fangnx fangnx deleted the async-producer-test branch September 3, 2025 22:59
MSeal added a commit that referenced this pull request Oct 2, 2025
* Asyncio Producer and Consumer implementation with same API as the sync ones * Add unit tests for asyncio producer + consumer (#2036) * update * update * fix producer and add consumer uts * refactor * rename * Add Benchmark Framework for ducktape (#2030) * Integrate Schema Registry with ducktape load tests (#2027) * basic sr test * more tests * update * update * lint * Add ducktape benchmark tests for consumer (sync + async) (#2045) * draft * update and cleanup * add perf comments, add batch_size param to consume test, lint fix * linter fix * Fix linter issues in consumer testing code (#2051) * Add remaining functions missing in async producer & consumer (#2050) * Add semaphore block for ducktape tests (#2037) * Add semaphore block for ducktape tests * Increase kafka start timeout * Increase kafka start timeout * Increase kafka start timeout * Add logs to debug pipeline * Start kafka in kraft mode * Fix directory failures * Fix directory failures * Fix directory failures * templatise path * Fix ductape run * Fix kafka broker listner * Fix ducktape version error * Cleanup * Fix bound voilation should fail tests * Now expand bounds for success * Add schema registry instance * Update Schema Registry hostname * Update Schema Registry hostname * Update Schema Registry hostname * Fix for linux CI environment * Address minor feedback * Fix semaphore * Minor fix after rebase * Add more async consumer unit & integration tests (#2052) * basic rebalance test * rebalance tests * refactor and linter fix * feebdack * refactor and cleanup * update * remove jit imports * Add produce batch api to producer (#2047) * Add integration tests for transactions (#2056) * add tests * cleanup and linter fix * remove jit import * refactor * cleanup * minot rlinter * Update AsyncIO producer architecture to improve performance (#2044) * Fix helper function name to avoid ducktape test discovery * Integrate schema registry with producer sync/async performance test + clean up the old SR test (#2063) * Add comprehensive producer benchmark tests with Schema Registry support - Updated message serialization to use comprehensive structure with all protobuf fields - Implemented proper strategy pattern for sync/async serializers - Added Schema Registry authentication configuration - Fixed JSON serialization issues (schema title, async serializer initialization) - Added performance validation with configurable JSON validation - Enhanced producer strategies with comprehensive Avro, JSON, and Protobuf support * remove * remove confusing msg * Minor: Producer close calls flush() (#2066) * Integrate schema registry with consumer sync/async performance test (#2059) * update * remove auth * cleanup and ensure same msg size * more cleanup * Add comprehensive producer benchmark tests with Schema Registry support - Updated message serialization to use comprehensive structure with all protobuf fields - Implemented proper strategy pattern for sync/async serializers - Added Schema Registry authentication configuration - Fixed JSON serialization issues (schema title, async serializer initialization) - Added performance validation with configurable JSON validation - Enhanced producer strategies with comprehensive Avro, JSON, and Protobuf support * update * Group messages by topic partition before passing to produce_batch API (#2069) * Merge master to async (#2068) * Pre release (#2067) * Attempting to add python versioning to read from project toml and setting beta flag * Updated docs to read project toml version as well * Updated to read from c file for now. Updaed docs and fixed bad AI code * NPI-7572: Add content for AsyncIO Python client (#2070) * Updates for AsyncIO and other improvements * Add updates based on asyncio blog * Add SR updates relatd to AsyncIO * Reorganize content, remove redundancy, and improve content * Edits to diagram and other content * Add why to use this client in both readme files * Improve CHANGELOG title * Add release dates to versions in CHANGELOG * Add release dates back to v2.4.0 * Edits based on feedback * AsyncIO: Only clear messages from buffer if executor passed (#2071) * Fix async producer transaction behavior + add transactional produce benchmark test (#2072) * update * linter fix * Fix the async transaction behavior related to flush() (#2073) * fix * linter * more linter fix * linter and add link * Removed very old librdkafka version checks * Resolved admin import conflict issue * Fix test_version unit test (#2079) * Fix broken tests (#2077) * fix tests * fix linter * Removed set operation from test --------- Co-authored-by: Matthew Seal <mseal@confluent.io> * Async fix buffer cleanup (#2078) * Fix buffer cleanup logic * Add tests * fix linter * Remove SR key * Removed incorrect assert * Change ducktape tests to install more dependencies * Fix semaphore for producer ducktape tests + clean up files that should've been removed (#2081) * update * use warning for producer validate * remove unnecessary assert --------- Co-authored-by: Emanuele Sabellico <esabellico@confluent.io> Co-authored-by: Matthew Seal <mseal@confluent.io> Co-authored-by: Kaushik Raina <103954755+k-raina@users.noreply.github.com> Co-authored-by: Matthew Seal <mseal007@gmail.com> Co-authored-by: Steve Bang <sbang@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants