On Tue, Jun 7, 2016 at 4:54 PM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote: > This is not an improvement in the test coverage but it helps in making > it explicit as to what exactly would be the error as other tests are > focussed on testing other things. It's not clear why you consider this as *not* improving test coverage. > Signed-off-by: Pranit Bauva <pranit.bauva@xxxxxxxxx> > --- > diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh > @@ -894,4 +894,21 @@ test_expect_success 'bisect start takes options and revs in any order' ' > +test_expect_success 'git bisect reset cleans bisection state properly' ' > + git bisect reset && > + git bisect start && > + git bisect good $HASH1 && > + git bisect bad $HASH4 && > + git bisect reset && > + test -z "$(git for-each-ref "refs/bisect/*")" && I wonder if this would be more easily read as: git for-each-ref "refs/bisect/*" >actual && test_must_be_empty actual && > + ! test -s "$GIT_DIR/BISECT_EXPECTED_REV" && > + ! test -s "$GIT_DIR/BISECT_ANCESTORS_OK" && > + ! test -s "$GIT_DIR/BISECT_LOG" && > + ! test -s "$GIT_DIR/BISECT_RUN" && > + ! test -s "$GIT_DIR/BISECT_TERMS" && > + ! test -s "$GIT_DIR/head-name" && > + ! test -s "$GIT_DIR/BISECT_HEAD" && > + ! test -s "$GIT_DIR/BISECT_START" Is it the intention that these should verify that the files don't exist? Maybe use test_path_is_missing() instead? > +' > + > test_done -- 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