From: Phillip Wood <phillip.wood123@gmail.com>
Date: Tue, 27 Dec 2022 17:13:49 GMT
Subject: Re: [PATCH v4 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )"
Message-ID: <2f41ce2e-cc6e-815e-9744-ea4e5c659b35@dunelm.org.uk>
In-Reply-To: <0825da46-b659-d18c-6e65-ced6ce85bd29@dunelm.org.uk>

On 27/12/2022 16:44, Phillip Wood wrote:
> On 19/12/2022 10:19, Ævar Arnfjörð Bjarmason wrote:
>> - For "t3700-add.sh" use "sed -n" to print the expected "bad" part,
>>    and use "test_must_be_empty" to assert that it's not there. If we used
>>    "grep" we'd get a non-zero exit code.
>>
>>    We could use "test_expect_code 1 grep", but this is more consistent
>>    with existing patterns in the test suite.
> 
> It seems strange to use sed here, you could just keep using grep and 
> check the output is empty if you don't want to use test_expect_code.

Sorry ignore that, using 'sed -n' means we don't have to worry about the 
exit code.

> There is also no need to redirect the input of the sed commands.
> 
> Best Wishes
> 
> Phillip
> 
>>    We can also remove a repeated invocation of "git ls-files" for the
>>    last test that's being modified in that file, and search the
>>    existing "files" output instead.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>   t/t0055-beyond-symlinks.sh | 14 ++++++++++++--
>>   t/t3700-add.sh             | 18 +++++++++++++-----
>>   2 files changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/t/t0055-beyond-symlinks.sh b/t/t0055-beyond-symlinks.sh
>> index 6bada370225..c3eb1158ef9 100755
>> --- a/t/t0055-beyond-symlinks.sh
>> +++ b/t/t0055-beyond-symlinks.sh
>> @@ -15,12 +15,22 @@ test_expect_success SYMLINKS setup '
>>   test_expect_success SYMLINKS 'update-index --add beyond symlinks' '
>>       test_must_fail git update-index --add c/d &&
>> -    ! ( git ls-files | grep c/d )
>> +    cat >expect <<-\EOF &&
>> +    a
>> +    b/d
>> +    EOF
>> +    git ls-files >actual &&
>> +    test_cmp expect actual
>>   '
>>   test_expect_success SYMLINKS 'add beyond symlinks' '
>>       test_must_fail git add c/d &&
>> -    ! ( git ls-files | grep c/d )
>> +    cat >expect <<-\EOF &&
>> +    a
>> +    b/d
>> +    EOF
>> +    git ls-files >actual &&
>> +    test_cmp expect actual
>>   '
>>   test_done
>> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
>> index 51afbd7b24a..82dd768944f 100755
>> --- a/t/t3700-add.sh
>> +++ b/t/t3700-add.sh
>> @@ -106,24 +106,32 @@ test_expect_success '.gitignore test setup' '
>>   test_expect_success '.gitignore is honored' '
>>       git add . &&
>> -    ! (git ls-files | grep "\\.ig")
>> +    git ls-files >files &&
>> +    sed -n "/\\.ig/p" <files >actual &&
>> +    test_must_be_empty actual
>>   '
>>   test_expect_success 'error out when attempting to add ignored ones 
>> without -f' '
>>       test_must_fail git add a.?? &&
>> -    ! (git ls-files | grep "\\.ig")
>> +    git ls-files >files &&
>> +    sed -n "/\\.ig/p" <files >actual &&
>> +    test_must_be_empty actual
>>   '
>>   test_expect_success 'error out when attempting to add ignored ones 
>> without -f' '
>>       test_must_fail git add d.?? &&
>> -    ! (git ls-files | grep "\\.ig")
>> +    git ls-files >files &&
>> +    sed -n "/\\.ig/p" <files >actual &&
>> +    test_must_be_empty actual
>>   '
>>   test_expect_success 'error out when attempting to add ignored ones 
>> but add others' '
>>       touch a.if &&
>>       test_must_fail git add a.?? &&
>> -    ! (git ls-files | grep "\\.ig") &&
>> -    (git ls-files | grep a.if)
>> +    git ls-files >files &&
>> +    sed -n "/\\.ig/p" <files >actual &&
>> +    test_must_be_empty actual &&
>> +    grep a.if files
>>   '
>>   test_expect_success 'add ignored ones with -f' '