Re: [PATCH v8 1/3] unit tests: Add a project plan document
On Tue, Oct 10, 2023 at 12:22 AM Josh Steadmon <steadmon@google.com> wrote:
Show 8 quoted lines
>
> 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). Describe what we hope to accomplish by
> implementing unit tests, and explain some open questions and milestones.
> Discuss desired features for test frameworks/harnesses, and provide a
> preliminary comparison of several different frameworks.
Nit: Not sure why the test framework comparison is "preliminary" as we
have actually selected a unit test framework and are adding it in the
next patch of the series. I understand that this was perhaps written
before the choice was made, but maybe we might want to update that
now.
Show 19 quoted lines
> diff --git a/Documentation/technical/unit-tests.txt b/Documentation/technical/unit-tests.txt
> new file mode 100644
> index 0000000000..b7a89cc838
> --- /dev/null
> +++ b/Documentation/technical/unit-tests.txt
> @@ -0,0 +1,220 @@
> += 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.
> +
> +We believe that a large body of unit tests, living alongside the existing test
> +suite, will improve code quality for the Git project.
Show 5 quoted lines
> +== Choosing a framework
> +
> +We believe the best option is to implement a custom TAP framework for the Git
> +project. We use a version of the framework originally proposed in
> +https://lore.kernel.org/git/c902a166-98ce-afba-93f2-ea6027557176@gmail.com/[1].
Nit: Logically I would think that our opinion should come after the
comparison and be backed by it.
Show 15 quoted lines
> +== Choosing a test harness
> +
> +During upstream discussion, it was occasionally noted that `prove` provides many
> +convenient features, such as scheduling slower tests first, or re-running
> +previously failed tests.
> +
> +While we already support the use of `prove` as a test harness for the shell
> +tests, it is not strictly required. The t/Makefile allows running shell tests
> +directly (though with interleaved output if parallelism is enabled). Git
> +developers who wish to use `prove` as a more advanced harness can do so by
> +setting DEFAULT_TEST_TARGET=prove in their config.mak.
> +
> +We will follow a similar approach for unit tests: by default the test
> +executables will be run directly from the t/Makefile, but `prove` can be
> +configured with DEFAULT_UNIT_TEST_TARGET=prove.
Nice that it can be used.
The rest of the file looks good.
Christian Couder <christian.couder@gmail.com>