Re: [PATCH v3 3/7] unit-tests: add for_test

From: Patrick Steinhardt <ps@pks.im>
Date: Jul 31, 2024, 05:19 UTC
Message-ID: <ZqnJcawtpzvguxtq@tanuki>
In-Reply-To: <53c1eaeb-6106-492c-9c44-13a7cd1bd0d0@web.de>

On Tue, Jul 30, 2024 at 04:00:07PM +0200, René Scharfe wrote:

Show 28 quoted lines

> Am 30.07.24 um 06:49 schrieb Patrick Steinhardt:
> > On Mon, Jul 29, 2024 at 11:55:52AM -0700, Junio C Hamano wrote:
> >> Patrick Steinhardt <ps@pks.im> writes:
> >>
> >>> is standalone nowadays and called "clar" [1]. The biggest downside of it
> >>> is that it depends on Python to auto-generate the test "main" function,
> >>> which is not really a good fit for the Git project.
> >>
> >> Is that because Python is optional (like, we only use it for
> >> optional Perforce thing and import-zip in contrib), or are there
> >> other concerns?
> >>
> >> Unlike these components, unit tests are not even for the end-user
> >> consumers, so if it is Python that you find it a blocker, I do not
> >> see a reason to reject it.  The thing looked like a simple script
> >> that does not use any tricky language construct or modules.
> >
> > No concerns other than adding another mandatory dependency for devs. We
> > already depend on Perl and Shell, so adding Python to the stack feels
> > suboptimal to me.
> >
> > As you say though, the script isn't all that involved. So it would also
> > be possible to port it to Perl if we want to do that, I guess.
> 
> From the point of view of a "minimal C unit testing framework" surely
> the implementation language with the best dependency story would be C.
> Perhaps it could then also test itself.  On the other hand, writing a
> string handling program in 2024 in C is probably not the smartest idea.

I certainly wouldn't want to do it ;) I think Perl would be perfectly fine given that we already rely on it rather extensively in Git.

> Then again, generate.py uses a non-greedy regex to remove single-line
> comments, which seems wrong, and doesn't seem to support preprocessor
> directives like #if and #ifdef, so whole tests can't be disabled this
> way.

I guess it doesn't have to be perfect, just good enough. clar comes with some niceties like being able to run only some tests, all tests of a particular suite or to exclude certain tests:

    # Run only submodule::add tests.
    $ ./libgit2_tests -t -ssubmodule::add::
    TAP version 13
    # start of suite 1: submodule::add
    ok 1 - submodule::add::url_absolute
    ok 2 - submodule::add::url_relative
    ok 3 - submodule::add::url_relative_to_origin
    ok 4 - submodule::add::url_relative_to_workdir
    ok 5 - submodule::add::path_exists_in_index
    ok 6 - submodule::add::file_exists_in_index
    ok 7 - submodule::add::submodule_clone
    ok 8 - submodule::add::submodule_clone_into_nonempty_dir_succeeds
    ok 9 - submodule::add::submodule_clone_twice_fails
    1..9

    # Run only a single test in the submodule::add suite.
    $ ./libgit2_tests -t -ssubmodule::add::url_absolute
    TAP version 13
    # start of suite 1: submodule::add
    ok 1 - submodule::add::url_absolute
    1..1

    # Run all submodule tests, but exclude submodule::add.
    $ ./libgit2_tests -t -ssubmodule::  -xsubmodule::add::
    TAP version 13
    # start of suite 1: submodule::escape
    ok 1 - submodule::escape::from_gitdir
    ok 2 - submodule::escape::from_gitdir_windows
    # start of suite 2: submodule::init
    ok 3 - submodule::init::absolute_url
    ...

So I'm not sure whether it's all that important to be able to recognize things like #if and #ifdef given that you can pick tests to run on the command line.

The alternative would be of course to hook up LLVM and properly parse the C sources. But that feels like overthinking it ;)

> And it uses pickle to cache results; does that mean it would be
> slow without it?

Good question. I plan to find some time this week to draft a PoC and will remember to benchmark this.

Patrick