Re: [PATCH v2 2/2] test-lib: introduce required prereq for test runs
- From
Fabian Stelzer <fs@gigacodes.de>
- Date
- Nov 19, 2021, 15:40 UTC
- Message-ID
- <20211119154036.5n5kpecgnptzkaqn@fs>
- In-Reply-To
- <211119.86sfvs2p9w.gmgdl@evledraar.gmail.com>
On 19.11.2021 15:26, Ævar Arnfjörð Bjarmason wrote:
Show 48 quoted lines
>
>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.