Junio C Hamano <gits...@pobox.com> writes:
> But by splitting these into separate tests, the patch makes such a
> potential failure with "git commit --short" break the later steps.
>
> Not very nice.
>
> It may be a better change to just do in the original one
>
> git add test-file &&
> git commit --dry-run &&
> + git commit --short &&
> + git commit --long &&
> + git commit --porcelain &&
> git commit -m "conflicts fixed from merge."
>
> without adding these new and separate tests, and then mark that one
> to expect a failure (because it would pass up to the --dry-run
> commit, but the --short commit would fail) at this step, perhaps?
Of course, if you want to be more thorough, anticipating that other
people in their future updates may break --short but not --long or
--porcelain, testing each option in separate test_expect_success is
a necessary way to do so, but then you'd need to actually be more
thorough, by not merely running each of them in separate
test_expect_success block but also arranging that each of them start
in an expected state to try the thing we want it to try. That is
for opt in --dry-run --short --long --porcelain
do
test_expect_success "commit $opt" '
set up the conflicted state after merge &&
git commit $opt
'
done
where the "set up the state" part makes sure it can tolerate
potential mistakes of previous run of "git commit $opt" (e.g. it
by mistake made a commit, making the index identical to HEAD and
taking us out of "merge in progress" state).
But from your 1/3 I did not get the impression that you particularly
want to be more thorough, and from your 3/3 I did not get the
impression that you anticipate --short/--long/--porcelain may get
broken independently. And if that is the case, then chaining all of
them together like the above is a more honest way to express that we
are only doing a minimum set of testing.
Thanks.
