Fix a false negative in t3301-notes.sh

It is always bad when test cases fail for the wrong reasons, but it is in some ways more scary when they pass for the wrong reasons.

I stumbled over this issue while chasing down a Windows-specific issue that caused two other test cases to fail, and should have caused this one to fail, too, but didn't.

Johannes Schindelin (1):
t3301: fix false negative

 t/t3301-notes.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-176%2Fdscho%2Ffix-false-negative-in-t3301-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-176/dscho/fix-false-negative-in-t3301-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/176

gitgitgadget

From: Johannes Schindelin <[email protected]>

In 6956f858f6 (notes: implement helpers needed for note copying during rewrite, 2010-03-12), we introduced a test case that verifies that the config setting notes.rewriteRef can be overridden via the environment variable GIT_NOTES_REWRITE_REF.

Back when it was introduced, it relied on a side effect of an earlier test case that configured core.noteRef to point to refs/notes/other.

In 908a320363 (t3301: modernize style, 2014-11-12), this side effect was removed.

The test case *still* passed, but for the wrong reason: we no longer overrode the rewrite ref, but there simply was nothing to rewrite anymore, as the overridden notes ref was "modernized" away.

Let's let that test case pass for the correct reason again.

To make sure of that, let's change the idea of the original test case:
it configured notes.rewriteRef to point to the actual notes ref, forced that to be ignored and then verified that the notes were *not* rewritten.

By turning that idea upside down (configure the notes.rewriteRef to another notes ref, override it via the environment variable to force the notes to be copied, and then verify that the notes *were* rewritten), we make it much harder for that test case to pass for the wrong reason.

---
 t/t3301-notes.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index 84bbf88cf9..704bbc6541 100755
@@ -1120,9 +1120,10 @@ test_expect_success 'GIT_NOTES_REWRITE_REF overrides config' '
 	test_config notes.rewriteMode overwrite &&
 	test_config notes.rewriteRef refs/notes/other &&
 	echo $(git rev-parse HEAD^) $(git rev-parse HEAD) |
-	GIT_NOTES_REWRITE_REF= git notes copy --for-rewrite=foo &&
+	GIT_NOTES_REWRITE_REF=refs/notes/commits \
+		git notes copy --for-rewrite=foo &&
 	git log -1 >actual &&
-	test_cmp expect actual
+	grep "replacement note 3" actual
 '
 
 test_expect_success 'git notes copy diagnoses too many or too few parameters' '

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Wow. Thanks for digging thru this

Yup. I agree that testing positive side, expressing what we want to happen in a more explicit manner, is always a better alternative.

Will queue. Thanks.