Skip to content

Commit 87433ed

Browse files
ByronByron
authored
Merge pull request #2595 from cruessler/add-hex-to-id-sha1-only
Add hex_to_id_sha1_only, use in SHA-1 only tests
2 parents 85c6087 + c547d4b commit 87433ed

11 files changed

Lines changed: 80 additions & 56 deletions

File tree

.github/copilot-instructions.md renamed to AGENTS.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,16 @@ Follow "purposeful conventional commits" style:
9494
- Run tests before making changes to understand existing issues
9595
- Use `GIX_TEST_IGNORE_ARCHIVES=1` when testing on macOS/Windows
9696
- Journey tests validate CLI behavior end-to-end
97+
- Fixture scripts should document what behavior they exercise and what makes
98+
the fixture special. Leave clear-text breadcrumbs so future readers can tell
99+
which details are essential to the test. Use markdown doc-strings when available.
100+
- Stabilize fixtures whose generated contents can vary by using the
101+
`_needs_archive` variants of functions in `gix-testtools`; these always use
102+
packaged archived fixtures instead of platform-local generated output.
103+
- Use assertion descriptions to state what is being asserted. For `assert*!`
104+
macros this is the last parameter; for `insta::assert*` macros it is the
105+
second parameter. Prefer messages that explain the invariant, not just that an
106+
assertion failed.
97107

98108
## Architecture Decisions
99109

‎gix-status/tests/status/main.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ fn hex_to_id(hex: &str) -> gix_hash::ObjectId {
115115
gix_hash::Kind::Sha256 => ObjectId::from_hex(
116116
SHA1_TO_SHA256_HASHES
117117
.get(hex)
118-
.expect("40 bytes hash to be present in mapping")
118+
.unwrap_or_else(|| panic!("SHA-1 {hex} wasn't mapped to SHA-256 yet"))
119119
.as_bytes(),
120120
)
121121
.expect("64 bytes hex"),

‎gix-traverse/tests/traverse/util.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ pub fn hex_to_id(hex: &str) -> ObjectId {
232232
gix_hash::Kind::Sha256 => ObjectId::from_hex(
233233
SHA1_TO_SHA256_HASHES
234234
.get(hex)
235-
.expect("40 bytes hash to be present in mapping")
235+
.unwrap_or_else(|| panic!("SHA-1 {hex} wasn't mapped to SHA-256 yet"))
236236
.as_bytes(),
237237
)
238238
.expect("64 bytes hex"),

‎gix/tests/gix/repository/mod.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,22 @@ mod worktree;
2727

2828
#[cfg(feature = "revision")]
2929
mod revision {
30-
use crate::util::hex_to_id;
30+
use crate::util::hex_to_id_sha1_only;
3131

3232
#[test]
3333
fn date() -> crate::Result {
3434
let repo = crate::named_repo("make_rev_parse_repo.sh")?;
3535
let actual = repo
3636
.rev_parse_single("old@{20 years ago}")
3737
.expect("it returns the oldest possible rev when overshooting");
38-
assert_eq!(actual, hex_to_id("be2f093f0588eaeb71e1eff7451b18c2a9b1d765"));
38+
assert_eq!(actual, hex_to_id_sha1_only("be2f093f0588eaeb71e1eff7451b18c2a9b1d765"));
3939

4040
let actual = repo
4141
.rev_parse_single("old@{1732184844}")
4242
.expect("it finds something in the middle");
4343
assert_eq!(
4444
actual,
45-
hex_to_id("b29405fe9147a3a366c4048fbe295ea04de40fa6"),
45+
hex_to_id_sha1_only("b29405fe9147a3a366c4048fbe295ea04de40fa6"),
4646
"It also figures out that we don't mean an index, but a date"
4747
);
4848
Ok(())

‎gix/tests/gix/revision/spec/from_bytes/ambiguous.rs‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::{
44
parse_spec, parse_spec_better_than_baseline, parse_spec_no_baseline, parse_spec_no_baseline_opts,
55
parse_spec_opts, rev_parse,
66
},
7-
util::hex_to_id,
7+
util::hex_to_id_sha1_only,
88
};
99
use gix::{
1010
prelude::{ObjectIdExt, RevSpecExt},
@@ -72,7 +72,7 @@ fn fully_failed_disambiguation_still_yields_an_ambiguity_error() {
7272
#[test]
7373
fn ranges_are_auto_disambiguated_by_committish() {
7474
let repo = repo("ambiguous_blob_tree_commit").unwrap();
75-
let id = hex_to_id("0000000000e4f9fbd19cf1e932319e5ad0d1d00b");
75+
let id = hex_to_id_sha1_only("0000000000e4f9fbd19cf1e932319e5ad0d1d00b");
7676
let expected = gix_revision::Spec::Range { from: id, to: id }.attach(&repo);
7777

7878
for spec in ["000000000..000000000", "..000000000", "000000000.."] {
@@ -103,25 +103,25 @@ fn blob_and_tree_can_be_disambiguated_by_type() {
103103

104104
assert_eq!(
105105
parse_spec("0000000000cdc^{tree}", &repo).unwrap(),
106-
Spec::from_id(hex_to_id("0000000000cdcf04beb2fab69e65622616294984").attach(&repo)),
106+
Spec::from_id(hex_to_id_sha1_only("0000000000cdcf04beb2fab69e65622616294984").attach(&repo)),
107107
"this is unambiguous anyway, but also asserts for tree which is naturally the case"
108108
);
109109

110110
assert_eq!(
111111
parse_spec_better_than_baseline("0000000000^{tree}", &repo).unwrap(),
112-
Spec::from_id(hex_to_id("0000000000cdcf04beb2fab69e65622616294984").attach(&repo)),
112+
Spec::from_id(hex_to_id_sha1_only("0000000000cdcf04beb2fab69e65622616294984").attach(&repo)),
113113
"the commit refers to the tree which also starts with this prefix, so ultimately the result is unambiguous. Git can't do that yet."
114114
);
115115

116116
assert_eq!(
117117
parse_spec("0000000000^{commit}", &repo).unwrap(),
118-
Spec::from_id(hex_to_id("0000000000e4f9fbd19cf1e932319e5ad0d1d00b").attach(&repo)),
118+
Spec::from_id(hex_to_id_sha1_only("0000000000e4f9fbd19cf1e932319e5ad0d1d00b").attach(&repo)),
119119
"disambiguation with committish"
120120
);
121121

122122
assert_eq!(
123123
parse_spec("0000000000e", &repo).unwrap(),
124-
Spec::from_id(hex_to_id("0000000000e4f9fbd19cf1e932319e5ad0d1d00b").attach(&repo)),
124+
Spec::from_id(hex_to_id_sha1_only("0000000000e4f9fbd19cf1e932319e5ad0d1d00b").attach(&repo)),
125125
"no disambiguation needed here"
126126
);
127127
}
@@ -132,7 +132,7 @@ fn trees_can_be_disambiguated_by_blob_access() {
132132
let actual = parse_spec_better_than_baseline("0000000000:a0blgqsjc", &repo).unwrap();
133133
assert_eq!(
134134
actual,
135-
Spec::from_id(hex_to_id("0000000000b36b6aa7ea4b75318ed078f55505c3").attach(&repo)),
135+
Spec::from_id(hex_to_id_sha1_only("0000000000b36b6aa7ea4b75318ed078f55505c3").attach(&repo)),
136136
"we can disambiguate by providing a path, but git cannot"
137137
);
138138
assert_eq!(
@@ -147,7 +147,7 @@ fn commits_can_be_disambiguated_with_commit_specific_transformations() {
147147
for spec in ["0000000000^0", "0000000000^{commit}"] {
148148
assert_eq!(
149149
parse_spec(spec, &repo).unwrap(),
150-
Spec::from_id(hex_to_id("0000000000e4f9fbd19cf1e932319e5ad0d1d00b").attach(&repo))
150+
Spec::from_id(hex_to_id_sha1_only("0000000000e4f9fbd19cf1e932319e5ad0d1d00b").attach(&repo))
151151
);
152152
}
153153
}
@@ -157,7 +157,7 @@ fn tags_can_be_disambiguated_with_commit_specific_transformations() {
157157
let repo = repo("ambiguous_commits").unwrap();
158158
assert_eq!(
159159
parse_spec_better_than_baseline("0000000000^{tag}", &repo).unwrap(),
160-
Spec::from_id(hex_to_id("0000000000f8f5507ab27a0d7bd3c75c0f64ffe0").attach(&repo)),
160+
Spec::from_id(hex_to_id_sha1_only("0000000000f8f5507ab27a0d7bd3c75c0f64ffe0").attach(&repo)),
161161
"disambiguation is possible by type, and git can't do that for some reason"
162162
);
163163
}
@@ -188,19 +188,19 @@ fn ambiguous_40hex_refs_are_ignored_and_we_prefer_the_object_of_the_same_name()
188188
let spec = "0000000000e4f9fbd19cf1e932319e5ad0d1d00b";
189189
assert_eq!(
190190
parse_spec(spec, &repo).unwrap(),
191-
Spec::from_id(hex_to_id(spec).attach(&repo)),
191+
Spec::from_id(hex_to_id_sha1_only(spec).attach(&repo)),
192192
"git shows an advisory here and ignores the ref, which makes it easy to just ignore it too. We are unable to show anything though, maybe traces?"
193193
);
194194

195195
assert_eq!(
196196
parse_spec_opts(spec, &repo, opts_ref_hint(RefsHint::PreferObject)).unwrap(),
197-
Spec::from_id(hex_to_id(spec).attach(&repo)),
197+
Spec::from_id(hex_to_id_sha1_only(spec).attach(&repo)),
198198
"preferring objects yields the same result here"
199199
);
200200

201201
assert_eq!(
202202
parse_spec_no_baseline_opts(spec, &repo, opts_ref_hint(RefsHint::PreferRef)).unwrap(),
203-
Spec::from_id(hex_to_id("cc60d25ccfee90e4a4105e73df36059db383d5ce").attach(&repo)),
203+
Spec::from_id(hex_to_id_sha1_only("cc60d25ccfee90e4a4105e73df36059db383d5ce").attach(&repo)),
204204
"we can prefer refs in any case, too"
205205
);
206206

@@ -219,19 +219,19 @@ fn ambiguous_short_refs_are_dereferenced() {
219219
let spec = "0000000000e";
220220
assert_eq!(
221221
parse_spec(spec, &repo).unwrap(),
222-
Spec::from_id(hex_to_id("cc60d25ccfee90e4a4105e73df36059db383d5ce").attach(&repo)),
222+
Spec::from_id(hex_to_id_sha1_only("cc60d25ccfee90e4a4105e73df36059db383d5ce").attach(&repo)),
223223
"git shows a warning here and we show nothing but have dials to control how to handle these cases"
224224
);
225225

226226
assert_eq!(
227227
parse_spec_opts(spec, &repo, opts_ref_hint(RefsHint::PreferRef)).unwrap(),
228-
Spec::from_id(hex_to_id("cc60d25ccfee90e4a4105e73df36059db383d5ce").attach(&repo)),
228+
Spec::from_id(hex_to_id_sha1_only("cc60d25ccfee90e4a4105e73df36059db383d5ce").attach(&repo)),
229229
"this does the same, but independently of the length of the ref"
230230
);
231231

232232
assert_eq!(
233233
parse_spec_no_baseline_opts(spec, &repo, opts_ref_hint(RefsHint::PreferObject)).unwrap(),
234-
Spec::from_id(hex_to_id("0000000000e4f9fbd19cf1e932319e5ad0d1d00b").attach(&repo)),
234+
Spec::from_id(hex_to_id_sha1_only("0000000000e4f9fbd19cf1e932319e5ad0d1d00b").attach(&repo)),
235235
"we can always prefer objects, too"
236236
);
237237

@@ -250,7 +250,7 @@ fn repository_local_disambiguation_hints_disambiguate() {
250250
let r = repo("ambiguous_objects_disambiguation_config_committish").unwrap();
251251
assert_eq!(
252252
rev_parse("0000000000f", &r).unwrap(),
253-
Spec::from_id(hex_to_id("0000000000f8f5507ab27a0d7bd3c75c0f64ffe0").attach(&r)),
253+
Spec::from_id(hex_to_id_sha1_only("0000000000f8f5507ab27a0d7bd3c75c0f64ffe0").attach(&r)),
254254
"we read the 'core.disambiguate' value and apply it to auto-disambiguate"
255255
);
256256
let err = rev_parse("0000000000", &r).unwrap_err();
@@ -281,7 +281,7 @@ fn repository_local_disambiguation_hints_disambiguate() {
281281
);
282282

283283
{
284-
let id = hex_to_id("00000000000434887f772f53e14e39497f7747d3");
284+
let id = hex_to_id_sha1_only("00000000000434887f772f53e14e39497f7747d3");
285285
let expected = gix_revision::Spec::Range { from: id, to: id }.attach(&r);
286286
assert_eq!(
287287
rev_parse("00000000000..00000000000", &r).unwrap(),
@@ -293,7 +293,7 @@ fn repository_local_disambiguation_hints_disambiguate() {
293293
let r = repo("ambiguous_objects_disambiguation_config_tree").unwrap();
294294
assert_eq!(
295295
rev_parse("0000000000f", &r).unwrap(),
296-
Spec::from_id(hex_to_id("0000000000fd8bcc566027a4d16bde8434cac1a4").attach(&r)),
296+
Spec::from_id(hex_to_id_sha1_only("0000000000fd8bcc566027a4d16bde8434cac1a4").attach(&r)),
297297
"disambiguation may work precisely even with a simple object type constraint"
298298
);
299299

@@ -310,7 +310,7 @@ fn repository_local_disambiguation_hints_disambiguate() {
310310
let r = repo("ambiguous_objects_disambiguation_config_blob").unwrap();
311311
assert_eq!(
312312
rev_parse("0000000000f", &r).unwrap(),
313-
Spec::from_id(hex_to_id("0000000000f2fdf63f36c0d76aece18a79ab64f2").attach(&r)),
313+
Spec::from_id(hex_to_id_sha1_only("0000000000f2fdf63f36c0d76aece18a79ab64f2").attach(&r)),
314314
);
315315
}
316316

‎gix/tests/gix/revision/spec/from_bytes/mod.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use gix::{prelude::ObjectIdExt, revision::Spec};
22
pub use util::*;
33

4-
use crate::util::hex_to_id;
4+
use crate::util::hex_to_id_sha1_only;
55

66
mod ambiguous;
77
mod regex;
@@ -15,7 +15,7 @@ mod peel;
1515
mod sibling_branch {
1616
use crate::{
1717
revision::spec::from_bytes::{parse_spec, repo},
18-
util::hex_to_id,
18+
util::hex_to_id_sha1_only,
1919
};
2020

2121
#[test]
@@ -31,7 +31,7 @@ mod sibling_branch {
3131
assert_eq!(actual.second_reference(), None);
3232
assert_eq!(
3333
actual.single().expect("just one"),
34-
hex_to_id("55e825ebe8fd2ff78cad3826afb696b96b576a7e")
34+
hex_to_id_sha1_only("55e825ebe8fd2ff78cad3826afb696b96b576a7e")
3535
);
3636
}
3737
}
@@ -44,7 +44,7 @@ mod index {
4444

4545
use crate::{
4646
revision::spec::from_bytes::{parse_spec, repo},
47-
util::hex_to_id,
47+
util::hex_to_id_sha1_only,
4848
};
4949

5050
#[test]
@@ -53,7 +53,7 @@ mod index {
5353
let actual = parse_spec(":file", &repo).unwrap();
5454
assert_eq!(
5555
actual,
56-
Spec::from_id(hex_to_id("fe27474251f7f8368742f01fbd3bd5666b630a82").attach(&repo))
56+
Spec::from_id(hex_to_id_sha1_only("fe27474251f7f8368742f01fbd3bd5666b630a82").attach(&repo))
5757
);
5858
assert_eq!(
5959
actual.path_and_mode().expect("set"),
@@ -115,7 +115,7 @@ fn bad_objects_are_valid_until_they_are_actually_read_from_the_odb() {
115115
let repo = repo("blob.bad").unwrap();
116116
assert_eq!(
117117
parse_spec("e328", &repo).unwrap(),
118-
Spec::from_id(hex_to_id("e32851d29feb48953c6f40b2e06d630a3c49608a").attach(&repo)),
118+
Spec::from_id(hex_to_id_sha1_only("e32851d29feb48953c6f40b2e06d630a3c49608a").attach(&repo)),
119119
"we are able to return objects even though they are 'bad' when trying to decode them, like git",
120120
);
121121
let err = parse_spec("e328^{object}", &repo).unwrap_err();
@@ -139,7 +139,7 @@ fn bad_objects_are_valid_until_they_are_actually_read_from_the_odb() {
139139
let repo = repo("blob.corrupt").unwrap();
140140
assert_eq!(
141141
parse_spec("cafea", &repo).unwrap(),
142-
Spec::from_id(hex_to_id("cafea31147e840161a1860c50af999917ae1536b").attach(&repo))
142+
Spec::from_id(hex_to_id_sha1_only("cafea31147e840161a1860c50af999917ae1536b").attach(&repo))
143143
);
144144
let err = parse_spec("cafea^{object}", &repo).unwrap_err();
145145
let actual = {
@@ -173,7 +173,7 @@ fn access_blob_through_tree() {
173173
let actual = parse_spec("0000000000cdc:a0blgqsjc", &repo).unwrap();
174174
assert_eq!(
175175
actual,
176-
Spec::from_id(hex_to_id("0000000000b36b6aa7ea4b75318ed078f55505c3").attach(&repo))
176+
Spec::from_id(hex_to_id_sha1_only("0000000000b36b6aa7ea4b75318ed078f55505c3").attach(&repo))
177177
);
178178
assert_eq!(
179179
actual.path_and_mode().expect("set"),

‎gix/tests/gix/revision/spec/from_bytes/peel.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,19 @@ use gix::{prelude::ObjectIdExt, revision::Spec};
22

33
use crate::{
44
revision::spec::from_bytes::{parse_spec, repo},
5-
util::hex_to_id,
5+
util::hex_to_id_sha1_only,
66
};
77

88
#[test]
99
fn peel_to_object() {
1010
let repo = &repo("complex_graph").unwrap();
1111

12-
let expected = Spec::from_id(hex_to_id("55e825ebe8fd2ff78cad3826afb696b96b576a7e").attach(repo));
12+
let expected = Spec::from_id(hex_to_id_sha1_only("55e825ebe8fd2ff78cad3826afb696b96b576a7e").attach(repo));
1313
assert_eq!(parse_spec("@^{}", repo).unwrap(), expected);
1414
assert_eq!(parse_spec("main^{}", repo).unwrap(), expected);
1515
assert_eq!(
1616
parse_spec("b-tag^{}", repo).unwrap(),
17-
Spec::from_id(hex_to_id("5b3f9e24965d0b28780b7ce5daf2b5b7f7e0459f").attach(repo))
17+
Spec::from_id(hex_to_id_sha1_only("5b3f9e24965d0b28780b7ce5daf2b5b7f7e0459f").attach(repo))
1818
);
1919
}
2020

‎gix/tests/gix/revision/spec/from_bytes/reflog.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use gix::{prelude::ObjectIdExt, revision::Spec};
22

33
use crate::{
44
revision::spec::from_bytes::{parse_spec, parse_spec_no_baseline, repo},
5-
util::hex_to_id,
5+
util::hex_to_id_sha1_only,
66
};
77

88
#[test]
@@ -44,7 +44,7 @@ fn by_index() {
4444
let spec = parse_spec("@{0}", repo).unwrap();
4545
assert_eq!(
4646
spec,
47-
Spec::from_id(hex_to_id("55e825ebe8fd2ff78cad3826afb696b96b576a7e").attach(repo))
47+
Spec::from_id(hex_to_id_sha1_only("55e825ebe8fd2ff78cad3826afb696b96b576a7e").attach(repo))
4848
);
4949
assert_eq!(
5050
spec.first_reference().expect("set").name.as_bstr(),
@@ -58,7 +58,7 @@ fn by_index() {
5858
let spec = parse_spec("HEAD@{5}", repo).unwrap();
5959
assert_eq!(
6060
spec,
61-
Spec::from_id(hex_to_id("5b3f9e24965d0b28780b7ce5daf2b5b7f7e0459f").attach(repo))
61+
Spec::from_id(hex_to_id_sha1_only("5b3f9e24965d0b28780b7ce5daf2b5b7f7e0459f").attach(repo))
6262
);
6363
assert_eq!(
6464
spec.first_reference().map(|r| r.name.to_string()),
@@ -85,6 +85,6 @@ fn by_date() {
8585

8686
assert_eq!(
8787
spec,
88-
Spec::from_id(hex_to_id("9f9eac6bd1cd4b4cc6a494f044b28c985a22972b").attach(&repo))
88+
Spec::from_id(hex_to_id_sha1_only("9f9eac6bd1cd4b4cc6a494f044b28c985a22972b").attach(&repo))
8989
);
9090
}

0 commit comments

Comments
 (0)