On 06/04/2023 10:41, Ævar Arnfjörð Bjarmason wrote:
On Tue, Apr 04 2023, Andrei Rybak wrote:
---
t/ | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index bca46378b2..8c442adb1a 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -989,10 +989,7 @@ test_expect_success 'fsck error and recovery on invalid object type' '
garbage_blob=$(git hash-object --stdin -w -t garbage --literally </dev/null) &&
- cat >err.expect <<-\EOF &&
- fatal: invalid object type
- EOF
- test_must_fail git fsck >out 2>err &&
+ test_must_fail git fsck 2>err &&
grep -e "^error" -e "^fatal" err >errors &&
test_line_count = 1 errors &&
grep "$garbage_blob: object is of unknown type '"'"'garbage'"'"':" err
...but ditto my review on other patches, this just seems like a mistake
of mine, i.e. if I add the "test_must_be_empty out" the test passes.
So isn't the answer here that my 31deb28f5e had an unintentional
regression, and we should bring the assertion back? Its commit message
says nothing about wanting to stop asserting stdout.
Maybe there was a reason I'm missing for why I remved it, it's since
been paged out of my wetware, but looking at it briefly now it just
seems like an unintentional bug / loss of test coverage that we should
fix.
Tests in t1450-fsck.sh that do enforce empty standard output do it mostly
via ">../actual 2>&1" and then a "test_must_be_empty actual".
For 'fsck error and recovery on invalid object type', the question is:
is having this assertion useful for a developer using this test? The test
is about invalid object types and what error messages "git fsck" prints
about them. The test creates a fresh repository for it:
git init --bare garbage-type &&
Is it useful to a developer working on this part of "git fsck" to have
a "reminder" that no dangling objects should be found in such a fresh
repository? Speaking of which, should there be such a test:
test_expect_success 'fresh repository has no dangling objects' '
git init fresh &&
git -C fresh fsck >out
test_must_be_empty out
'
? Maybe even in t0001-init.sh?
