Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Calvin Wan <calvinwan@google.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC PATCH 0/2] add an external testing library for unit tests
Date: Tue, 02 May 2023 15:52:33 +0200	[thread overview]
Message-ID: <230502.861qjyj0cb.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <20230427175007.902278-1-calvinwan@google.com>


On Thu, Apr 27 2023, Calvin Wan wrote:

> In our current testing environment, we spend a significant amount of
> effort crafting end-to-end tests for error conditions that could easily
> be captured by unit tests (or we simply forgo some hard-to-setup and
> rare error conditions). Unit tests additionally provide stability to the
> codebase and can simplify debugging through isolation. Turning parts of
> Git into libraries[1] gives us the ability to run unit tests on the
> libraries and to write unit tests in C. Writing unit tests in pure C,
> rather than with our current shell/test-tool helper setup, simplifies
> test setup, simplifies passing data around (no shell-isms required), and
> reduces testing runtime by not spawning a separate process for every
> test invocation.
>
> Unit testing in C requires a separate testing harness that we ideally
> would like to be TAP-style and to come with a non-restrictive license.
> Fortunately, there already exists a C TAP harness library[2] with an MIT
> license (at least for the files included in this series). 
>
> This first patch introduces the C TAP harness and includes only the
> necessary files. The second patch showcases a basic example of it. As an
> RFC, I am wondering what the list thinks about using a second testing
> library for unit testing? Are there any problems with this particular
> external testing library and does it provide the necessary functionality
> we would want for unit testing Git libraries? How should the folders be
> structured and how should the new Makefile rules be written? Ultimately,
> this will help us determine the setup of our unit tests in future
> libification patches.
>
> [1] https://lore.kernel.org/git/CAJoAoZ=Cig_kLocxKGax31sU7Xe4==BGzC__Bg2_pr7krNq6MA@mail.gmail.com/
> [2] https://github.com/rra/c-tap-harness/ 

I have some out-of-tree patches I've been meaning to submit that massage
some of our TAP output, and I'd really prefer if we don't end up with
two TAP emitters in-tree if we can help it.

We can support such a thing, but nothing about your goals or your
explanation here provides the "why".

Or rather, I'm not really buying the "passing data around" or "recuding
[...] runtime [overhead]". I think you're describing how *some* of our
*.sh to *.c interop goes, but we have test-lib.sh driving C code without
those issues.

We already have pure-C libraries that we add a really shim to unit test,
the most extreme example of this is t0032-reftable-unittest.sh, whose
body is simply (excluding comments):
	
	#!/bin/sh
	test_description='reftable unittests'
	
	TEST_PASSES_SANITIZE_LEAK=true
	. ./test-lib.sh
	
	test_expect_success 'unittests' '
		TMPDIR=$(pwd) && export TMPDIR &&
		test-tool reftable
	'
	
	test_done

Now, that goes into reftable/test_framework.h which basically implements
its own mini-test framework, so that's at least a *partial* argument for
what you're suggesting here, but note that it doesn't emit TAP, it just
returns an exit code, the EXPECT() etc. is purely internal. I.e. "what
should we return?".

Probably a more git-y example is t0071-sort.sh, whose body is similar
(skipping most of the boilerplate):
	
	test_expect_success 'DEFINE_LIST_SORT_DEBUG' '
		test-tool mergesort test
	'

We then have similar library tests as e.g. t0063-string-list.sh,
t0061-run-command.sh, t3070-wildmatch.sh etc.

None of those are perfect, but I think the current arrangement is rather
ideal. We can write most or all of the test in C, but we just do so by
calling a function that returns an exit code.

It does mean we need to spawn a shell for test-lib.sh, and call a
test-tool at least once, but the overhead of that is trivial. It's not
trivial in some cases where we call the helper in a loop, but that's
much more easily addressed than hoisting all of TAP generation into the
C space.

  parent reply	other threads:[~2023-05-02 14:05 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-27 17:50 [RFC PATCH 0/2] add an external testing library for unit tests Calvin Wan
2023-04-27 17:50 ` [RFC PATCH 1/2] Add C TAP harness Calvin Wan
2023-04-27 18:29   ` SZEDER Gábor
2023-04-27 18:38     ` Calvin Wan
2023-04-27 20:15   ` Phillip Wood
2023-04-28 16:31     ` Calvin Wan
2023-05-02 15:46       ` Felipe Contreras
2023-05-10 15:46       ` Phillip Wood
2023-05-11 23:16         ` Glen Choo
2023-05-18 10:04           ` Phillip Wood
2023-06-21 15:57         ` Linus Arver
2023-06-26 13:15           ` Phillip Wood
2023-06-28 21:17             ` Linus Arver
2023-06-29  5:52             ` Oswald Buddenhagen
2023-06-30  9:48               ` Phillip Wood
2023-05-02 15:54     ` Felipe Contreras
2023-05-02 16:39       ` Ævar Arnfjörð Bjarmason
2023-05-02 18:11         ` Felipe Contreras
2023-05-02 16:34     ` Ævar Arnfjörð Bjarmason
2023-05-10  8:18       ` Phillip Wood
2023-04-27 17:50 ` [RFC PATCH 2/2] unit test: add basic example Calvin Wan
2023-04-27 18:47   ` Junio C Hamano
2023-05-02 15:58     ` Felipe Contreras
2023-04-27 18:39 ` [RFC PATCH 0/2] add an external testing library for unit tests Junio C Hamano
2023-04-27 18:46   ` Calvin Wan
2023-04-27 21:35 ` brian m. carlson
2023-05-02  4:18 ` Felipe Contreras
2023-05-02 13:52 ` Ævar Arnfjörð Bjarmason [this message]
2023-05-02 15:28   ` Felipe Contreras

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=230502.861qjyj0cb.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=calvinwan@google.com \
    --cc=git@vger.kernel.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.
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).