GNU bug report logs - #74394
[PATCH 0/2] Skip slow tests by default and run 'check' in Git pre-push hook.

Previous Next

Full log

Message #17 received at 74394 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 74394 <at> debbugs.gnu.org
Subject: Re: [bug#74394] [PATCH 0/2] Skip slow tests by default and run
 'check' in Git pre-push hook.
Date: Tue, 17 Dec 2024 09:28:59 +0900

Hi Ludovic,

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>
>> This is a simple change that should ensure test suite breakages are detected
>> as early as possible and avoid tests breaking changes to be pushed.  This is
>> made possible by skipping a few expensive tests suite, bringing down the total
>> test time to about 1 minute on a fast machine.
>>
>> We could call it a "distributed CI" approach ;-).
>
> I agree with the goal, of course, but not with the method: even without
> expensive tests, “make check” is going to take maybe 5–10 minutes, and
> having that happen when you run “git push” can be a terrible development
> experience (especially since the developer most likely either already
> ran the test suite or part of it right before, or pushes package changes
> that have infinitely small probability of breaking “make check”).

As I wrote, 'make check' with this change takes about 1 minute on my
machine; I'd be curious to know how long it takes on other people
machines; I suspect a bit more; if it's too slow, we can skip more, or
find out ways to make the tests run faster.

> Back to CI and not breaking things: I think that we should have a
> workflow where the forge triggers those checks and puts a green light if
> it passes, red light otherwise.  (Basically what everybody else is
> doing. :-))
>
> To me this should be one of the goals for the project in 2025.

That would be great.  I wonder how far QA is from making this
achievable.

>> To run the complete test suite including the slow tests (as is the case prior
>> this change):
>>
>> make check WITH_SLOW_TESTS=1
>
> This variable itself may still be useful though (I’d call it
> ‘RUN_EXPENSIVE_TESTS’ or something like that—that’s the name used in
> Coreutils—, “expensive” being the key word).  I would also keep it on by
> default.

One of the tests that was unbearably long when I measured was the
time-machine test.  It took about 20 minutes to fetch the git repository
with guile-git and run the test (which does extra work compared to the
CLI like validating each object).  I don't think we want this kind of
experience by default (because that'd probably convince people that
running the test suite often is not a reasonable thing to do).  The
other tests were more reasonable, with the longer ones in the 2-3
minutes range on my machine, IIRC.  Perhaps we could have this 20 minute
outlier skipped by default, maybe with a RUN_PROHIBITIVE_TESTS flag that
would default to 0 (false).

A long time ago I had read a blog post that argued that unit tests
should be small and fast [0], and there's a lot of good about that.
Fast tests usually translates in running the test suite more often and
catching breakage earlier.  As the author states, it also makes it
possible to determine whether a bug lies in the core logic or in the
integration of the many parts (unit tests vs integration tests), when
unit tests are decoupled from the whole system (typically by mocking all
external interfaces).

[0]  https://www.artima.com/weblogs/viewpost.jsp?thread=126923

-- 
Thanks,
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.