Hey Eric, On Wed, Jun 8, 2016 at 4:51 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > 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. My mistake as I forgot to include a comment in this patch. I did it in the previous patch[1]. I manually changed the file names and saw that there were a couple of tests failing in each case. >> 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 && I just tried to imitate what the test t6030 previously had (a lot of occurrences). Should I still change it to your specified format? Should I also change the others as a side cleanup "while I am at it"? >> + ! 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? True. In fact it would be much more appropriate to use test_path_is_missing() as we are using remove_path() and thus there shouldn't even exist a file with file size != 0. Thanks! >> +' >> + >> test_done [1]: http://thread.gmane.org/gmane.comp.version-control.git/294520 Regards, Pranit Bauva -- 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