xiangfu0 commented on code in PR #20016:
URL: https://github.com/apache/pinot/pull/20016#discussion_r4363749654
##########
pinot-common/src/test/java/org/apache/pinot/common/utils/TarCompressionUtilsTest.java:
##########
@@ -128,7 +128,10 @@ public void testLz4ArchiveRoundTrip(String compressorName)
if (compressorName.equals(LZ4_FAST)) {
File defaultArchive = new File(TAR_DIR, "default" +
TarCompressionUtils.TAR_LZ4_FILE_EXTENSION);
TarCompressionUtils.createCompressedTarFile(inputFile, defaultArchive);
- assertEquals(Files.readAllBytes(defaultArchive.toPath()),
Files.readAllBytes(archive.toPath()));
+ // Tar entry headers record the input file's access time, which reading
it for the first archive can advance, so
+ // the two archives may differ. Re-encode the default archive's own tar
stream to verify its compressor instead.
+ byte[] defaultArchiveBytes = Files.readAllBytes(defaultArchive.toPath());
+ assertEquals(defaultArchiveBytes, compress(LZ4_FAST,
decompress(defaultArchiveBytes)));
Review Comment:
Just a small thought (non-blocking): this now checks that the default equals
`LZ4_FAST`'s own encoding rather than cross-checking against the
separately-built `archive`, which is actually cleaner and no longer
atime-sensitive. One nit — a short assertion message here (e.g. "default
archive should be LZ4_FAST-encoded") would make a future failure
self-explanatory, since a mismatch would otherwise just print two byte arrays.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
