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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 23, 2015, 19:27 UTC
Message-ID: <xmqqwpyucjj1.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1435009369-11496-1-git-send-email-remi.galan-alfonso@ensimag.grenoble-inp.fr>

Galan Rémi  <remi.galan-alfonso@ensimag.grenoble-inp.fr> 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...

Show 10 quoted lines

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