Re: [PATCH] t3501: use test_path_is_* functions

On Wed, Mar 30 2022, Labnan via GitGitGadget wrote:

Thanks for contributing to git, this is a much needed area of improvement.

While perfect shouldn't be the enemy of the good, I think it would also be a very nice use of review bandwidth to end up with some good end-state here if possible.

I.e. a pre-existing issue here is:

 * We are hiding the exit code of git due to using "test", see
   c419562860e (checkout tests: don't ignore "git <cmd>" exit code,
   2022-03-07) for one example of how to fixthat.

 * The "test -f" here is really redundant to begin with, because we'd
   get an error from "grep" if opos didn't exist.

 * Ditto ignoring the exit code of "git reflog -1".

Same issues here, except that the "test -f" aka "test_path_is_file" isn't redundant, as the grep is inverted.

It seems to me (other issues aside) that what this test actually wants
to express is something like this:
	
	diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
	index 8617efaaf1e..00096b75376 100755
	--- a/t/t3501-revert-cherry-pick.sh
	+++ b/t/t3501-revert-cherry-pick.sh
	@@ -78,8 +78,9 @@ test_expect_success 'revert after renaming branch' '
	 	git checkout rename1 &&
	 	git revert added &&
	 	test $(git rev-parse HEAD^) = $(git rev-parse rename1) &&
	-	test -f spoo &&
	-	! grep "Add extra line at the end" spoo &&
	+	git rev-parse initial:oops >expect &&
	+	git rev-parse HEAD:spoo >actual &&
	+	test_cmp expect actual &&
	 	git reflog -1 | grep revert
	 
	 '

I.e. we did a revert of a file we had so that it's the same as in "initial", but now it's at a different path, which we can exhaustively do by checking the blob OIDs.