Test cases do not include multiple metablocks with data #63

New Issue

claunia · 2026-01-29T20:30:48Z

claunia commented

2026-01-29 20:30:48 +00:00

Originally created by @ende76 on GitHub (Nov 10, 2015).

Unless my analysis is incorrect, all of the test cases with multiple metablocks have the compressed data contained in just one of those metablocks.
Consequently, the tests don't seem to cover a case where

backreferences for copy literals span multiple metablocks,
p1, p2 refer to literals in a previous metablock,
distance codes refer to distances in the distance ring buffer that have been inserted in a previous metablock.

That means that erikzenker's PR #258 passes, while I believe it should fail.

Originally created by @ende76 on GitHub (Nov 10, 2015). Unless my analysis is incorrect, all of the test cases with multiple metablocks have the compressed data contained in just one of those metablocks. Consequently, the tests don't seem to cover a case where - backreferences for copy literals span multiple metablocks, - p1, p2 refer to literals in a previous metablock, - distance codes refer to distances in the distance ring buffer that have been inserted in a previous metablock. That means that erikzenker's PR #258 passes, while I believe it should fail.

claunia closed this issue

2026-01-29 20:30:48 +00:00

claunia commented

2026-01-29 20:30:50 +00:00

@ende76 commented on GitHub (Nov 10, 2015):

@szabadka You have already addressed my immediate concern with PR #258 (I was wrong there).

I still think it would be good to have a test case with multiple non-empty metablocks, but I don't think it's as much of a pressing matter any more.

I would not object to closing this as not important enough to warrant constructing a test case, possibly manually.

@ende76 commented on GitHub (Nov 10, 2015): @szabadka You have already addressed my immediate concern with PR #258 (I was wrong there). I still think it would be good to have a test case with multiple non-empty metablocks, but I don't think it's as much of a pressing matter any more. I would not object to closing this as not important enough to warrant constructing a test case, possibly manually.

claunia commented

2026-01-29 20:30:52 +00:00

@ende76 commented on GitHub (Nov 10, 2015):

I'm actually going to close this. I think, constructing a viable test case would be unnecessarily tedious, while providing no value for encoders, and only very limited value for decoders, since multiple metablocks that are non-empty are not that much more informative to a working decoder than just general multiple metablocks.

@ende76 commented on GitHub (Nov 10, 2015): I'm actually going to close this. I think, constructing a viable test case would be unnecessarily tedious, while providing no value for encoders, and only very limited value for decoders, since multiple metablocks that are non-empty are not that much more informative to a working decoder than just general multiple metablocks.

claunia commented

2026-01-29 20:30:53 +00:00

@dsnet commented on GitHub (Nov 11, 2015):

I think it would be helpful to have series of small brotli files that tests the edge conditions of the format where a decoder could implement it wrong.

I have generated some files by hand; you can see them in TestReader. So far, I only have tests that strongly exercise Sections 3.4. and 3.5, but plan on adding more tests in the near future. If you have any good test files (both valid and invalid), please share them :)

@dsnet commented on GitHub (Nov 11, 2015): I think it would be helpful to have series of small brotli files that tests the edge conditions of the format where a decoder could implement it wrong. I have [generated some files by hand; you can see them in TestReader](https://github.com/dsnet/compress/blob/master/brotli/reader_test.go). So far, I only have tests that strongly exercise Sections 3.4. and 3.5, but plan on adding more tests in the near future. If you have any good test files (both valid and invalid), please share them :)

claunia commented

2026-01-29 20:30:55 +00:00

@ende76 commented on GitHub (Nov 12, 2015):

@dsnet I do have a number of test files that test for mistakes I've made. They're almost all invalid files, because my mistakes were mostly in detecting as early as possible when I could dismiss a stream. I'll put them together within the next few days.

@ende76 commented on GitHub (Nov 12, 2015): @dsnet I do have a number of test files that test for mistakes I've made. They're almost all invalid files, because my mistakes were mostly in detecting as early as possible when I could dismiss a stream. I'll put them together within the next few days.

claunia referenced this issue

2026-01-29 20:48:02 +00:00

[PR #63] [MERGED] Proof-of-concept encoder for parallel compression. #652

claunia referenced this issue

2026-01-29 20:48:03 +00:00

[PR #63] Proof-of-concept encoder for parallel compression. #657

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/brotli#63