[PATCH v4 0/1][GSoC] t3501: remove test -f and stop ignoring git <cmd> exit code

Resending this email because the previous email did not seem to reach gitgitgadget's PR and the mailing list.

T3501 contains two test cases where we used to check if a file exists using test -f which is not necessary because we already have helper function for it. In fact in one test case test -f is redundant because we grep right after it. Also the use of git rev-parse can cause segfault.

This patch tries to resolve these two issues in T3501.

Khalid Masum (1):
  t3501: remove test -f and stop ignoring git <cmd> exit code

 t/t3501-revert-cherry-pick.sh | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Range-diff against v3:
1:  aeb28e3249 ! 1:  1090429b86 t3501: remove redundant file check and stop ignoring git <cmd> exit code
    @@
      ## Metadata ##
    -Author: Labnann <khalid.masum.92@gmail.com>
    +Author: Khalid Masum <khalid.masum.92@gmail.com>
     
      ## Commit message ##
    -    t3501: remove redundant file check and stop ignoring git <cmd> exit code
    +    t3501: remove test -f and stop ignoring git <cmd> exit code
     
         In the test 'cherry-pick after renaming branch', stop checking for
         the presence of a file (opos) because we are going to "grep" in it in
    @@ Commit message
     
         In the test 'revert after renaming branch', instead of allowing any
         random contents as long as a known phrase is not there in it, we can
    -    expect the exact outcome---after the successful revert of
    -    "added", the contents of file "spoo" should become identical to
    -    what was in file "oops" in the "initial" commit.
    +    expect the exact outcome---after the successful revert of "added", the
    +    contents of file "spoo" should become identical to what was in file
    +    "oops" in the "initial" commit. This test also contains 'test -f' that
    +    verifies presence of a file, but we have a helper function to do the same
    +    thing. Replace it with appropriate helper function 'test_path_is_file'
    +    for better readability and better error messages.
     
    -    In both tests, we would not notice when "git rev-parse" starts
    -    segfaulting without emitting any output.  The 'test' command would
    -    end up being just "test =", which yields success. Therefore we could
    -    use test_cmp_rev
    +    In both tests, we will not notice when "git rev-parse" starts segfaulting
    +    without emitting any output.  The 'test' command will end up being just
    +    "test =", which yields success. Use the 'test_cmp_rev' helper to make
    +    sure we will notice such a breakage.
     
    -    Signed-off-by: Labnann <khalid.masum.92@gmail.com>
    +    Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
     
      ## t/t3501-revert-cherry-pick.sh ##
     @@ t/t3501-revert-cherry-pick.sh: test_expect_success 'cherry-pick after renaming branch' '
    @@ t/t3501-revert-cherry-pick.sh: test_expect_success 'revert after renaming branch
     -	test -f spoo &&
     -	! grep "Add extra line at the end" spoo &&
     +	test_cmp_rev rename1 HEAD^ &&
    ++	test_path_is_file spoo &&
     +	test_cmp_rev initial:oops HEAD:spoo &&
      	git reflog -1 | grep revert