Skip to content
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

Jump to bottom

test(remote_wal): add unit tests for kafka remote wal #2993

Merged
merged 34 commits into from
Jan 8, 2024
Merged

test(remote_wal): add unit tests for kafka remote wal #2993

merged 34 commits into from
Jan 8, 2024

Conversation

niebayes
Copy link
Contributor

@niebayes niebayes commented Dec 25, 2023 •
edited

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

Added unit tests for components of Kafka remote wal, including kafka log store, client manager, topic manager, etc.

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

close #2722

@niebayes niebayes self-assigned this Dec 25, 2023
@niebayes niebayes marked this pull request as draft December 25, 2023 06:23
@niebayes niebayes added the docs-not-required This change does not impact docs. label Dec 25, 2023
@codecov Codecov
Copy link

codecov bot commented Dec 25, 2023 •
edited

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (fcacb10) 85.72% compared to head (341ae67) 85.72%.

Additional details and impacted files 
@@           Coverage Diff            @@
##             main    #2993    +/-   ##
========================================
  Coverage   85.72%   85.72%            
========================================
  Files         808      810     +2     
  Lines      132543   132914   +371     
========================================
+ Hits       113618   113940   +322     
- Misses      18925    18974    +49

@niebayes niebayes changed the title test(remote_wal): prepare test files for wal test(remote_wal): integration tests Dec 25, 2023
This was referenced Dec 25, 2023
Closed
Closed
@niebayes niebayes marked this pull request as ready for review December 25, 2023 13:07
@niebayes niebayes changed the title test(remote_wal): integration tests test(remote_wal): integration tests - preview Dec 25, 2023
@niebayes niebayes marked this pull request as draft December 26, 2023 06:11
@github-actions github-actions bot added Size: M and removed docs-not-required This change does not impact docs. labels Dec 27, 2023
@niebayes niebayes closed this Dec 27, 2023
@github-actions github-actions bot added size/xs and removed Size: M labels Dec 27, 2023
@niebayes niebayes reopened this Dec 27, 2023
@github-actions github-actions bot added Size: M and removed size/xs labels Dec 27, 2023
@github-actions github-actions bot added Size: L and removed Size: M labels Dec 27, 2023
@niebayes niebayes changed the title test(remote_wal): integration tests - preview test(remote_wal): introduce kafka runtime backed by testcontainers Dec 27, 2023
@niebayes niebayes added the docs-not-required This change does not impact docs. label Dec 27, 2023
@github-actions github-actions bot removed the docs-not-required This change does not impact docs. label Dec 27, 2023
@niebayes niebayes marked this pull request as ready for review December 27, 2023 11:41
@niebayes niebayes changed the title test(remote_wal): introduce kafka runtime backed by testcontainers [WIP] test(remote_wal): introduce kafka runtime backed by testcontainers Dec 27, 2023
MichaelScofield
MichaelScofield reviewed Jan 3, 2024
View reviewed changes
src/common/test-util/src/wal/kafka/topic_decorator.rs Outdated Show resolved Hide resolved
src/log-store/src/kafka/client_manager.rs Outdated Show resolved Hide resolved
src/log-store/src/kafka/log_store.rs Outdated Show resolved Hide resolved
src/log-store/src/test_util/kafka.rs Outdated Show resolved Hide resolved
src/log-store/src/test_util/kafka.rs Show resolved Hide resolved
@niebayes
Copy link
Contributor Author

niebayes commented Jan 6, 2024

We need to skip some tests if the GT_KAFKA_ENDPOINTS environment variable is not set. The skip is powered by #3110

@WenyXu
Copy link
Member

WenyXu commented Jan 6, 2024 •
edited

We need to skip some tests if the GT_KAFKA_ENDPOINTS environment variable is not set. The skip is powered by #3110

You can cherry-pick that PR. And you are also able to drop the picked commits(using interactive rebase) if that PR is merged before yours. Without any blocking.

WenyXu
WenyXu reviewed Jan 6, 2024
View reviewed changes
src/log-store/src/kafka/log_store.rs Outdated Show resolved Hide resolved
src/log-store/src/kafka/client_manager.rs Outdated Show resolved Hide resolved
@niebayes niebayes requested a review from WenyXu January 8, 2024 03:43
@niebayes niebayes requested review from MichaelScofield and removed request for MichaelScofield January 8, 2024 06:48
MichaelScofield
MichaelScofield approved these changes Jan 8, 2024
View reviewed changes
src/common/meta/src/wal/kafka/test_util.rs Outdated Show resolved Hide resolved
src/log-store/src/test_util/kafka.rs Outdated Show resolved Hide resolved
@fengjiachun fengjiachun added this pull request to the merge queue Jan 8, 2024
Merged via the queue into main with commit 8c58d3f Jan 8, 2024
16 checks passed
@fengjiachun fengjiachun deleted the test/integrate_test_infra branch January 8, 2024 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelScofield MichaelScofield MichaelScofield approved these changes

@WenyXu WenyXu WenyXu approved these changes

@killme2008 killme2008 Awaiting requested review from killme2008

@fengjiachun fengjiachun Awaiting requested review from fengjiachun

@evenyag evenyag Awaiting requested review from evenyag

Assignees

@niebayes niebayes

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tracking issue for remote WAL based on Kafka
6 participants
@niebayes @WenyXu @MichaelScofield @evenyag @fengjiachun @waynexia