From: Phillip Wood <phillip.wood123@gmail.com>
To: steadmon@google.com, git@vger.kernel.org
Cc: calvinwan@gmail.com, szeder.dev@gmail.com, chooglen@google.com,
avarab@gmail.com, gitster@pobox.com,
sandals@crustytoothpaste.net
Subject: Re: [PATCH RFC v2 2/4] unit tests: Add a project plan document
Date: Thu, 18 May 2023 14:13:32 +0100 [thread overview]
Message-ID: <669532af-fa0b-5dea-7da2-ac5caa0560c8@gmail.com> (raw)
In-Reply-To: <20230517-unit-tests-v2-v2-2-21b5b60f4b32@google.com>
On 18/05/2023 00:56, steadmon@google.com wrote:
> Describe what we hope to accomplish by implementing unit tests, and
> explain some open questions and milestones.
Thanks for adding this.
> Change-Id: I182cdc1c15bdd1cbef6ffcf3d216b386f951e9fc
> ---
> Documentation/Makefile | 1 +
> Documentation/technical/unit-tests.txt | 47 ++++++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index b629176d7d..3f2383a12c 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -122,6 +122,7 @@ TECH_DOCS += technical/scalar
> TECH_DOCS += technical/send-pack-pipeline
> TECH_DOCS += technical/shallow
> TECH_DOCS += technical/trivial-merge
> +TECH_DOCS += technical/unit-tests
> SP_ARTICLES += $(TECH_DOCS)
> SP_ARTICLES += technical/api-index
>
> diff --git a/Documentation/technical/unit-tests.txt b/Documentation/technical/unit-tests.txt
> new file mode 100644
> index 0000000000..7c575e6ef7
> --- /dev/null
> +++ b/Documentation/technical/unit-tests.txt
> @@ -0,0 +1,47 @@
> += Unit Testing
> +
> +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. 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.
As we're already using prove as a TAP harness for our existing tests I'd
prefer not to add another harness unless we really need to. prove allows
us to run tests in parallel and has options for rerunning only the tests
that failed last time and for running slow tests first to reduce the
overall run time.
I haven't looked at runtests in detail but at a quick glance I'm not
sure which of those features it supports. I'm also worried about its
windows compatibility. I see it sets some environment variables that
some features of the test library require. I'm not sure if we plan to
use those features, if we do I think we could probably set those paths
when the tests are compiled.
Are you able to expand on why it needs a non-restrictive license? From a
technical point of view surely anything that is GPLv2 compatible would
be fine as that is the license we're already using for our code.
> Fortunately,
> +there already exists a https://github.com/rra/c-tap-harness/[C TAP harness
> +library] with an MIT license (at least for the files needed for our purposes).
> +We might also consider implementing
> +https://lore.kernel.org/git/c902a166-98ce-afba-93f2-ea6027557176@gmail.com/[our
> +own TAP harness] just for Git.
If we do decide to go that route I'm very happy for you or one of your
colleagues to take that patch forward.
> +We believe that a large body of unit tests, living alongside the existing test
> +suite, will improve code quality for the Git project.
This is slightly off-topic and can be addressed later. One thing that
occurred to me was that if we end up with hundreds of unit files it
would be good to link them into a single executable as we do with
test-tool to avoid wasting time and disc space having to link hundreds
of individual programs. We'd have to figure out how to run the
individual tests though if we do that.
> +
> +== Open questions
> +
> +=== TAP harness
> +
> +We'll need to decide on a TAP harness. The C TAP library is easy to integrate,
> +but has a few drawbacks:
> +* (copy objections from lore thread)
> +* We may need to carry local patches against C TAP. We'll need to decide how to
> + manage these. We could vendor the code in and modify them directly, or use a
> + submodule (but then we'll need to decide on where to host the submodule with
> + our patches on top).
> +
> +Phillip Wood has also proposed a new implementation of a TAP harness (linked
> +above). While it hasn't been thoroughly reviewed yet, it looks to support a few
> +nice features that C TAP does not, e.g. lazy test plans and skippable tests.
strictly speaking both those are supported in terms of TAP output by
c-tap-harness but they're not very friendly to use. For me the big
difference is that my library provides a set of check* macros and
functions that automatically print diagnostic messages when a check
fails and the test framework maintains the pass/fail state based on
those checks.
Best Wishes
Phillip
> +== Milestones
> +
> +* Settle on final TAP harness
> +* Add useful tests of library-ish code
> +* Integrate with CI
> +* Integrate with
> + https://lore.kernel.org/git/20230502211454.1673000-1-calvinwan@google.com/[stdlib
> + work]
> +* Run along with regular `make test` target
>
next prev parent reply other threads:[~2023-05-18 13:15 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-17 23:56 [PATCH RFC v2 0/4] Add an external testing library for unit tests steadmon
2023-05-17 23:56 ` [PATCH RFC v2 1/4] common-main: split common_exit() into a new file steadmon
2023-05-18 17:17 ` Junio C Hamano
2023-07-14 23:38 ` Splitting common-main (Was: Re: [PATCH RFC v2 1/4] common-main: split common_exit() into a new file) Josh Steadmon
2023-07-15 0:34 ` Splitting common-main Junio C Hamano
2023-08-14 13:09 ` Splitting common-main (Was: Re: [PATCH RFC v2 1/4] common-main: split common_exit() into a new file) Jeff Hostetler
2023-05-17 23:56 ` [PATCH RFC v2 2/4] unit tests: Add a project plan document steadmon
2023-05-18 13:13 ` Phillip Wood [this message]
2023-05-18 20:15 ` Glen Choo
2023-05-24 17:40 ` Josh Steadmon
2023-06-01 9:19 ` Phillip Wood
2023-05-17 23:56 ` [PATCH RFC v2 3/4] Add C TAP harness steadmon
2023-05-18 13:15 ` Phillip Wood
2023-05-18 20:50 ` Josh Steadmon
2023-05-17 23:56 ` [PATCH RFC v2 4/4] unit test: add basic example and build rules steadmon
2023-05-18 13:32 ` Phillip Wood
2023-06-09 23:25 ` [RFC PATCH v3 0/1] Add a project document for adding unit tests Josh Steadmon
2023-06-09 23:25 ` [RFC PATCH v3 1/1] unit tests: Add a project plan document Josh Steadmon
2023-06-13 22:30 ` Junio C Hamano
2023-06-30 22:18 ` Josh Steadmon
2023-06-29 19:42 ` Linus Arver
2023-06-29 20:48 ` Josh Steadmon
2023-06-30 19:31 ` Linus Arver
2023-07-06 18:24 ` Glen Choo
2023-07-06 19:02 ` Junio C Hamano
2023-07-06 22:48 ` Glen Choo
2023-06-30 21:33 ` Josh Steadmon
2023-06-29 21:21 ` Junio C Hamano
2023-06-30 0:11 ` Linus Arver
2023-06-30 14:07 ` Phillip Wood
2023-06-30 18:47 ` K Wan
2023-06-30 22:35 ` Josh Steadmon
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=669532af-fa0b-5dea-7da2-ac5caa0560c8@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=avarab@gmail.com \
--cc=calvinwan@gmail.com \
--cc=chooglen@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=phillip.wood@dunelm.org.uk \
--cc=sandals@crustytoothpaste.net \
--cc=steadmon@google.com \
--cc=szeder.dev@gmail.com \
/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).