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. > +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? -Peff -- 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