Re: [PATCH v2] Add option to git-commit to allow empty log messages

On Wed, Apr 7, 2010 at 05:16, Junio C Hamano [off-list ref] wrote:

Ævar Arnfjörð Bjarmason [off-list ref] writes:

quoted

diff --git a/t/t7510-commit-allow-empty-message.sh b/t/t7510-commit-allow-empty-message.sh
new file mode 100755
index 0000000..d7dc0da

I do not think a separate script is worth it.  I'd just add tests to t7500.

quoted

+test_expect_success 'a basic commit in an empty tree should succeed' '
+     (
+             echo content > foo &&
+             git add foo &&
+             git commit -m "initial commit"
+     ) &&
+     commit_msg_is "initial commit"
+'

No need for this, especially if this becomes part of t7500.

quoted

+test_expect_success 'Commit no message with --allow-empty-message' '
+     (
+             echo "more content" >> foo &&
+             git add foo &&
+             printf "" | git commit --allow-empty-message
+     ) &&
+     commit_msg_is ""
+'

No need for subprocess, nor printf piped to the command.

quoted

+test_expect_success 'Commit a message with --allow-empty-message' '
+     (
+             echo "even more content" >> foo &&
+             git add foo &&
+             git commit --allow-empty-message -m"hello there"
+     ) &&
+     commit_msg_is "hello there"
+'

Ditto.

You are falling into the same trap as everybody else does when showing off
your shiny new toy.  A missing but very necessary test is that "commit"
with your patch does still fail when an empty message is given without the
new option.

It takes a conscious effort to carefully make sure that your shiny new toy
does not trigger when it shouldn't, especially when you are so excited by
seeing it work when it should.

But that is part of the art of writing good test scripts.

Thanks for shepherding the patch and committing it, and for pointing
out what I could have done better. I'll try not to make similar
mistakes with future submissions.