On 06/04/2023 10:38, Ævar Arnfjörð Bjarmason wrote:
On Tue, Apr 04 2023, Andrei Rybak wrote:
Three tests in t1300-config.sh check that "git config --get" barfs when
syntax errors are present in the config file. The tests redirect
standard output and standard error of "git config --get" to files,
"actual" and "error" correspondingly. They assert presence of an error
message in file "error". However, these tests don't use file "actual"
for assertions.
Don't redirect standard output of "git config --get" to file "actual" in
t1300-config.sh to avoid creating unnecessary files.
Signed-off-by: Andrei Rybak <rybak.a.v@xxxxxxxxx>
---
t/t1300-config.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index d566729d74..8ac4531c1b 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1575,7 +1575,7 @@ 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 2>error &&
test_i18ngrep " line 3 " error
'
@@ -1585,7 +1585,7 @@ test_expect_success 'barf on incomplete section header' '
[section
key = value
EOF
- test_must_fail git config --get section.key >actual 2>error &&
+ test_must_fail git config --get section.key 2>error &&
test_i18ngrep " line 2 " error
'
@@ -1595,7 +1595,7 @@ test_expect_success 'barf on incomplete string' '
[section]
key = "value string
EOF
- test_must_fail git config --get section.key >actual 2>error &&
+ test_must_fail git config --get section.key 2>error &&
test_i18ngrep " line 3 " error
'
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.
