""徐沛文 (Aleen)" via GitGitGadget" <gitgitgadget@xxxxxxxxx>
writes:
> +test_expect_success 'skip an empty patch in the middle of an am session' '
> + git checkout empty-commit^ &&
> + test_must_fail git am empty-commit.patch >err &&
> + grep "Patch is empty." err &&
> + grep "If you want to record it as an empty commit, run \"git am --allow-empty\"." err &&
> + git am --skip &&
> + test_path_is_missing .git/rebase-apply &&
> + git rev-parse empty-commit^ >expected &&
> + git rev-parse HEAD >actual &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'record an empty patch as an empty commit in the middle of an am session' '
> + git checkout empty-commit^ &&
> + test_must_fail git am empty-commit.patch >err &&
> + grep "Patch is empty." err &&
> + grep "If you want to record it as an empty commit, run \"git am --allow-empty\"." err &&
> + git am --allow-empty &&
> + test_path_is_missing .git/rebase-apply &&
> + git show empty-commit --format="%s" >expected &&
> + git show HEAD --format="%s" >actual &&
> + test_cmp actual expected
> +'
These two are "positive" tests, i.e. the feature does the right
thing when used in the expected situation.
I am worried about the cases where "--allow-empty" is used and
creates a commit that is empty, when it is reasonably expected to
either create a non-empty commit or still fail the same way to give
the user another chance to try.
For example, when "git am -3" fails on such an email message (there
are two failure modes: (a) one results in an unmerged index due to
merge conflicts, (b) the other results in a clean index due to not
finding the blob object recorded as a preimage), what happens when
the user runs "git am --allow-empty", after:
(1) doing nothing since the failed "git am -3" application,
(2) resolving the conflicts in working tree, or
(3) resolving the conflicts in working tree and recording the
resolution in the index.
There are 6 cases in the above matrix, and "git am --allow-empty"
should not create an empty commit in any one of them. In 1a and 1b,
"git am --allow-empty" should continue to fail ("git am --skip"
would be a way to ignore and proceed). 3a and 3b should record a
non-empty commit as the result.
Similarly, when an email message with a patch is given to "git am"
and the application fails, and the user runs "git am --allow-empty"
without doing anything in the working tree, what happens? It should
not happily create an empty commit with only the log message,
without the change, but continue to fail. If the user resolves and
registers the resolution to the index before "git am --allow-empty",
then the command should create a non-empty commit.
Having positive tests are of course important to avoid regression in
the future, but negative tests, the new and shiny feature does not
misbehave when it shouldn't even kick in, is even more important.
Thanks.
