add test for deterministic deflate output#461
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
85f235c to
b28fe20
Compare
| const DATA_A: &[u8] = b"\0AAAA\0AAAAAAAA"; | ||
| const DATA_B: &[u8] = &[1u8; DATA_A.len() + 1]; | ||
|
|
||
| pub unsafe fn compress_data(src: &[u8], zs: &mut z_stream) -> Vec<u8> { |
There was a problem hiding this comment.
This function can be simplified, e.g. as in https://github.com/zlib-ng/zlib-ng/pull/2102/files
There was a problem hiding this comment.
Sure, I'll hold off on that until they merge the PR.
Even then we're running into some failing tests here, so we'll have to wait for a new libz-ng release before we can merge this PR.
|
The zlib-ng fix has just been merged to the develop branch: zlib-ng/zlib-ng#2102 |
b28fe20 to
acb1487
Compare
|
Cool, I've updated the implementation here but we'd need a release before we can merge here (plus I'd need to update |
acb1487 to
0ed5ebf
Compare
d65d0d8 to
1c2609e
Compare
|
The |
Cargo.toml
Outdated
| libz-sys = { version = "1.1.23", default-features = false, features = ["zlib-ng"] } # use libz-ng in libz compat mode | ||
| arbitrary = { version = "1.0" } | ||
| quickcheck = { version = "1.0.3", default-features = false, features = [] } | ||
| getrandom = "0.2.17" # work around https://github.com/rust-random/getrandom/issues/775 |
There was a problem hiding this comment.
I did say this is the proper fix. Something is broken with the older version, so you effectively have a dependency on the newer version already, this is just declaring said dependency.
| getrandom = "0.2.17" # work around https://github.com/rust-random/getrandom/issues/775 | |
| getrandom = "0.2.17" # Earlier getrandom versions contain UB: https://github.com/rust-random/getrandom/issues/775 |
Does specifying this in workspace.dependencies have any effect though? Aren't workspace.dependencies and workspace.package just inert templates that an actual package can use with foo.workspace = true? Maybe move it to the dev-dependencies section of test-libz-rs-sys/Cargo.toml ?
There was a problem hiding this comment.
It doesn't, it's already bumped in the Cargo.lock file in d1aad5e, so this is not really needed.
compressing an input after a `reset` should give the same output as if it was done with a fresh compression state. There was a bug where some of the state from before the reset could 'leak' into influencing what match was picked. The output was still valid, but it is inconsistent.
1c2609e to
8928095
Compare
fixes #459
Compressing an input after a
resetshould give the same output as if it was done with a fresh compression state. There was a bug where some of the state from before the reset could influence what match was picked for later input. The output was still valid, but it is inconsistent.Thank you @QrczakMK for the detective work here!