garydgregory commented on code in PR #699:
URL: https://github.com/apache/commons-compress/pull/699#discussion_r2303619091
##########
src/test/java/org/apache/commons/compress/compressors/bzip2/BZip2CompressorInputStreamTest.java:
##########
@@ -149,4 +160,103 @@ void testSingleByteReadConsistentlyReturnsMinusOneAtEof()
throws IOException {
assertEquals(-1, in.read());
}
}
+
+ @Test
+ void testCreateHuffmanDecodingTablesWithLargeAlphaSize() {
+ final Data data = new Data(1);
+ // Use a codeLengths array with length equal to MAX_ALPHA_SIZE (258)
to test array bounds.
+ final char[] codeLengths = new char[258];
+ for (int i = 0; i < codeLengths.length; i++) {
+ // Use all code lengths within valid range [1, 20]
+ codeLengths[i] = (char) ((i % MAX_CODE_LEN) + 1);
+ }
+ data.temp_charArray2d[0] = codeLengths;
+ assertDoesNotThrow(
+ () ->
BZip2CompressorInputStream.createHuffmanDecodingTables(codeLengths.length, 1,
data),
+ "createHuffmanDecodingTables should not throw for valid
codeLengths array of MAX_ALPHA_SIZE");
+ assertEquals(data.minLens[0], 1, "Minimum code length should be 1");
+ }
+
+ @ParameterizedTest(name = "code length {0} -> must be rejected")
+ @ValueSource(ints = {MIN_CODE_LEN - 1, MAX_CODE_LEN + 1})
+ void testRecvDecodingTablesWithOutOfRangeCodeLength(final int codeLength)
throws IOException {
+ try (BitInputStream tables = prepareDecodingTables(codeLength)) {
+ final Data data = new Data(1);
+
+ final CompressorException ex = assertThrows(
+ CompressorException.class,
+ () ->
BZip2CompressorInputStream.recvDecodingTables(tables, data),
+ "Expected CompressorException for invalid code length " +
codeLength);
+
+ final String msg = ex.getMessage();
+ assertAll(
Review Comment:
Please don't use `assertAll()`, it's just clutter IMO. The code is clearer
and easier to maintain without using assertAll(). Should I add an assertion to
an existing assertAll(), outside of it, in a separate assertAll()? It's crazy
making IMO. We don't use assertAll() anywhere else.
Every single line of code is not expected to throw an exception; it's a
given in a unit test. If you want to document something, use an inline `//`
comment.
We do use `assertThrows()` when we _expect_ exceptions, of course.
Everything else is expected not to throw.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
