"git notes show" barfs when there is no note to show. This introduces a
test (yes, a test!) and, in a second round, reacts more gracefully to
empty notes and adjust the expected test output accordingly.

Note that in both cases (before/after the patch) the return code is
non-zero: It's 128 in the ungraceful case, 1 when "dieing gracefully",
uhm...

Michael J Gruber (2):
  git notes show: test empty notes
  handle empty notes gracefully

 git-notes.sh     |    2 ++
 t/t3301-notes.sh |    5 +++++
 2 files changed, 7 insertions(+), 0 deletions(-)

--

[ message continues ]

Add a test for the handling of empty notes by "git notes show".
---
 t/t3301-notes.sh |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 9393a25..4900dca 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -35,6 +35,11 @@ test_expect_success 'need valid notes ref' '
 	! MSG=2 GIT_NOTES_REF='/' git notes show
 '
 
+# 1 indicates caught gracefully by die, 128 means git-show barfed
+test_expect_failure 'handle empty notes gracefully' '
+	git notes show || test 1 = $?
+'
+
 test_expect_success 'create notes' '
 	git config core.notesRef refs/notes/commits &&
 	MSG=b1 git notes edit &&
-- 
1.6.1.2.253.ga34a

--

[ message continues ]

Currently, git-notes barfs when asked to show an empty (i.e.
non-existing) note. Change this to explicitely say there is none.
---
 git-notes.sh     |    2 ++
 t/t3301-notes.sh |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/git-notes.sh b/git-notes.sh
index bfdbaa8..9cbad02 100755
--- a/git-notes.sh
+++ b/git-notes.sh
@@ -58,6 +58,8 @@ edit)
 		"$GIT_NOTES_REF" $NEW_HEAD $CURRENT_HEAD
 ;;
 show)
+	git rev-parse -q --verify "$GIT_NOTES_REF":$COMMIT > /dev/null ||
+		die "No note for commit $COMMIT."
 	git show "$GIT_NOTES_REF":$COMMIT
 ;;
 *)
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 4900dca..81d5028 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -36,7 +36,7 @@ test_expect_success 'need valid notes ref' '
 '
 
 # 1 indicates caught gracefully by die, 128 means git-show barfed
-test_expect_failure 'handle empty notes gracefully' '
+test_expect_success 'handle empty notes gracefully' '
 	git notes show || test 1 = $?
 '
 
-- 
1.6.1.2.253.ga34a

--

[ message continues ]

Hi,



Completely forgot to mention that I think you want to use test_must_fail 
here.  And maybe you want to be more explicit, by specifying which 
commit's notes are expected not to be there.

We would not want the test to succeed for all the wrong reasons, would we?

Ciao,
Dscho


--

[ message continues ]

Wouldn't work. We want to differentiate between different exit codes. But
after test_must_fail the exit code is gone. In this simple case this
should work:

	git notes show; test 1 = $?

-- Hannes

--

[ message continues ]

Well, I could test for the friendly "No note for commit" message on
output, I just thought that is fragile.

You see, I did not even want to write a test for such a simple patch... ;)

OK, do we agree on the following intended behaviour for git notes show:
- return 0 if a note can be shown
- return 1 if there is none (i.e. die gracefully)
- return something else (i.e. die fatally) if something really bad happens

Then I should rewrite the test to check for "1" and only "1".

Now, the coffee...

Michael
--

[ message continues ]

Hi,


That test would succeed if the exit status is 0.

Ciao,
Dscho

--

[ message continues ]

Yes. If "git notes show" returns 0 then even better. It does neither
before nor after the patch. Or should we always expect 1 and test for
that? I thought about grepping the output for "fatal" (which appears
now) but that seemed ugly.

Michael
--

[ message continues ]

Uhm, I'm about 1 hour late with my afternoon coffee, and I'm afraid that
shows:

1) I meant "bark", not "barf", although both make sense here.

2) When did "format.signoff = true" in config stop working? OK, it never
did. So please consider this (anyway trivial series):

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
--

[ message continues ]