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

From: Jeff Hostetler <git@jeffhostetler.com>
Date: Aug 11, 2016, 20:51 UTC
Message-ID: <57ACE550.9040501@jeffhostetler.com>
In-Reply-To: <xmqqtwer17b6.fsf@gitster.mtv.corp.google.com>

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

Show 10 quoted lines

> Jeff Hostetler <git@jeffhostetler.com> writes:
>
>> From: Jeff Hostetler <jeffhost@microsoft.com>
>> +. ./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.

Show 10 quoted lines

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

Show 12 quoted lines

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

Show 8 quoted lines

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

Show 32 quoted lines

>> +	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 000000 000000 $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 000000 000000 $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