On 06/04/2023 23:30, Andrei Rybak wrote:
On 06/04/2023 10:38, Ævar Arnfjörð Bjarmason wrote:
Ditto my comment on 1/6, shouldn't we instead be doing e.g.:
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 2575279ab84..df2070c2f09 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1575,7 +1575,8 @@ test_expect_success 'barf on syntax error' '
[section]
key garbage
EOF
- test_must_fail git config --get section.key >actual 2>error &&
+ test_must_fail git config --get section.key >out 2>error &&
+ test_must_be_empty out &&
test_i18ngrep " line 3 " error
'
I.e. before this we had no coverage on the error being the only output,
but seemingly by mistake. Let's just assert that, rather than dropping
the redirection entirely, no?
Here, failing invocations of "git config" are tested, and an argument,
as Junio C Hamano outlined in
https://lore.kernel.org/git/xmqqsfe8s56p.fsf@gitster.g/
for output of failing "git mktree", could be applied here.
Thinking about it more, such assertions enforcing empty standard output for
these commands might be helpful if some tools and/or scripts rely on empty
standard output instead of checking the exit code. Hyrum's Law applies
here,
I guess.
There are some tests in t/t1300-config.sh that do check that standard output
or standard error is empty. And I think I stumbled some other broken tests,
while checking those.
Test 'git config ignores pairs without count' checks that standard error
(2>error) is empty. Just below it, there seems to be a copy-paste error:
there are two tests titled 'git config ignores pairs with zero count'.
First one doesn't check any output, but the second checks standard output,
while calling the file "error" (>error). Test 'git config ignores pairs
with empty count' checks >error as well.
They were all introduced in d8d77153ea (config: allow specifying config entries
via envvar pairs, 2021-01-12) by Patrick Steinhardt. Patrick, what do you think?
