On Thu, Sep 16 2021, Taylor Blau wrote: > On Tue, Sep 07, 2021 at 12:57:57PM +0200, Ævar Arnfjörð Bjarmason wrote: >> Fix a blindspot in the fsck tests by checking what we do when we >> encounter an unknown "garbage" type produced with hash-object's >> --literally option. >> >> This behavior needs to be improved, which'll be done in subsequent >> patches, but for now let's test for the current behavior. >> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> >> --- >> t/t1450-fsck.sh | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh >> index 7becab5ba1e..f10d6f7b7e8 100755 >> --- a/t/t1450-fsck.sh >> +++ b/t/t1450-fsck.sh >> @@ -863,4 +863,16 @@ test_expect_success 'detect corrupt index file in fsck' ' >> test_i18ngrep "bad index file" errors >> ' >> >> +test_expect_success 'fsck hard errors on an invalid object type' ' >> + git init --bare garbage-type && > > I wondered whether it was really possible to not cover this, since I > figured such a test may have just been hiding elsewhere. But we really > do seem to be lacking coverage. So, adding this test is good. > >> + empty_blob=$(git -C garbage-type hash-object --stdin -w -t blob </dev/null) && >> + garbage_blob=$(git -C garbage-type hash-object --stdin -w -t garbage --literally </dev/null) && > > I'm nitpicking, but I find the -C garbage-type pattern less than ideal > for two reasons: > > - It makes every line longer (since "-C garbage type" is wider than an > 8-wide tab, even indenting this in a subshell would take up fewer > characters visually) > > - It pollutes the current directory with things like "err.expect" and > "err.actual" that have nothing to do with the current directory (and > much more to do with the garbage-type repository within it). > > So I don't care, really, but it may be better to just put all of this in > a subshell. Yes, it does look much nicer like that. Thanks! Some aspects of style I use I have some informed/strong opinion about, like the teardown/setup pattern noted in [1], but for some other stuff like this ... I think I was just following the pattern of some recent test I'd read or something. Well, one advantage of using "git -C" is that if it fails you can cd to the trash directory and run the command you saw fail as-is without cd-ing further, and in that case the "polluting" is a feature, you can cat the top-level expect/actual consistently. But I think on balance having the test itself be easier to read is more important, so I'm going with the subshell. 1. https://lore.kernel.org/git/87y27veeyj.fsf@xxxxxxxxxxxxxxxxxxx/
