-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix][test] Fix invalid test CompactionTest.testDeleteCompactedLedgerWithSlowAck #24166
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
- also replace deprecated topic naming syntax
nodece
left a comment
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.
LGTM.
However, this PR changes the topic name format from v1 to v2, which I am concerned will cause a conflict when cherry-picking this PR to the maintenance branches.
Would make sense that create a new PR to improve that?
It was necessary to change to v2 for the single test case so that topic policies can be used. Topic policies fail with v1 topics. Because of consistency, I changed the complete test class to use v2 topic format. @nodece I can handle resolving merge conflicts while cherry-picking, so no need to worry about that. I have a pretty good workflow for merge conflict resolution so that's not a big problem in this case. |
dao-jun
left a comment
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.
lgtm
…WithSlowAck (apache#24166) (cherry picked from commit 1a0f4ba)
…WithSlowAck (apache#24166) (cherry picked from commit 1a0f4ba) (cherry picked from commit b3027fa)
…WithSlowAck (apache#24166) (cherry picked from commit 1a0f4ba) (cherry picked from commit b3027fa)
…WithSlowAck (apache#24166) (cherry picked from commit 1a0f4ba) (cherry picked from commit b3027fa)
…WithSlowAck (apache#24166) (cherry picked from commit 1a0f4ba) (cherry picked from commit b3027fa)
…WithSlowAck (apache#24166) (cherry picked from commit 1a0f4ba) (cherry picked from commit b3027fa)
…WithSlowAck (apache#24166) (cherry picked from commit 1a0f4ba) (cherry picked from commit 9e79e26)
…WithSlowAck (apache#24166) (cherry picked from commit 1a0f4ba) (cherry picked from commit 9e79e26)
Motivation
The test CompactionTest.testDeleteCompactedLedgerWithSlowAck fails with PR #23980. The test appears to be invalid and needs to be revisited.
Modifications
Documentation
docdoc-requireddoc-not-neededdoc-complete