Subject: Re: [PATCHv6 1/3] git-rebase -i: add command "drop" to remove a commit

From: Junio C Hamano <[email-masked]>
Date: Tue, 23 Jun 2015 12:27:30 -0700

Galan Rémi  <[email-masked]> writes:

> +test_rebase_end () {
> +	test_when_finished "git checkout master &&
> +	git branch -D $1 &&

Is this one guaranteed to succeed?  Do we want to consider it a
failure to remove "$1" (e.g. dropTest)?

    $ git branch -D no-such-branch ; echo $?
    error: branch 'no-such-branch' not found.
    1

If dropTest branch did not exist before the test that begins with
a call to this function, what happens?

Besides, a function that must be called at the beginning of a test
piece has a name that ends with _end?  That sounds funny, no?

> +	test_might_fail git rebase --abort" &&
> +	git checkout -b $1 master
> +}

I'm wondering if this is not sufficient.

	test_rebase_i_drop_prepare () {
		git reset --hard &&
	        git checkout -B "$1" master
	}

I am guessing that you named _end because it has when_finished, but
as far as I can tell, even after these three patches, the tests do
not really rely on the fact that it is on 'master' branch.  More
importantly, just being on 'master' branch is not a sufficient
cleanliness for the next test (and that is why you added these
"branch -D" and "might-fail rebase --abort" to this function in the
first place), it seems.  So...

> +test_expect_success 'drop' '
> +	test_rebase_end dropTest &&
> +	set_fake_editor &&
> +	FAKE_LINES="1 drop 2 3 drop 4 5" git rebase -i --root &&
> +	test E = $(git cat-file commit HEAD | sed -ne \$p) &&
> +	test C = $(git cat-file commit HEAD^ | sed -ne \$p) &&
> +	test A = $(git cat-file commit HEAD^^ | sed -ne \$p)
> +'
> +
>  test_done