Subject: Re: [PATCH v3 5/9] t3404: relax rebase.missingCommitsCheck tests

From: Junio C Hamano <[email-masked]>
Date: Wed, 26 Apr 2017 22:05:56 -0700

Johannes Schindelin <[email-masked]> writes:

> These tests were a bit anal about the *exact* warning/error message
> printed by git rebase. But those messages are intended for the *end
> user*, therefore it does not make sense to test so rigidly for the
> *exact* wording.
>
> In the following, we will reimplement the missing commits check in
> the sequencer, with slightly different words.

Up to this point I thought your strategy was to mimic the original
as closely as possible, and changes to update (and/or improve) the
end user experience (like rewording messages) are left as a separate
step.  Changes to the test can be used to demonstrate the improved
end-user experience that way, and I found it a good way to structure
your series.  Is there a technical reason why you deviate from that
pattern here?  

Just being curious, as I do not particularly mind seeing things done
differently (especially if there is a good reason).

> -cat >expect <<EOF
> -Warning: the command isn't recognized in the following line:
> - - badcmd $(git rev-list --oneline -1 master~1)
> -
> -You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'.
> -Or you can abort the rebase with 'git rebase --abort'.
> -EOF
> -
>  test_expect_success 'static check of bad command' '
>  	rebase_setup_and_clean bad-cmd &&
>  	set_fake_editor &&
>  	test_must_fail env FAKE_LINES="1 2 3 bad 4 5" \
>  		git rebase -i --root 2>actual &&
> -	test_i18ncmp expect actual &&
> +	test_i18ngrep "badcmd $(git rev-list --oneline -1 master~1)" actual &&
> +	test_i18ngrep "You can fix this with .git rebase --edit-todo.." actual &&
>  	FAKE_LINES="1 2 3 drop 4 5" git rebase --edit-todo &&
>  	git rebase --continue &&
>  	test E = $(git cat-file commit HEAD | sed -ne \$p) &&
> @@ -1277,20 +1270,13 @@ test_expect_success 'tabs and spaces are accepted in the todolist' '
>  	test E = $(git cat-file commit HEAD | sed -ne \$p)
>  '
>  
> -cat >expect <<EOF
> -Warning: the SHA-1 is missing or isn't a commit in the following line:
> - - edit XXXXXXX False commit
> -
> -You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'.
> -Or you can abort the rebase with 'git rebase --abort'.
> -EOF
> -
>  test_expect_success 'static check of bad SHA-1' '
>  	rebase_setup_and_clean bad-sha &&
>  	set_fake_editor &&
>  	test_must_fail env FAKE_LINES="1 2 edit fakesha 3 4 5 #" \
>  		git rebase -i --root 2>actual &&
> -	test_i18ncmp expect actual &&
> +	test_i18ngrep "edit XXXXXXX False commit" actual &&
> +	test_i18ngrep "You can fix this with .git rebase --edit-todo.." actual &&
>  	FAKE_LINES="1 2 4 5 6" git rebase --edit-todo &&
>  	git rebase --continue &&
>  	test E = $(git cat-file commit HEAD | sed -ne \$p)