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

On Tue, Jun 26, 2018 at 11:17 AM, Junio C Hamano [off-list ref] wrote:

Elijah Newren [off-list ref] writes:

quoted

+# 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).

Indeed; I'll fix that up and resubmit.  Thanks for catching it.

quoted

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

quoted

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

Yes, and I was thinking in particular that we could start by teaching
rebase -i to understand --ignore-space-change/--ignore-whitespace by
just transliterating it into -Xignore-space-change.  That is, after
https://public-inbox.org/git/20180607050845.19779-2-newren@gmail.com/
is picked up and eventually merged down.  Speaking of which, I need to
resubmit that.

So my worry is totally unfounded, which is good.