Re: [PATCH v2 3/7] Add a test for `git replace --convert-graft-file`

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> The proof, as the saying goes, lies in the pudding. So here is a
> regression test that not only demonstrates what the option is supposed to
> accomplish, but also demonstrates that it does accomplish it.

The above spreads the misconception that the value of the test is
"what I wrote works, look here".  It is not.  "Here is how this
thing is supposed to work.  You are free to improve it, but do not
break the basic promises these tests outline" to protect the
resulting system is.

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/t6050-replace.sh | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> index c630aba657e..77ded6df653 100755
> --- a/t/t6050-replace.sh
> +++ b/t/t6050-replace.sh
> @@ -444,4 +444,24 @@ test_expect_success GPG '--graft on a commit with a mergetag' '
>  	git replace -d $HASH10
>  '
>  
> +test_expect_success '--convert-graft-file' '
> +	: add and convert graft file &&
> +	printf "%s\n%s %s\n%s\n" \
> +		$(git rev-parse HEAD^^ HEAD^ HEAD^^ HEAD^2) \
> +		>.git/info/grafts &&

I find the above much less readbale than something like

	{
		git rev-parse HEAD^^
		git rev-parse HEAD^ HEAD^^
		git rev-parse HEAD^2
	} >.git/info/grafts

because printf formatting string must be deciphered and then matched
against the order and number of rev-parse arguments (and printf's
ability to happily accept more args than the placeholders does not
help in readablity---the reader needs to verify that the code is not
doing anything overly clever exploiting that ability) before I can
figure out who's parent of whom.

Of course, it saves a few spawning of subprocesses; I am not sure if
that is worth the loss of readability in this case, though.

> +	git replace --convert-graft-file &&
> +	test_path_is_missing .git/info/grafts &&

Successful conversion exits with success, and removes the grafts
file.  Good.

> +	: verify that the history is now "grafted" &&
> +	git rev-list HEAD >out &&
> +	test_line_count = 4 out &&

We are pretending that HEAD^ has only one parent HEAD^^ and that
HEAD^^ and HEAD^2 are roots with the above graft.  We want the
resulting "apparent" history to also show that fact, and the
traversal should stop after counting HEAD, HEAD^, HEAD^2 (which is
root), and HEAD^^ (which is also root).  Good.

> +	: create invalid graft file and verify that it is not deleted &&
> +	test_when_finished "rm -f .git/info/grafts" &&
> +	echo $EMPTY_BLOB $EMPTY_TREE >.git/info/grafts &&
> +	test_must_fail git replace --convert-graft-file 2>err &&
> +	grep "$EMPTY_BLOB $EMPTY_TREE" err &&
> +	grep "$EMPTY_BLOB $EMPTY_TREE" .git/info/grafts
> +'
> +
>  test_done