On Wed, Jul 14, 2021 at 07:23:51PM +0200, Ævar Arnfjörð Bjarmason wrote:
> While git can be compiled with SANITIZE=leak there has been no
> corresponding GIT_TEST_* mode for it, i.e. memory leaks have been
> fixed as one-offs without structured regression testing.
This opening puzzled me. I'm not sure I understand why we need a special
GIT_TEST_* mode for it. If you do "make SANITIZE=leak test", then your
binaries will leak-check while running the tests.
I.e., there is nothing that test-lib.sh itself needs to do differently
to enable it.
What we _do_ need is some mechanism of annotating to tests to say "this
is known to leak", so that we can skip them for normal integration runs.
And that is part of what's going on in this patch, but I'm not sure it
is the simplest way to do it. The first question is: how do we want to
annotate the tests. By marking individual scripts or tests in the
test-files themselves? Or by using a separate list of "these scripts or
tests are known to pass"?
IMHO the latter is preferable. It keeps the annotations out of the way
of normal work (they are a temporary thing until we eventually pass the
whole suite leak free, but I expect they'll be with us for a while). The
downside is that the annotations may get out of sync with test numbers.
But if we are primarily annotating whole scripts (and not individual
tests), then that is generally pretty stable.
And with that in mind, can we just use an existing mechanism for picking
which tests to run, and drive it externally from the CI job?
We already have GIT_SKIP_TESTS and --run. Those are perhaps a bit
awkward for feeding huge lists to, and there is no environment
equivalent for --run (so you can't trigger it easily from "make test").
But what if we could do something like:
GIT_TEST_RUN_FROM=t/leak-free make SANITIZE=leak test
and then t/leak-free contained the usual patterns like:
t000*
t1234.5
and so on. That requires two new features in test-lib.sh:
- making a GIT_TEST_RUN variable that is the opposite of GIT_TEST_SKIP
(instead of just the command-line --run).
- adding GIT_TEST_{RUN,SKIP}_FROM variables to read the values from a
file rather than the environment (I suppose the caller could just
stuff the contents into the variable, but I expect that test-lib.sh
may want to pare down the entries that do not even apply to the
current script for the sake of efficiency in checking each test).
That infrastructure would then be applicable to other cases, too. Or
even just useful for using another list (or no list at all) when you
are looking at whether other tests are leak-free or not.
> This change add such a mode, we now have new
> linux-{clang,gcc}-sanitize-leak CI targets, these targets run the same
> tests as linux-{clang,gcc}, except that almost all of them are
> skipped.
I'm not clear on what we expect to get out of running it with both clang
and gcc. They should be producing identical results.
-Peff
