Re: [PATCH] tests: add a special setup where rebase.useBuiltin is off (Re: Git Test Coverage Report (v2.20.0-rc0))

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 19, 2018, 21:03 UTC
Message-ID: <87wop8zsol.fsf@evledraar.gmail.com>
In-Reply-To: <20181119185109.GA47930@google.com>

On Mon, Nov 19 2018, Jonathan Nieder wrote:

Show 24 quoted lines

> Ævar Arnfjörð Bjarmason wrote:
>> On Mon, Nov 19 2018, Derrick Stolee wrote:
>
>>> [...]
>>> builtin/rebase.c
>>> 62c23938fa 55) return env;
>>> [...]
>>> Ævar Arnfjörð Bjarmason 62c23938f: tests: add a special setup
>>> where rebase.useBuiltin is off
>>
>> This one would be covered with
>> GIT_TEST_REBASE_USE_BUILTIN=false. Obviously trivial, but I wonder if
>> the rest of the coverage would look different when passed through the various GIT_TEST_* options.
>
> I wonder if we can do better for this kind of thing.
>
> When I do routine development, I am not running tests with any
> non-default flags.  So why should tests run with non-default flags
> count toward coverage?  Is there a way to make the default test
> settings dip their feet into some non-default configurations, without
> running the full battery of tests and slowing tests down accordingly?
> E.g. is there some kind of smoke test that rebase with
> useBuiltin=false works at all that could run, even if I am not running
> the full battery of rebase tests?

Yeah, definitely. Just pointing out that it would smoke out coverage we don't have at all v.s. cases where we just don't have coverage with the default tests without any special modes.

Derrick: I think it would be useful to produce some delta report showing
covered lines without any GIT_TEST_* variables v.s. when they're set. As
Jonathan points out those should ideally be tested with the normal test
suite, leaving GIT_TEST_* just for stress testing to find new bugs.

> That's a bit of a non sequitor for this example, which is actual code
> to handle GIT_TEST_REBASE_USE_BUILTIN, though.  For it, I wonder why
> we need rebase.c to understand the envvar --- couldn't test-lib.sh
> take care of setting rebase.useBuiltin to false when it's set?

I guess the test-lib.sh could pass down things like "GIT_CONFIG_PARAMETERS='x.y=z'". Maybe that's a better way to do it.