-
-
Notifications
You must be signed in to change notification settings - Fork 122
Normalize topics names #717
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.
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
Conversation
There was a problem hiding this 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 refactors test topic names to use dynamically generated UUIDs instead of hardcoded strings. The goal is to normalize topic names across the test suite to avoid conflicts and ensure test isolation.
Key changes:
- Introduces a
TestTopicsmodule that generates unique topic names with UUID suffixes - Replaces all hardcoded topic name strings throughout the test suite with calls to the new module
- Updates topic creation logic in the test setup to use the new dynamic topic names
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 |
|---|---|
| spec/spec_helper.rb | Adds TestTopics module for dynamic topic generation and updates topic creation setup |
| spec/lib/rdkafka/producer_spec.rb | Replaces hardcoded topic names with TestTopics method calls |
| spec/lib/rdkafka/producer/partitions_count_cache_spec.rb | Updates topic variables to use TestTopics.unique |
| spec/lib/rdkafka/producer/delivery_report_spec.rb | Updates topic name to use TestTopics.unique |
| spec/lib/rdkafka/producer/delivery_handle_spec.rb | Updates topic references to use TestTopics methods |
| spec/lib/rdkafka/metadata_spec.rb | Updates topic references to use TestTopics methods |
| spec/lib/rdkafka/consumer_spec.rb | Updates all topic references to use TestTopics methods |
| spec/lib/rdkafka/admin_spec.rb | Updates topic references to use TestTopics methods |
| spec/lib/rdkafka/admin/*_spec.rb | Updates topic references and removes unnecessary require statements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| retry | ||
| end | ||
|
|
||
| def notify_listener(listener, &block) |
Copilot
AI
Oct 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The consumer variable is not defined in this context. This line appears to be within the notify_listener function but no consumer is being passed to the function or created locally.
| def notify_listener(listener, &block) | |
| def notify_listener(consumer, &block) |
| TestTopics.example_topic => 1 | ||
| }.each do |topic, partitions| | ||
| create_topic_handle = admin.create_topic(topic.to_s, partitions, 1) | ||
| create_topic_handle = admin.create_topic(topic, partitions, 1) |
Copilot
AI
Oct 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Removing the .to_s call may cause issues if the TestTopics methods ever return non-string values. Consider adding explicit string conversion for safety.
| create_topic_handle = admin.create_topic(topic, partitions, 1) | |
| create_topic_handle = admin.create_topic(topic.to_s, partitions, 1) |
* small api syncs (karafka#706) * small api syncs * fix branch ref * Update confluentinc/cp-kafka Docker tag to v8.0.1 (karafka#708) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update ruby/setup-ruby action to v1.260.0 (karafka#707) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update ruby/setup-ruby action to v1.261.0 (karafka#709) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update ruby/setup-ruby action to v1.262.0 (karafka#710) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update ruby/setup-ruby action to v1.263.0 (karafka#713) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update actions/cache action to v4.3.0 (karafka#714) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * verify kafka warnings, verify features (karafka#715) * Normalize topics names (karafka#717) * normalize topics names * small remarks --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* small api syncs (karafka#706) * small api syncs * fix branch ref * Update confluentinc/cp-kafka Docker tag to v8.0.1 (karafka#708) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update ruby/setup-ruby action to v1.260.0 (karafka#707) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update ruby/setup-ruby action to v1.261.0 (karafka#709) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update ruby/setup-ruby action to v1.262.0 (karafka#710) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update ruby/setup-ruby action to v1.263.0 (karafka#713) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update actions/cache action to v4.3.0 (karafka#714) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * verify kafka warnings, verify features (karafka#715) * Normalize topics names (karafka#717) * normalize topics names * small remarks * stabilize spec (karafka#718) * Update dependency ruby to v3.4.7 (karafka#721) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update peter-evans/repository-dispatch action to v4 (karafka#722) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * v0.22.1 --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
close #621
close #614