All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Josh Steadmon <steadmon@google.com>,
	Calvin Wan <calvinwan@google.com>,
	Emily Shaffer <emilyshaffer@google.com>,
	Glen Choo <chooglen@google.com>, John Cai <johncai86@gmail.com>
Subject: Re: [RFC PATCH 1/3] test-tool: don't fake up BUG() exits as code 99
Date: Fri, 03 Jun 2022 12:25:32 -0700	[thread overview]
Message-ID: <xmqqv8theehf.fsf@gitster.g> (raw)
In-Reply-To: <RFC-patch-1.3-78431bdc8f0-20220525T234908Z-avarab@gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Thu, 26 May 2022 02:30:42 +0200")

Æ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?

> 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.

> +	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 ;-)

  reply	other threads:[~2022-06-03 19:25 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-26  0:30 [RFC PATCH 0/3] trace2: log "signal" end events if we invoke BUG() Ævar Arnfjörð Bjarmason
2022-05-26  0:30 ` [RFC PATCH 1/3] test-tool: don't fake up BUG() exits as code 99 Ævar Arnfjörð Bjarmason
2022-06-03 19:25   ` Junio C Hamano [this message]
2022-06-03 21:05     ` Junio C Hamano
2022-06-03 23:05       ` Ævar Arnfjörð Bjarmason
2022-06-08 19:17       ` Jeff King
2022-06-08 21:03         ` Junio C Hamano
2022-06-09  8:09         ` Ævar Arnfjörð Bjarmason
2022-06-09 15:23           ` Jeff King
2022-06-03 23:03     ` Ævar Arnfjörð Bjarmason
2022-05-26  0:30 ` [RFC PATCH 2/3] refs API: rename "abort" callback to avoid macro clash Ævar Arnfjörð Bjarmason
2022-05-26  0:30 ` [RFC PATCH 3/3] trace2: emit "signal" events after calling BUG() Ævar Arnfjörð Bjarmason
2022-05-26  3:04   ` Bagas Sanjaya
2022-05-31 18:16   ` Josh Steadmon
2022-05-26  1:20 ` [RFC PATCH 0/3] trace2: log "signal" end events if we invoke BUG() Junio C Hamano
2022-05-31 17:59   ` Josh Steadmon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqv8theehf.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=calvinwan@google.com \
    --cc=chooglen@google.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=johncai86@gmail.com \
    --cc=steadmon@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.