Use GIX_TEST_FIXTURE_HASH for gix-archive tests - #2893
Use GIX_TEST_FIXTURE_HASH for gix-archive tests#2893Christoph Rüßler (cruessler) wants to merge 1 commit into
GIX_TEST_FIXTURE_HASH for gix-archive tests#2893Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33d1003de8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
| env GIX_TEST_FIXTURE_HASH=sha1 cargo nextest run -p gix-archive --no-default-features --features sha1,tar --no-fail-fast | ||
| env GIX_TEST_FIXTURE_HASH=sha1 cargo nextest run -p gix-archive --no-default-features --features sha1,tar_gz --no-fail-fast | ||
| env GIX_TEST_FIXTURE_HASH=sha1 cargo nextest run -p gix-archive --no-default-features --features sha1,zip --no-fail-fast | ||
| env GIX_TEST_FIXTURE_HASH=sha256 cargo nextest run -p gix-archive --features sha256 --no-fail-fast |
There was a problem hiding this comment.
On Windows/local runs where GIX_TEST_IGNORE_ARCHIVES is not set, this newly added sha256 test mode will cause scripted_fixture_read_only("basic.sh") to prefer the checked-in basic_sha256.tar instead of rerunning the fixture script. The existing fixture ignore file explicitly says basic.sh uses symlinks and executable bits that cannot be round-tripped from archives on every supported platform (gix-archive/tests/fixtures/generated-archives/.gitignore:1-5), and the helper extracts existing archives by default (tests/tools/src/lib.rs:705-707), so the sha256 archive path reintroduces the non-portable fixture behavior the sha1 path avoids.
Useful? React with 👍 / 👎.
| #[cfg(target_pointer_width = "32")] | ||
| let expected_buffer_length = match gix_testtools::object_hash() { | ||
| gix_hash::Kind::Sha1 => 479, | ||
| gix_hash::Kind::Sha256 => todo!("let the test fail on CI and add the value here"), |
There was a problem hiding this comment.
On 32-bit targets with GIX_TEST_FIXTURE_HASH=sha256 (the justfile now invokes that mode), this todo! panics before the assertion can run, so the gix-archive sha256 tests cannot pass on i686/armv7. The expected length is deterministic here — the sha1 value plus nine entries times the 12 extra SHA-256 id bytes — so this should use the actual 32-bit value instead of a runtime placeholder.
Useful? React with 👍 / 👎.
33d1003 to
4a0ca97
Compare
1 participant
This PR makes
gix-archivetests run withGIX_TEST_FIXTURE_HASH=sha256.The changes are pretty straightforward, but there’s one thing I’m not sure about:
justfilehas 5 newcargo nextestentries in this initial version. If you feel this is unwarranted, feel free to remove those that are not needed.In this PR, I tried to follow the patterns used in 3bca1a1.
Some of the code changes, in particular the exact SHA-1 to SHA-256 mappings, were drafted by GPT-5.5.