"Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > From: Johannes Schindelin <johannes.schindelin@xxxxxx> > > 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. Wow. Thanks for digging thru this > 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. 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. > index 84bbf88cf9..704bbc6541 100755 > --- a/t/t3301-notes.sh > +++ b/t/t3301-notes.sh > @@ -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' '
