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

Re: [PATCH 00/15] tests: don't ignore "git" exit codes

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Mar 3, 2022, 14:06 UTC
Message-ID
<e3090436-4479-bbc2-3b62-00473f6f530e@gmail.com>
In-Reply-To
<76a1ff22-3eb0-08fb-5aa9-e612ee5b522f@github.com>
On 03/03/2022 02:02, Derrick Stolee wrote:
Show 10 quoted lines
> On 3/2/2022 12:27 PM, Ævar Arnfjörð Bjarmason wrote:
>> As an aside we still have various potential issues with hidden
>> segfaults etc. in the test suite after this that are tricked to solve,
>> because:
>>
>>   * Our tests will (mostly) catch segfaults and abort(), but if we
>>     invoke a command that invokes another command it needs to ferry the
>>     exit code up to us.
>>
>>   * run-command.c notably does not do that, so for e.g. "git push"

I'm not sure what you mean by this, the return value of run_command() already indicates which signal if any killed the child see for example run_specified_editor() which re-raises SIGINT and SIGQUIT if the editor is killed by those signals.

>>     tests where we expect a failure and an underlying "git" command
>>     fails we won't ferry up the segfault or abort exit code.
 >
> Perhaps run-command.c could auto-exit for certain well-known error
> codes that could only happen on certain kinds of failures (segfault,
> for example). A simple die() might be something that is expected to
> be handled by the top-level command in some cases.

I think we need to be careful that run_command() does not re-raise a signal before the caller has a chance to do any cleanup. A caller to run_command() can already check the return value and choose to die based on that after doing any cleanup. If run_command() starts dying we'll end up adding more unsafe signal handlers to do the cleanup.

Best Wishes
Phillip
Show 12 quoted lines
>>   * We have gitweb.perl and some other perl code ignoring return values
>>     from close(), i.e. ignoring exit codes from "git rev-parse" et al.
>>
>>   * We have in-tree shellscripts like "git-merge-one-file.sh" invoking
>>     git commands, and if they fail returning "1", not ferrying up the
>>     segfault or abort() exit code.
> 
> These are more involved and harder to evaluate. Add them to the pile
> of projects for new contributors?
> 
> Thanks,
> -Stolee
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 27 of 48 in “tests: don't ignore "git" exit codes”
  1. 00/15 tests: don't ignore "git" exit codesÆvar Arnfjörð Bjarmason, Mar 2, 2022
  2. 07/15 merge tests: use "test_must_fail" instead of ad-hoc patternÆvar Arnfjörð Bjarmason, Mar 2, 2022
  3. 08/15 rev-parse tests: don't ignore "git reflog" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  4. 02/15 tests: use "test_stdout_line_count", not "test $(git [...] | wc -l)"Ævar Arnfjörð Bjarmason, Mar 2, 2022
  5. 05/15 diff tests: don't ignore "git diff" exit code in "read" loopÆvar Arnfjörð Bjarmason, Mar 2, 2022
  6. 01/15 tests: change some 'test $(git) = "x"' to test_cmpÆvar Arnfjörð Bjarmason, Mar 2, 2022
  7. 12/15 gettext tests: don't ignore "test-tool regex" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  8. Junio C HamanoMar 2, 2022
  9. Ævar Arnfjörð BjarmasonMar 3, 2022
  10. Junio C HamanoMar 3, 2022
  11. 13/15 apply tests: don't ignore "git ls-files" exit code, drop sub-shellÆvar Arnfjörð Bjarmason, Mar 2, 2022
  12. 14/15 checkout tests: don't ignore "git <cmd>" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  13. 15/15 rev-list simplify tests: don't ignore "git" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  14. 11/15 rev-list tests: don't hide abort() in "test_expect_failure"Ævar Arnfjörð Bjarmason, Mar 2, 2022
  15. Junio C HamanoMar 2, 2022
  16. Ævar Arnfjörð BjarmasonMar 3, 2022
  17. 10/15 diff tests: don't ignore "git rev-list" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  18. 09/15 notes tests: don't ignore "git" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  19. 03/15 read-tree tests: check "diff-files" exit code on failureÆvar Arnfjörð Bjarmason, Mar 2, 2022
  20. Junio C HamanoMar 2, 2022
  21. 06/15 apply tests: use "test_must_fail" instead of ad-hoc patternÆvar Arnfjörð Bjarmason, Mar 2, 2022
  22. Junio C HamanoMar 2, 2022
  23. 04/15 diff tests: don't ignore "git diff" exit codeÆvar Arnfjörð Bjarmason, Mar 2, 2022
  24. Junio C HamanoMar 2, 2022
  25. Derrick StoleeMar 3, 2022
  26. Ævar Arnfjörð BjarmasonMar 3, 2022
  27. Phillip WoodMar 3, 2022
  28. Ævar Arnfjörð BjarmasonMar 3, 2022
  29. Junio C HamanoMar 4, 2022
  30. Ævar Arnfjörð BjarmasonMar 4, 2022
  31. Shubham MishraMar 3, 2022
  32. Ævar Arnfjörð BjarmasonMar 3, 2022
  33. 00/15 tests: don't ignore "git" exit codesÆvar Arnfjörð Bjarmason, Mar 7, 2022
  34. 01/15 tests: change some 'test $(git) = "x"' to test_cmpÆvar Arnfjörð Bjarmason, Mar 7, 2022
  35. 02/15 tests: use "test_stdout_line_count", not "test $(git [...] | wc -l)"Ævar Arnfjörð Bjarmason, Mar 7, 2022
  36. 03/15 read-tree tests: check "diff-files" exit code on failureÆvar Arnfjörð Bjarmason, Mar 7, 2022
  37. 04/15 diff tests: don't ignore "git diff" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  38. 05/15 diff tests: don't ignore "git diff" exit code in "read" loopÆvar Arnfjörð Bjarmason, Mar 7, 2022
  39. 06/15 apply tests: use "test_must_fail" instead of ad-hoc patternÆvar Arnfjörð Bjarmason, Mar 7, 2022
  40. 07/15 merge tests: use "test_must_fail" instead of ad-hoc patternÆvar Arnfjörð Bjarmason, Mar 7, 2022
  41. 08/15 rev-parse tests: don't ignore "git reflog" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  42. 09/15 notes tests: don't ignore "git" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  43. 10/15 diff tests: don't ignore "git rev-list" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  44. 12/15 gettext tests: don't ignore "test-tool regex" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  45. 13/15 apply tests: don't ignore "git ls-files" exit code, drop sub-shellÆvar Arnfjörð Bjarmason, Mar 7, 2022
  46. 14/15 checkout tests: don't ignore "git <cmd>" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022
  47. 11/15 rev-list tests: don't hide abort() in "test_expect_failure"Ævar Arnfjörð Bjarmason, Mar 7, 2022
  48. 15/15 rev-list simplify tests: don't ignore "git" exit codeÆvar Arnfjörð Bjarmason, Mar 7, 2022

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.