Re: [PATCH v4 3/9] t3422: new testcases for checking when incompatible options passed

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 26, 2018, 18:17 UTC
Message-ID: <xmqqk1qll8f7.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180625161300.26060-4-newren@gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 23 quoted lines

> +# Rebase has lots of useful options like --whitepsace=fix, which are
> +# actually all built in terms of flags to git-am.  Since neither
> +# --merge nor --interactive (nor any options that imply those two) use
> +# git-am, using them together will result in flags like --whitespace=fix
> +# being ignored.  Make sure rebase warns the user and aborts instead.
> +#
> +
> +test_rebase_am_only () {
> +	opt=$1
> +	shift
> +	test_expect_failure "$opt incompatible with --merge" "
> +		git checkout B^0 &&
> +		test_must_fail git rebase $opt --merge A
> +	"
> +
> +	test_expect_failure "$opt incompatible with --strategy=ours" "
> +		git checkout B^0 &&
> +		test_must_fail git rebase $opt --strategy=ours A
> +	"
> +
> +	test_expect_failure "$opt incompatible with --strategy-option=ours" "
> +		git checkout B^0 &&
> +		test_must_fail git rebase $opt --strategy=ours A

This line is broken and it is carried over to later patches. It needs to be -Xours (or --strategy-option=ours, if we really want ot be verbose).

Show 13 quoted lines

> +	"
> +
> +	test_expect_failure "$opt incompatible with --interactive" "
> +		git checkout B^0 &&
> +		test_must_fail git rebase $opt --interactive A
> +	"
> +
> +	test_expect_failure "$opt incompatible with --exec" "
> +		git checkout B^0 &&
> +		test_must_fail git rebase $opt --exec 'true' A
> +	"
> +
> +}

Show 5 quoted lines

> +
> +test_rebase_am_only --whitespace=fix
> +test_rebase_am_only --ignore-whitespace
> +test_rebase_am_only --committer-date-is-author-date
> +test_rebase_am_only -C4

I was hesitant to hardcode what I perceive as limitations of non-am rebase implementations with a test like this, but once somebody fixes "rebase -i" for example to be capable of --whitespace=fix for example, then we can just drop one line from the above four (and write other tests for "rebase -i --whitespace=fix"). The test_rebase_am_only is to help us make sure what is (still) not supported by non-am rebases gets diagnosed as an error.

So my worry is totally unfounded, which is good.

Show 21 quoted lines

> +test_expect_success '--preserve-merges incompatible with --signoff' '
> +	git checkout B^0 &&
> +	test_must_fail git rebase --preserve-merges --signoff A
> +'
> +
> +test_expect_failure '--preserve-merges incompatible with --rebase-merges' '
> +	git checkout B^0 &&
> +	test_must_fail git rebase --preserve-merges --rebase-merges A
> +'
> +
> +test_expect_failure '--rebase-merges incompatible with --strategy' '
> +	git checkout B^0 &&
> +	test_must_fail git rebase --rebase-merges -s resolve A
> +'
> +
> +test_expect_failure '--rebase-merges incompatible with --strategy-option' '
> +	git checkout B^0 &&
> +	test_must_fail git rebase --rebase-merges -Xignore-space-change A
> +'
> +
> +test_done