All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fabian Stelzer <fs@gigacodes.de>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Adam Dinwoodie <adam@dinwoodie.org>,
	Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2 2/2] test-lib: introduce required prereq for test runs
Date: Fri, 19 Nov 2021 16:40:36 +0100	[thread overview]
Message-ID: <20211119154036.5n5kpecgnptzkaqn@fs> (raw)
In-Reply-To: <211119.86sfvs2p9w.gmgdl@evledraar.gmail.com>

On 19.11.2021 15:26, Ævar Arnfjörð Bjarmason wrote:
>
>On Fri, Nov 19 2021, Fabian Stelzer wrote:
>
>> On 19.11.2021 12:13, Ævar Arnfjörð Bjarmason wrote:
>>>
>>>On Wed, Nov 17 2021, Fabian Stelzer wrote:
>>>
>>>> In certain environments or for specific test scenarios we might expect a
>>>> specific prerequisite check to succeed. Therefore we would like to
>>>> trigger an error when running our tests if this is not the case.
>>>
>>>trigger an error but...
>>>
>>>> To remedy this we add the environment variable GIT_TEST_REQUIRE_PREREQ
>>>> which can be set to a comma separated list of prereqs. If one of these
>>>> prereq tests fail then the whole test run will abort.
>>>
>>>..here it's "abort the whole test run". If that's what you want use
>>>BAIL_OUT, not error. See: 234383cd401 (test-lib.sh: use "Bail out!"
>>>syntax on bad SANITIZE=leak use, 2021-10-14)
>>>
>>
>> Hm, while testing this change i noticed another problem that i really
>> have no idea how to fix.
>> When a test uses test_have_prereq then the error/BAIL_OUT message will only be printed
>> when run with '-v'. This is not the case when the prereq is specified
>> in the test header. The test run will abort, but no error will be
>> printed which can be quite confusing :/
>> I guess this has something to do with how tests are run in subshells and
>> their outputs only printed with -v. Maybe there should be some kind of
>> override for BAIL_OUT at least? Not sure if/how this could be done.
>
>It has to do with how we juggle file descriptors around, see test_eval_
>in test-lib.sh.
>
>So the "real" stdout is fd 5, not 1 when you're in a prereq.
>
>Just:
>
>    BAIL_OUT "bad" >&5
>
>Will work, maybe it's a good idea to have:
>
>	BAIL_OUT_PREREQ () {
>		BAIL_OUT $@ >&5
>	}
>
>Sorry, I forgot about that caveat when suggesting it.

Hm. Any reason to not do this in BAIL_OUT itself?  As far as i can see
the setup of the additional fd's would only need to move up a few lines.

  reply	other threads:[~2021-11-19 15:40 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-17  9:04 [PATCH v2 0/2] test-lib: improve missing prereq handling Fabian Stelzer
2021-11-17  9:04 ` [PATCH v2 1/2] test-lib: show missing prereq summary Fabian Stelzer
2021-11-17  9:04 ` [PATCH v2 2/2] test-lib: introduce required prereq for test runs Fabian Stelzer
2021-11-18 23:42   ` Junio C Hamano
2021-11-19  9:07     ` Fabian Stelzer
2021-11-19 11:13   ` Ævar Arnfjörð Bjarmason
2021-11-19 13:48     ` Fabian Stelzer
2021-11-19 14:09     ` Fabian Stelzer
2021-11-19 14:26       ` Ævar Arnfjörð Bjarmason
2021-11-19 15:40         ` Fabian Stelzer [this message]
2021-11-19 16:37           ` Ævar Arnfjörð Bjarmason
2021-11-20 15:03   ` [PATCH v3 0/3] test-lib: improve missing prereq handling Fabian Stelzer
2021-11-20 15:03     ` [PATCH v3 1/3] test-lib: show missing prereq summary Fabian Stelzer
2021-11-20 15:04     ` [PATCH v3 2/3] test-lib: introduce required prereq for test runs Fabian Stelzer
2021-11-20 15:04     ` [PATCH v3 3/3] test-lib: make BAIL_OUT() work in tests and prereq Fabian Stelzer
2021-11-22 11:52       ` Ævar Arnfjörð Bjarmason
2021-11-22 17:05         ` Junio C Hamano
2021-11-26  9:55           ` Fabian Stelzer
2021-11-26 21:02             ` Junio C Hamano
2021-11-27 12:47               ` Fabian Stelzer
2021-11-28 23:38                 ` Junio C Hamano
2021-11-30 14:38                   ` Fabian Stelzer
2021-11-30 14:59                     ` Ævar Arnfjörð Bjarmason
2021-12-01  8:53     ` [PATCH v4 0/3] test-lib: improve missing prereq handling Fabian Stelzer
2021-12-01  8:53       ` [PATCH v4 1/3] test-lib: show missing prereq summary Fabian Stelzer
2021-12-01  8:53       ` [PATCH v4 2/3] test-lib: introduce required prereq for test runs Fabian Stelzer
2021-12-01  8:53       ` [PATCH v4 3/3] test-lib: make BAIL_OUT() work in tests and prereq Fabian Stelzer
2021-12-01 23:13         ` Junio C Hamano
2021-12-01 21:05       ` [PATCH v4 0/3] test-lib: improve missing prereq handling Adam Dinwoodie

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=20211119154036.5n5kpecgnptzkaqn@fs \
    --to=fs@gigacodes.de \
    --cc=adam@dinwoodie.org \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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.