Run gix-refspec tests with both sha1 and sha256#2448
Run gix-refspec tests with both sha1 and sha256#2448Sebastian Thiel (Byron) merged 6 commits intoGitoxideLabs:mainfrom
gix-refspec tests with both sha1 and sha256#2448Conversation
![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: 3dcb7ee9b7
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d58a35e673
ℹ️ 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".
Christoph Rüßler (cruessler)
changed the title
GIX_TEST_FIXTURE_HASH for gix-refspecgix-refspec tests with both sha1 and sha256
Christoph Rüßler (cruessler)
force-pushed
the
add-sha-256-to-gix-refspec
branch
from
5c1e949 to
37bef49
Compare
…TURE_HASH` This is useful when tests want to inspect the hash under test.
This script now unconditionally expands `GIX_TEST_FIXTURE_HASH` under `set -u`, so running the refspec tests without explicitly exporting that variable (for example via `cargo nextest run -p gix-refspec`) aborts fixture generation before any baseline is produced. Previously the tests worked with the default hash behavior, so this change introduces a regression unless every invocation path sets the env var; please make the script treat an unset value as `sha1`.
Sebastian Thiel (Byron)
force-pushed
the
add-sha-256-to-gix-refspec
branch
from
37bef49 to
3cf53e2
Compare
|
Sorry for the late response, and thanks for keeping this topic going! As you can see, I changed my review style and create changes on top of each commit (if needed).
I think that's fine as well. Finally, I hope I will not get into the situation again that so many PRs are piling up - have to take the time now to review everything 😅. |
No worries, I’ll happily continue whenever I get a new review. :-)
|
I think I meant that |
Understood. I’ll have a look! |
|
Sebastian Thiel (@Byron) Which command did you run exactly that did not work? On my Ubuntu machine, all of the following work:
|
|
Thanks for double-checking - I retract my statement :D. Maybe I really was confused, and will take a look if it happens again. |
2 participants
This PR adds
sha256support togix-refspectests. It adds thesha256feature flag todev-dependenciesand runs the test suite forgix-refspecfor bothsha1andsha256. It adds separate branches for SHA-256 in a few places where SHA-1 hashes were hardcoded.Open questions
gix-refspec/tests/fixtures/generated-archives/.gitignoreincludes/match_baseline.tar. Does it also need to include/match_baseline_sha256.tar? (I haven’t committed the latter yet.)gix_refspec::parse::function::looks_like_object_hashusesgix_hash::Kind::shortest. Does it need to be changed? (I suspect it doesn’t.)This PR is related to #281.