KAFKA-16689: Move LogValidatorTest to storage module#16167
Merged
chia7712 merged 30 commits intoapache:trunkfrom Aug 13, 2024
Merged
KAFKA-16689: Move LogValidatorTest to storage module#16167chia7712 merged 30 commits intoapache:trunkfrom
chia7712 merged 30 commits intoapache:trunkfrom
Conversation
TaiJuWu
force-pushed
the
KAFKA-16689
branch
4 times, most recently
from
c968a9d to
07dda87
Compare
TaiJuWu
changed the title
Collaborator
Author
|
@chia7712 Please take look! Thanks. |
chia7712
reviewed
Jun 13, 2024
chia7712
reviewed
Jun 19, 2024
Collaborator
Author
|
Hi @chia7712 , thanks for your review. |
chia7712
reviewed
Jun 23, 2024
Address below comments 1. replace time with Duration 2. Delete iterToStream 3. remove unused variable 4. Inline createNonIncreasingOffsetRecords
chia7712
reviewed
Jun 27, 2024
chia7712
reviewed
Jul 1, 2024
TaiJuWu
force-pushed
the
KAFKA-16689
branch
3 times, most recently
from
e725614 to
eb33904
Compare
…endency" This reverts commit 51bffc7.
chia7712
reviewed
Jul 8, 2024
| try { | ||
| record.ensureValid(); | ||
| } catch (InvalidRecordException e) { | ||
| } catch (InvalidRecordException | CorruptRecordException e) { |
Collaborator
Author
There was a problem hiding this comment.
here is the exception should be thrown so we need to add it.
Member
There was a problem hiding this comment.
My point was why we need to change the production code in this test migration?
Collaborator
Author
There was a problem hiding this comment.
I can't enter recordInvalidChecksums if I don't have this modification.
Member
There was a problem hiding this comment.
ok, it seems that is another issue.
#7150 changes the thrown exception from InvalidRecordException to CorruptRecordException, and so LogValidator can't catch it to update recordInvalidChecksums. Let me file a Jira for it
Member
There was a problem hiding this comment.
@TaiJuWu I have assigned https://issues.apache.org/jira/browse/KAFKA-17104 to you
| } | ||
|
|
||
| @Test | ||
| public void testInvalidSequence() { |
Member
There was a problem hiding this comment.
nice one. could you please use EnumSource to rewrite it? for example:
@ParameterizedTest
@EnumSource(CompressionType.class)
public void testInvalidSequenceV0(CompressionType type) {
checkInvalidSequence(RecordBatch.MAGIC_VALUE_V0, type);
}
@ParameterizedTest
@EnumSource(CompressionType.class)
public void testInvalidSequenceV1(CompressionType type) {
checkInvalidSequence(RecordBatch.MAGIC_VALUE_V2, type);
}
@ParameterizedTest
@EnumSource(CompressionType.class)
public void testInvalidSequenceV2(CompressionType type) {
checkInvalidSequence(RecordBatch.MAGIC_VALUE_V2, type);
}
Collaborator
Author
There was a problem hiding this comment.
Hi @chia7712 , Thanks for review and nice suggestions.
I rewrite this part, please take a look.
chia7712
reviewed
Jul 17, 2024
3 tasks
Collaborator
Author
Member
|
@TaiJuWu please fix the conflicts |
Member
|
@TaiJuWu please rebase code to fix build error |
Collaborator
Author
Done. |
Member
|
@TaiJuWu Could you please merge trunk to fix those known failed tests? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
*More detailed description of your change,
Rewrite LogValidatorTest.scala to java version
*Summary of testing strategy (including rationale)
Committer Checklist (excluded from commit message)