From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: 74394@debbugs.gnu.org
Subject: [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 [thread overview]
Message-ID: <87zfkvrw38.fsf@gmail.com> (raw)
In-Reply-To: <87y1122w3k.fsf@gnu.org> ("Ludovic Courtès"'s message of "Fri, 29 Nov 2024 11:05:51 +0100")
Hi Ludovic,
Ludovic Courtès <ludo@gnu.org> writes:
> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@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
next prev parent reply other threads:[~2024-12-17 0:31 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-17 12:03 [bug#74394] [PATCH 0/2] Skip slow tests by default and run 'check' in Git pre-push hook Maxim Cournoyer
2024-11-17 12:21 ` [bug#74394] [PATCH 1/2] build: Exclude expensive tests in check target by default Maxim Cournoyer
2024-11-17 12:21 ` [bug#74394] [PATCH 2/2] etc: Ensure test suite passes in pre-push git hook Maxim Cournoyer
2024-11-29 10:05 ` [bug#74394] [PATCH 0/2] Skip slow tests by default and run 'check' in Git pre-push hook Ludovic Courtès
2024-12-17 0:28 ` Maxim Cournoyer [this message]
2024-12-17 14:51 ` Ludovic Courtès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zfkvrw38.fsf@gmail.com \
--to=maxim.cournoyer@gmail.com \
--cc=74394@debbugs.gnu.org \
--cc=ludo@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).