From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 07 Apr 2010 05:16:49 GMT
Subject: Re: [PATCH v2] Add option to git-commit to allow empty log messages
Message-ID: <7vsk77g7um.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1270543235-8570-1-git-send-email-avarab@gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

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

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

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

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

 t/t7500-commit.sh |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 9f5c3ed..aa9c577 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -193,4 +193,26 @@ test_expect_success 'commit -F overrides -t' '
 	commit_msg_is "-F log"
 '
 
+test_expect_success 'Commit without message is allowed with --allow-empty-message' '
+	echo "more content" >>foo &&
+	git add foo &&
+	>empty &&
+	git commit --allow-empty-message <empty &&
+	commit_msg_is ""
+'
+
+test_expect_success 'Commit without message is no-no without --allow-empty-message' '
+	echo "more content" >>foo &&
+	git add foo &&
+	>empty &&
+	test_must_fail git commit <empty
+'
+
+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"
+'
+
 test_done