Re: [PATCH v6 9/9] status: unit tests for --porcelain=v2

2016-08-11 Thread Jeff Hostetler



On 08/11/2016 02:36 PM, Junio C Hamano wrote:

Jeff Hostetler  writes:


From: Jeff Hostetler 
+. ./test-lib.sh
+
+OID_EMPTY=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391


It seems that test-lib.sh these days has EMPTY_BLOB defined for your
use.  You can remove this and replace its use (just two lines) with
$EMPTY_BLOB down in the "add -N" test.


That's a recent addition to test-lib.sh. I'll rebase my series
onto a more recent master to get that.



+test_expect_success setup '
+   test_tick &&
+   git config --local core.autocrlf false &&


I'd suggest removing "--local".

Existing use of "git config" in the test suite, unless their use is
about testing "git config" itself to validate the operation of the
--local/--global/--system options, do not seem to explicitly say
"--local", which is the default anyway.


ok. just being cautious, but if that's the convention, that's fine.



+test_expect_success 'after first commit, make dirt, confirm unstaged changes' '


Did you mean s/dirt/dirty/?  "make and confirm unstaged changes"
would be sufficient.  Because "confirming" is implicit (as these
are all tests), "after the first commit, modify working tree files"
might even be better, perhaps?


+   echo x >>file_x &&
+   OID_X1=$(git hash-object -t blob -- file_x) &&
+   rm file_z &&
+   H0=$(git rev-parse HEAD) &&
+ ...


got it.



+test_expect_success 'after first commit, stage dirt, confirm staged changes' '


What you "git add" is meant to be good changes, so they are no
longer dirt ;-)  More importantly, because I never heard of "dirt"
used in Git context, it is unclear if it is an untracked file, a
modification that is not meant to be committed immediately, or what.

"after the first commit, fully add changes to the index"?


reworded and simplified.



+   git add file_x &&
+   git rm file_z &&
+   H0=$(git rev-parse HEAD) &&
+
+   cat >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
+   1 D. N... 100644 00 00 $OID_Z $_z40 file_z
+   ? actual
+   ? expect
+   EOF



+test_expect_success 'after first commit, also stage rename, confirm 2 path 
line format' '
+   git mv file_y renamed_y &&
+   H0=$(git rev-parse HEAD) &&
+
+   q_to_tab >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
+   1 D. N... 100644 00 00 $OID_Z $_z40 file_z
+   2 R. N... 100644 100644 100644 $OID_Y $OID_Y R100 renamed_yQfile_y
+   ? actual
+   ? expect
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'


Do we want to test -z format on this, too?


Sure.


I'll send these up in a v7 series in a few minutes.

Thanks
Jeff

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 9/9] status: unit tests for --porcelain=v2

2016-08-11 Thread Junio C Hamano
Jeff Hostetler  writes:

> From: Jeff Hostetler 
>
> Test porcelain v2 status format.
>
> Signed-off-by: Jeff Hostetler 
> ---
>  t/t7064-wtstatus-pv2.sh | 576 
> 
>  1 file changed, 576 insertions(+)
>  create mode 100755 t/t7064-wtstatus-pv2.sh
>
> diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
> new file mode 100755
> index 000..44a8671
> --- /dev/null
> +++ b/t/t7064-wtstatus-pv2.sh
> @@ -0,0 +1,576 @@
> +#!/bin/sh
> +
> +test_description='git status --porcelain=v2
> +
> +This test exercises porcelain V2 output for git status.'

A general comment on the titles; with retitling of individual tests,
the result has become a lot easier to understand.  I know coming up
with a short and to-the-point description for them is hard, but that
is effort and time well spent and it shows in the result.  Thanks.

> +. ./test-lib.sh
> +
> +OID_EMPTY=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391

It seems that test-lib.sh these days has EMPTY_BLOB defined for your
use.  You can remove this and replace its use (just two lines) with
$EMPTY_BLOB down in the "add -N" test.

> +test_expect_success setup '
> + test_tick &&
> + git config --local core.autocrlf false &&

I'd suggest removing "--local".

Existing use of "git config" in the test suite, unless their use is
about testing "git config" itself to validate the operation of the
--local/--global/--system options, do not seem to explicitly say
"--local", which is the default anyway.

> +test_expect_success 'after first commit, make dirt, confirm unstaged 
> changes' '

Did you mean s/dirt/dirty/?  "make and confirm unstaged changes"
would be sufficient.  Because "confirming" is implicit (as these
are all tests), "after the first commit, modify working tree files"
might even be better, perhaps?

> + echo x >>file_x &&
> + OID_X1=$(git hash-object -t blob -- file_x) &&
> + rm file_z &&
> + H0=$(git rev-parse HEAD) &&
> + ...

> +test_expect_success 'after first commit, stage dirt, confirm staged changes' 
> '

What you "git add" is meant to be good changes, so they are no
longer dirt ;-)  More importantly, because I never heard of "dirt"
used in Git context, it is unclear if it is an untracked file, a
modification that is not meant to be committed immediately, or what.

"after the first commit, fully add changes to the index"?

> + git add file_x &&
> + git rm file_z &&
> + H0=$(git rev-parse HEAD) &&
> +
> + cat >expect <<-EOF &&
> + # branch.oid $H0
> + # branch.head master
> + 1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
> + 1 D. N... 100644 00 00 $OID_Z $_z40 file_z
> + ? actual
> + ? expect
> + EOF

> +test_expect_success 'after first commit, also stage rename, confirm 2 path 
> line format' '
> + git mv file_y renamed_y &&
> + H0=$(git rev-parse HEAD) &&
> +
> + q_to_tab >expect <<-EOF &&
> + # branch.oid $H0
> + # branch.head master
> + 1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
> + 1 D. N... 100644 00 00 $OID_Z $_z40 file_z
> + 2 R. N... 100644 100644 100644 $OID_Y $OID_Y R100 renamed_yQfile_y
> + ? actual
> + ? expect
> + EOF
> +
> + git status --porcelain=v2 --branch --untracked-files=all >actual &&
> + test_cmp expect actual
> +'

Do we want to test -z format on this, too?

> ...
> +test_expect_success 'create ignored files, confirm they are not printed' '
> +test_expect_success 'create ignored files, confirm --ignored prints them' '
> ...

These are all good and readably titled. 

> +test_expect_success 'verify upstream fields in branch header' '
> + git checkout master &&
> + test_when_finished rm -rf sub_repo &&

Forgot to quote?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html