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

Re: [RFC PATCH 1/3] test-tool: don't fake up BUG() exits as code 99

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jun 3, 2022, 23:03 UTC
Message-ID
<220604.867d5xxs90.gmgdl@evledraar.gmail.com>
In-Reply-To
<xmqqv8theehf.fsf@gitster.g>
On Fri, Jun 03 2022, Junio C Hamano wrote:
Show 17 quoted lines
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Change the BUG() function invoked be "test-tool" to be the "real" one,
>> instead of one that avoids producing core files. In
>> a86303cb5d5 (test-tool: help verifying BUG() code paths, 2018-05-02)
>> to test the (then recently added) BUG() function we faked up the
>> abort() in favor of an exit with code 99.
>>
>> However, in doing so we've been fooling ourselves when it comes to
>> what trace2 events we log. The events tested for in
>> 0a9dde4a04c (usage: trace2 BUG() invocations, 2021-02-05) are not the
>> real ones, but those that we emit only from the "test-tool".
>
> I can fully agree with the above reasoning, i.e. let's test what we
> do use in production, instead of something nobody uses for real, if
> we were adding a test for BUG() in vacuum, but why did we have to
> "fake" it in the first place?

Per a86303cb5d5 (test-tool: help verifying BUG() code paths, 2018-05-02) I think it was to get rid of coredump clutter outside of trash directories (which would require some non-standard config), which doesn't seem worth it.

Show 8 quoted lines
>> Let's just stop faking it, and call abort(). As a86303cb5d5 notes this
>> will produce core files on some OS's, but as the default behavior for
>> that is to dump them in the current directory they'll be placed in the
>> trash directory that we'll shortly me "rm -rf"-ing.
>
> Are we sure that the reason no longer applies?  How do we know?  We
> would want to explain that to future developers in the proposed log
> message, I would think.

I was trying to get the above across, i.e. that the reasoning still applies, but that the fakery isn't worth it.

Show 8 quoted lines
>> +	elif test_match_signal 6 $exit_code && list_contains "$_test_ok" sigabrt
>> +	then
>> +		return 0
>>  	elif test_match_signal 13 $exit_code && list_contains "$_test_ok" sigpipe
>
> Not a new problem, but these numberings are probably not very
> portable.  I am willing to take this as-is and let people on
> minority platforms complain ;-)
I'll test it better for portability before a non-RFC.
Previous: Jeff KingNext: Ævar Arnfjörð Bjarmason
Message 11 of 16 in “trace2: log "signal" end events if we invoke BUG()”
  1. 0/3 trace2: log "signal" end events if we invoke BUG()Ævar Arnfjörð Bjarmason, May 26, 2022
  2. 2/3 refs API: rename "abort" callback to avoid macro clashÆvar Arnfjörð Bjarmason, May 26, 2022
  3. 1/3 test-tool: don't fake up BUG() exits as code 99Ævar Arnfjörð Bjarmason, May 26, 2022
  4. Junio C HamanoJun 3, 2022
  5. Junio C HamanoJun 3, 2022
  6. Ævar Arnfjörð BjarmasonJun 3, 2022
  7. Jeff KingJun 8, 2022
  8. Junio C HamanoJun 8, 2022
  9. Ævar Arnfjörð BjarmasonJun 9, 2022
  10. Jeff KingJun 9, 2022
  11. Ævar Arnfjörð BjarmasonJun 3, 2022
  12. 3/3 trace2: emit "signal" events after calling BUG()Ævar Arnfjörð Bjarmason, May 26, 2022
  13. Bagas SanjayaMay 26, 2022
  14. Josh SteadmonMay 31, 2022
  15. Junio C HamanoMay 26, 2022
  16. Josh SteadmonMay 31, 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.