Jeff King <peff@xxxxxxxx> writes: > On Mon, Jun 01, 2015 at 05:53:49PM +0200, Michael Haggerty wrote: > >> Add tests that for-each-ref correctly reports broken loose reference >> files and references that point at missing objects. In fact, two of >> these tests fail, because (1) NULL_SHA1 is not recognized as an >> invalid reference value, and (2) for-each-ref doesn't respect >> REF_ISBROKEN. Fixes to come. > > This whole series looks straightforward and correct to me. Thanks for a > pleasant read. I have two minor comments on the tests: > >> --- /dev/null >> +++ b/t/t6301-for-each-ref-errors.sh >> @@ -0,0 +1,45 @@ >> +#!/bin/sh >> + >> +test_description='for-each-ref errors for broken refs' >> + >> +. ./test-lib.sh >> + >> +ZEROS=0000000000000000000000000000000000000000 >> +MISSING=abababababababababababababababababababab > > The test suite provides $_z40, so you can skip $ZEROS. I don't think > it's a big deal, though, and it may be nicer to have it explicitly next > to $MISSING here. Yeah, ZEROS=$_z40 next to MISSING=abababa... may not be too bad. >> +test_expect_success 'Missing objects are reported correctly' ' >> + r=refs/heads/missing && >> + echo $MISSING >.git/$r && >> + test_when_finished "rm -f .git/$r" && >> + echo "fatal: missing object $MISSING for $r" >missing-err && >> + test_must_fail git for-each-ref 2>err && >> + test_cmp missing-err err >> +' > > Due to b7dd2d2 (that you mentioned in the message for patch 2), we only > sometimes notice the missing objects. Is it worth testing that: > > git for-each-ref --format='%(refname)' > > does _not_ barf here? Sound like a good thing to check, too. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html