Skip to content

Add more tests for HuffmanDecoder and performance benchmark tests - #795

Merged
garydgregory merged 2 commits into
apache:masterfrom
fkjellberg:add-huffmandecoder-tests
Aug 9, 2026
Merged

Add more tests for HuffmanDecoder and performance benchmark tests#795
garydgregory merged 2 commits into
apache:masterfrom
fkjellberg:add-huffmandecoder-tests

Conversation

@fkjellberg

Copy link
Copy Markdown
Contributor

Thanks for your contribution to Apache Commons! Your help is appreciated!

Before you push a pull request, review this list:

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

I'm investigating if we can replace BinaryTree used by LhStaticHuffmanCompressorInputStream with the new generic HuffmanDecoder. ExplodingInputStream is also using a similar BinaryTree implementation and could possibly use HuffmanDecoder as well but I will investigate that later on.

There are some minor differences between the generic HuffmanDecoder and how huffman trees are stored for LHA and I will create another PR to fix this once this PR has been merged.

This PR just adds more unit tests for the HuffmanDecoder and performance benchmark tests.

@garydgregory garydgregory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @fkjellberg
Thank you for the PR. Please see my comments.

Comment thread src/test/java/org/apache/commons/compress/huffman/HuffmanDecoderTest.java Outdated
Comment thread src/test/java/org/apache/commons/compress/huffman/HuffmanDecoderTest.java Outdated
Comment thread src/test/java/org/apache/commons/compress/huffman/HuffmanDecoderTest.java Outdated
Comment thread src/test/java/org/apache/commons/compress/huffman/HuffmanDecoderTest.java Outdated
Comment thread src/test/java/org/apache/commons/compress/huffman/HuffmanDecoderTest.java Outdated
@fkjellberg
fkjellberg force-pushed the add-huffmandecoder-tests branch from d1cce2e to ad0400e Compare August 8, 2026 21:35
@fkjellberg

Copy link
Copy Markdown
Contributor Author

@garydgregory I updated the PR to use assertThrows

@garydgregory
garydgregory merged commit 1ef1b86 into apache:master Aug 9, 2026
22 of 23 checks passed
@garydgregory

Copy link
Copy Markdown
Member

Thank you @fkjellberg , merged 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants