-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-18505. Handled ArrayIndexOutOfBoundsException in TestCoderBase #5065
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
base: trunk
Are you sure you want to change the base?
Conversation
|
🎊 +1 overall
This message was automatically generated. |
| if (inputs.length != decoder.getNumParityUnits() + | ||
| decoder.getNumDataUnits()) { | ||
| throw new IllegalArgumentException("Invalid inputs length"); | ||
| throw new HadoopIllegalArgumentException("Invalid inputs length"); |
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.
implicit changes in public api; revert
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.
Thanks @steveloughran for the review. I have reverted the change.
| toEraseChunks[idx ++] = dataChunks[erasedDataIndexes[i]]; | ||
| dataChunks[erasedDataIndexes[i]] = null; | ||
| } else { | ||
| throw new HadoopIllegalArgumentException( |
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.
ditto
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.
@steveloughran, sorry I don't quite understand this. Is this also a public API? I found that this is only being used in Test classes.
I have changed the exception type to IllegalArgumentException.
|
|
||
| // decode | ||
| backupAndEraseChunks(clonedDataChunks, parityChunks); | ||
| try { |
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.
use LambdaTestUtils.intercept() for all bits of test code which validates expected exceptions
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 backupAndEraseChunks method would only throw exception if the erased data index is out of bound. This is only true for the newly added value set.
Adding LambdaTestUtils.intercept() fails the tests for other parameterized values since LambdaTestUtils.intercept() @throws AssertionError if the evaluation call didn't raise an exception.
...op-common/src/test/java/org/apache/hadoop/io/erasurecode/rawcoder/TestDecodingValidator.java
Show resolved
Hide resolved
|
🎊 +1 overall
This message was automatically generated. |
Description of PR
JIRA: HADOOP-18505
How was this patch tested?
Added a new value set and consequently modified the existing unit tests
testValidateandtestValidateWithBadDecodingof test classTestDecodingValidatorto verify the code change.For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?