Eric Sunshine [off-list ref] writes:
quoted
test_expect_success 'the values are returned in the same order they were requested' '
printf "references.format=files\nlayout.bare=false\n" >expected1 &&
printf "layout.bare=false\nreferences.format=files\n" >expected2 &&
git init --ref-format=files same-order &&
git -C same-order repo info references.format layout.bare >actual1 &&
git -C same-order repo info layout.bare references.format >actual2 &&
test_cmp expected1 actual1 &&
test_cmp expected2 actual2
'
Rather than the above, I think a more satisfactory and meaningful test would be:
test_expect_success 'values returned in order requested' '
cat >expect <<-\EOF &&
layout.bare=false
references.format=files
layout.bare=false
EOF
git init --ref-format=files ordered &&
git -C ordered repo info layout.bare references.format
layout.bare >actual &&
test_cmp expect actual
'
I do not think the second "layout.bare" should be line-wrapped.
Your point that it is more obvious when the expectations are shown
in HERE-doc may be valid. Overly long printf with \n indeed is
harder to follow. Even though there is no reason for a real user to
do so, asking for the same piece of information twice would
demonstrate that there is no deduplication.
I also care about future-proofing, though. When Git is built with
WITH_BREAKING_CHANGES=YesPlease, this test would break as the
default reference backend will be reftable in that alternate world,
wouldn't it?
On Thu, Aug 14, 2025 at 2:51 PM Junio C Hamano [off-list ref] wrote:
Eric Sunshine [off-list ref] writes:
quoted
Rather than the above, I think a more satisfactory and meaningful test would be:
test_expect_success 'values returned in order requested' '
cat >expect <<-\EOF &&
layout.bare=false
references.format=files
layout.bare=false
EOF
git init --ref-format=files ordered &&
git -C ordered repo info layout.bare references.format
layout.bare >actual &&
test_cmp expect actual
'
I do not think the second "layout.bare" should be line-wrapped.
I typed that command all one one line; Gmail wrapped the line.
Your point that it is more obvious when the expectations are shown
in HERE-doc may be valid. Overly long printf with \n indeed is
harder to follow. Even though there is no reason for a real user to
do so, asking for the same piece of information twice would
demonstrate that there is no deduplication.
Yes, part of the point of the illustrated test was indeed to
demonstrate lack of deduplication.
By the way, as a real-world developer/user, I do periodically find
myself in situations in which it *is* convenient to ask for the same
piece of information twice (or thrice) because it simplifies
downstream scripting in ad hoc (and not so ad hoc) situations when I
need to manipulate the same value in different ways. In such cases,
asking for the information more than once saves me the trouble of
having to assign the value to a variable, which is handy when the
downstream language or tool doesn't provide variables.
I also care about future-proofing, though. When Git is built with
WITH_BREAKING_CHANGES=YesPlease, this test would break as the
default reference backend will be reftable in that alternate world,
wouldn't it?
I think Lucas already future-proofed this (and my example copied his
future-proofing) by using `--ref-format=files` with the git-init
invocation.
I also care about future-proofing, though. When Git is built with
WITH_BREAKING_CHANGES=YesPlease, this test would break as the
default reference backend will be reftable in that alternate world,
wouldn't it?
To be honest, it wouldn't matter what are the keys selected for
testing this behavior. I'm only using references.format because it
was the first that I implemented. But given that, I can also change
their order to something like:
[1/5] repo: declare the repo command
[2/5] repo: add the field layout.bare
[3/5] repo: add the field layout.shallow
[2/5] repo: add the field references.format
[5/5] repo: add the --format flag
This way, this tests could be placed in 3/5 and using layout.bare and
layout.shallow as keys.
Thanks!
On Thu, Aug 14, 2025 at 6:18 PM Lucas Seiki Oshiro
[off-list ref] wrote:
quoted
I also care about future-proofing, though. When Git is built with
WITH_BREAKING_CHANGES=YesPlease, this test would break as the
default reference backend will be reftable in that alternate world,
wouldn't it?
To be honest, it wouldn't matter what are the keys selected for
testing this behavior. I'm only using references.format because it
was the first that I implemented. But given that, I can also change
their order to something like:
[1/5] repo: declare the repo command
[2/5] repo: add the field layout.bare
[3/5] repo: add the field layout.shallow
[2/5] repo: add the field references.format
[5/5] repo: add the --format flag
This way, this tests could be placed in 3/5 and using layout.bare and
layout.shallow as keys.
I don't have a strong preference since I think you already
future-proofed the test by using `--ref-format=files` with the
git-init
invocation, but the above suggested patch order would work, as well,
and seems sufficiently reviewer-friendly.