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

Re: [PATCH v3 3/3] test-lib: make BAIL_OUT() work in tests and prereq

From
Fabian Stelzer <fs@gigacodes.de>
Date
Nov 26, 2021, 09:55 UTC
Message-ID
<20211126095509.weeknmg4p6sx7bdn@fs>
In-Reply-To
<xmqqh7c4i0jh.fsf@gitster.g>
On 22.11.2021 09:05, Junio C Hamano wrote:
Show 25 quoted lines
>Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> -exec 5>&1
>>> -exec 6<&0
>>> -exec 7>&2
>>
>> This doesn't break (I think) with your change here because you only
>> manipulate >&5, but I think the post-image would be lot clearer if...
>>
>>>  if test "$verbose_log" = "t"
>>>  then
>>>  	exec 3>>"$GIT_TEST_TEE_OUTPUT_FILE" 4>&3
>>
>> ...this bit of code were moved up along with the "exec". They're
>> currently intentionally snuggled together as we conditionally set the
>> 3rd and 4th fd depending on verbose/tee settings right after the setup
>> of 5/6/7, keeping them grouped in the post-image makes more sense than
>> splitting them up here.
>
>I actually have to wonder if the handling of 3 and 4 should be moved
>down, not up like you suggest, so that they are grouped together
>with the maybe_teardown_verbose and the maybe_setup_verbose helper
>functions.  The lines we see here are the file descriptors that are
>always redirected, which is a bit different.  Raising all of them up
>to group them together is also fine.

Since there is lots of other things happening in between i'd rather leave 3 & 4 where they are for now. I'm having a hard time grasping what test-lib.sh does when / where since there is lots of intermingled function definition / executed code. It could probably benefit from a new include to move functions to (and wrap some code into new ones). At the moment i'm a bit swamped with other work though :/

Show 5 quoted lines
>
>In any case, the comment in front of the block of exec wants to
>become a bit more detailed than just "# Set up additional fds", with
>an explanation about which FD is used for what.
>
How about:

# Set up additional fds to allow i/o with the surrounding test # harness when redirecting individual test i/o in test_eval_ # fd 5 -> stdout # fd 6 <- stdin # fd 7 -> stderr

exec 5>&1 exec 6<&0 exec 7>&2

Previous: Junio C HamanoNext: Junio C Hamano
Message 18 of 29 in “test-lib: improve missing prereq handling”
  1. 0/2 test-lib: improve missing prereq handlingFabian Stelzer, Nov 17, 2021
  2. 1/2 test-lib: show missing prereq summaryFabian Stelzer, Nov 17, 2021
  3. 2/2 test-lib: introduce required prereq for test runsFabian Stelzer, Nov 17, 2021
  4. Junio C HamanoNov 18, 2021
  5. Fabian StelzerNov 19, 2021
  6. Ævar Arnfjörð BjarmasonNov 19, 2021
  7. Fabian StelzerNov 19, 2021
  8. Fabian StelzerNov 19, 2021
  9. Ævar Arnfjörð BjarmasonNov 19, 2021
  10. Fabian StelzerNov 19, 2021
  11. Ævar Arnfjörð BjarmasonNov 19, 2021
  12. 0/3 test-lib: improve missing prereq handlingFabian Stelzer, Nov 20, 2021
  13. 1/3 test-lib: show missing prereq summaryFabian Stelzer, Nov 20, 2021
  14. 2/3 test-lib: introduce required prereq for test runsFabian Stelzer, Nov 20, 2021
  15. 3/3 test-lib: make BAIL_OUT() work in tests and prereqFabian Stelzer, Nov 20, 2021
  16. Ævar Arnfjörð BjarmasonNov 22, 2021
  17. Junio C HamanoNov 22, 2021
  18. Fabian StelzerNov 26, 2021
  19. Junio C HamanoNov 26, 2021
  20. Fabian StelzerNov 27, 2021
  21. Junio C HamanoNov 28, 2021
  22. Fabian StelzerNov 30, 2021
  23. Ævar Arnfjörð BjarmasonNov 30, 2021
  24. 0/3 test-lib: improve missing prereq handlingFabian Stelzer, Dec 1, 2021
  25. 1/3 test-lib: show missing prereq summaryFabian Stelzer, Dec 1, 2021
  26. 2/3 test-lib: introduce required prereq for test runsFabian Stelzer, Dec 1, 2021
  27. 3/3 test-lib: make BAIL_OUT() work in tests and prereqFabian Stelzer, Dec 1, 2021
  28. Junio C HamanoDec 1, 2021
  29. Adam DinwoodieDec 1, 2021

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.