Re: [GSOC][PATCH 1/1] t3701: Avoid suppression of exit status of git
- From
Andrei Rybak <rybak.a.v@gmail.com>
- Date
- Mar 26, 2023, 20:14 UTC
- Message-ID
- <be2f23ec-4b3c-eb9f-fc2d-fecdf76604e1@gmail.com>
- In-Reply-To
- <20230326173147.39626-2-edwinfernando734@gmail.com>
Hello, Edwin.
In subject line "t3701: Avoid suppression of exit status of git" -- the first word after a colon shouldn't be capitalized. By the way, similar existing commits call it:
don't lose "git" exit codes
and
preserve "git" exit codes
On 26/03/2023 19:31, Edwin Fernando wrote:
> Pipes and command substitution in bash suppress the exit codes of the
Bash is not the only Shell used for running Git's test suite, so referring to it like this in the commit message might not be a good idea.
See also similar commit 9fdc79ecba ("tests: don't lose misc "git" exit codes", 2023-02-06) for inspiration in writing the commit message.
Show 20 quoted lines
> first executed command. In the test scripts errors when running git > should be exposed. Remove all such suppressions of git by writing > output to files, chaining commands with &&, or other means. > > Signed-off-by: Edwin Fernando <edwinfernando734@gmail.com> > --- > t/t3701-add-interactive.sh | 25 ++++++++++++++++--------- > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh > index 3a99837d9b..80446b311d 100755 > --- a/t/t3701-add-interactive.sh > +++ b/t/t3701-add-interactive.sh > @@ -292,8 +292,10 @@ test_expect_success FILEMODE 'patch does not affect mode' ' > echo content >>file && > chmod +x file && > printf "n\\ny\\n" | git add -p && > - git show :file | grep content && > - git diff file | grep "new mode" > + git show :file > show_file &&
Code style here and in all output redirections below: lose the space between redirection and the path, like so:
git show :file >show_file
Also, the suffix "_file" is unnecessary. Just
git show :file >show
or
git show :file >out
or
git show :file >actual
might be better.
Show 32 quoted lines
> + grep content show_file && > + git diff file > diff_file && > + grep "new mode" diff_file > ' > > test_expect_success FILEMODE 'stage mode but not hunk' ' > @@ -301,8 +303,10 @@ test_expect_success FILEMODE 'stage mode but not hunk' ' > echo content >>file && > chmod +x file && > printf "y\\nn\\n" | git add -p && > - git diff --cached file | grep "new mode" && > - git diff file | grep "+content" > + git diff --cached file > diff_file && > + grep "new mode" diff_file && > + git diff file > diff_file && > + grep "+content" diff_file > ' > > > @@ -311,9 +315,12 @@ test_expect_success FILEMODE 'stage mode and hunk' ' > echo content >>file && > chmod +x file && > printf "y\\ny\\n" | git add -p && > - git diff --cached file | grep "new mode" && > - git diff --cached file | grep "+content" && > - test -z "$(git diff file)" > + git diff --cached file > diff_file && > + grep "new mode" diff_file && > + git diff --cached file diff_file && > + grep "+content" diff_file && > + git diff file > diff_file && > + test -z $(cat diff_file)
Function test_stdout_line_count can be used here instead of "test -z". test_stdout_line_count would produce a more helpful error message in case of test failure.
Show 14 quoted lines
> ' > > # end of tests disabled when filemode is not usable > @@ -970,8 +977,8 @@ test_expect_success 'handle submodules' ' > > force_color git -C for-submodules add -p dirty-head >output 2>&1 <y && > git -C for-submodules ls-files --stage dirty-head >actual && > - rev="$(git -C for-submodules/dirty-head rev-parse HEAD)" && > - grep "$rev" actual > + git -C for-submodules/dirty-head rev-parse HEAD > rev && > + grep -f rev actual > ' > > test_expect_success 'set up pathological context' '