On 10/06/2021 15:38, Jeff King wrote:
I ran it, noted the failing tests, produced a giant GIT_SKIP_TESTS list
and hacked ci/ to run that as a new linux-clang-SANITIZE job. That messy
WIP code is currently running at:
https://github.com/avar/git/runs/2793150092
Wouldn't it be a good idea to have such a job and slowly work on the
exclusion list?
E.g. I saw that t0004 failed, which was trivially fixed with a single
strbuf_release(), and we could guard against regressions.
I don't mind that. My intent was to get the whole suite clean
eventually, and then start worrying about regressions. But that may take
a while.
I do think it would be worth splitting out ASan from leak-checking. The
whole suite should run clean with regular ASan already, and we'd want to
find regressions there even in the tests that aren't leak-clean. I do
periodic ASan runs already; the main argument against doing it for every
CI run is just that's a lot more CPU. But maybe not enough to be
prohibitive? It's probably still way cheaper than running the test suite
on Windows.
I've been running tests with ASAN in the Github Actions environment, and
a single run takes just over 30 minutes [1] - which I believe is similar
to the normal test jobs (they do run the test suite twice in that time I
think).
I've been doing the same with UBSAN, and that's even faster at 15-20
minutes [2]. However I get the impression that ASAN issues are both more
common (at least on seen), and more impactful - so I would argue that
ASAN should be prioritised if there's spare capacity. (I have no idea if
ASAN+UBSAN can be combined, but I suspect that doing so would make the
tests slower?)
I'm also running LSAN tests in CI to try and catch regressions, but I've
only enabled a handful of tests so far. My much simpler approach was to
specify the range of tests to run as 0-X, and as we make progress on
fixing leaks, X will slowly approach 9999 (currently we're at something
like X~=5, although I'm not too far off sending out some patches to
boost that to 99). The skip-tests approach seems much more useful!
ATB,
Andrzej
[1] https://github.com/ahunt/git/runs/2789921851?check_suite_focus=true
[2] https://github.com/ahunt/git/runs/2760632000?check_suite_focus=true
