git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4 2/8] [Newcomer] t7004: Do not lose exit status to pipe

From
AbdAlRahman Gad <abdobngad@gmail.com>
Date
Aug 6, 2024, 08:38 UTC
Message-ID
<3cba795c-631b-4e86-a1f2-ddefe76ef0f4@gmail.com>
In-Reply-To
<CAPig+cS6-gqZbJSz3wBcummyycJVudKybVZW=D33WHk-Fa9ytQ@mail.gmail.com>
On 8/6/24 06:13, Eric Sunshine wrote:
Show 31 quoted lines
> On Mon, Aug 5, 2024 at 8:00 PM AbdAlRahman Gad <abdobngad@gmail.com> wrote:
>> Split "test-tool ... | sed" pipeline into two commands to avoid losing
>> exit status from test-tool.
>>
>> Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
>> ---
>> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
>> @@ -97,7 +97,8 @@ test_expect_success 'creating a tag with --create-reflog should create reflog' '
>> -       test-tool ref-store main for-each-reflog-ent refs/tags/tag_with_reflog1 | sed -e "s/^.* //" >actual &&
>> +       test-tool ref-store main for-each-reflog-ent refs/tags/tag_with_reflog1 >actual.body &&
>> +       sed -e "s/^.*   //" actual.body >actual &&
> 
> It's not just `test_tool` we care about; we also (importantly) don't
> want to see `git` itself upstream of a pipe, and there are many such
> instances remaining in this script. Here are some common examples:
> 
>      test $(git tag -l | wc -l) -eq 0 &&
>      git cat-file tag "$1" | sed -e "/BEGIN PGP/q"
>      git tag -l | grep "^tag-one-line" >actual &&
>      forged=$(git cat-file tag ... | sed -e ... | git mktag) &&
>      git tag -l --no-sort "foo*" | sort >actual &&
> 
> By the way, these days, rather than:
> 
>      test $(git tag -l | wc -l) -eq 0 &&
> 
> we would say:
> 
>      test_stdout_line_count = 0 git tag -l &&
> 
> which nicely avoids placing `git` upstream of a pipe.

Thanks for the review. Could this be done in a separate patch series? As modifying a patch in the beginning of a patch series requires lots of time to adapt the rest of the series to the change.

Previous: Eric SunshineNext: Eric Sunshine
Message 5 of 21 in “t7004: Modernize the style”
  1. 0/8 t7004: Modernize the styleAbdAlRahman Gad, Aug 5, 2024
  2. 1/8 [Newcomer] t7004: Remove space after redirect operatorsAbdAlRahman Gad, Aug 5, 2024
  3. 2/8 [Newcomer] t7004: Do not lose exit status to pipeAbdAlRahman Gad, Aug 5, 2024
  4. Eric SunshineAug 6, 2024
  5. AbdAlRahman GadAug 6, 2024
  6. Eric SunshineAug 6, 2024
  7. 3/8 [Newcomer] t7004: One command per lineAbdAlRahman Gad, Aug 5, 2024
  8. 4/8 [Newcomer] t7004: Do not prepare things outside test_expect_successAbdAlRahman Gad, Aug 5, 2024
  9. Eric SunshineAug 6, 2024
  10. AbdAlRahman GadAug 6, 2024
  11. 5/8 [Newcomer] t7004: Use indented here-docAbdAlRahman Gad, Aug 5, 2024
  12. 6/8 [Newcomer] t7004: Description on the same line as test_expect_successAbdAlRahman Gad, Aug 5, 2024
  13. 7/8 [Newcomer] t7004: test Description and test body seperated with backslashAbdAlRahman Gad, Aug 5, 2024
  14. 8/8 [Newcomer] t7004: Use single quotes instead of double quotesAbdAlRahman Gad, Aug 5, 2024
  15. Eric SunshineAug 6, 2024
  16. AbdAlRahman GadAug 6, 2024
  17. Eric SunshineAug 6, 2024
  18. Junio C HamanoAug 6, 2024
  19. Eric SunshineAug 6, 2024
  20. Junio C HamanoAug 6, 2024
  21. AbdAlRahman GadAug 6, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.