On Sat, Oct 17, 2020 at 12:02 PM Amanda Shafack via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> Using the logical not operator on both the git and grep command is
> redundant, since the main check is done at the level of the grep
> command.
>
> Apply the logical not operator only to the grep command.
> ---
> diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
> @@ -179,10 +179,8 @@ test_expect_success 'add -u resolves unmerged paths' '
> - ! (
> - git ls-files >actual &&
> - grep "non-existent" actual
> - )
> + git ls-files >actual &&
> + ! grep "non-existent" actual
The commit message talks only about redundancy of applying the logical
NOT operator to the combined subshell content, but what it misses is
that transformation of:
! (git ls-files | grep "non-existent")
in patch [1/2] into:
! (
git ls-files >actual &&
grep "non-existent" actual
)
is actively wrong. In particular, if `git ls-files` itself fails, then
the `grep` will never get run, and the subshell will exit with a
failure, however, the NOT then turns that failure into a success,
which is quite undesirable. The test should fail if either `git
ls-files` fails or if the `grep` expectation is not met; it should not
succeed when `git ls-files` fails.
So, the correct thing to do is to merge [1/2] and [2/2] into a single
patch which directly transforms:
! (git ls-files | grep "non-existent")
into:
git ls-files >actual &&
! grep "non-existent" actual
