Re: [PATCH 2/4] t6030: explicitly test for bisection cleanup

Hey Eric,

On Wed, Jun 8, 2016 at 4:51 AM, Eric Sunshine [off-list ref] wrote:

On Tue, Jun 7, 2016 at 4:54 PM, Pranit Bauva [off-list ref] wrote:

quoted

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.

quoted

Signed-off-by: Pranit Bauva <redacted>
---

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"?

quoted

+       ! 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!

quoted

+'
+
 test_done

[1]: http://thread.gmane.org/gmane.comp.version-control.git/294520


Regards,
Pranit Bauva