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. This series begins with a project document covering our goals for adding unit tests and a discussion of alternative frameworks considered, as well as the features used to evaluate them. A rendered preview of this doc can be found at [2]. It also adds Phillip Wood's TAP implemenation (with some slightly re-worked Makefile rules) and a sample strbuf unit test. Finally, we modify the configs for GitHub and Cirrus CI to run the unit tests. Sample runs showing successful CI runs can be found at [3], [4], and [5]. [1] https://lore.kernel.org/git/CAJoAoZ=Cig_kLocxKGax31sU7Xe4==BGzC__Bg2_pr7krNq6MA@xxxxxxxxxxxxxx/ [2] https://github.com/steadmon/git/blob/unit-tests-asciidoc/Documentation/technical/unit-tests.adoc [3] https://github.com/steadmon/git/actions/runs/5884659246/job/15959781385#step:4:1803 [4] https://github.com/steadmon/git/actions/runs/5884659246/job/15959938401#step:5:186 [5] https://cirrus-ci.com/task/6126304366428160 (unrelated tests failed, but note that t-strbuf ran successfully) In addition to reviewing the patches in this series, reviewers can help this series progress by chiming in on these remaining TODOs: - Figure out how to ensure tests run on additional OSes such as NonStop - Figure out if we should collect unit tests statistics similar to the "counts" files for shell tests - Decide if it's OK to wait on sharding unit tests across "sliced" CI instances - Provide guidelines for writing new unit tests Changes in v6: - Officially recommend using Phillip Wood's TAP framework - Add an example strbuf unit test using the TAP framework as well as Makefile integration - Run unit tests in CI Changes in v5: - Add comparison point "License". - Discuss feature priorities - Drop frameworks: - Incompatible licenses: libtap, cmocka - Missing source: MyTAP - No TAP support: µnit, cmockery, cmockery2, Unity, minunit, CUnit - Drop comparison point "Coverage reports": this can generally be handled by tools such as `gcov` regardless of the framework used. - Drop comparison point "Inline tests": there didn't seem to be strong interest from reviewers for this feature. - Drop comparison point "Scheduling / re-running": this was not supported by any of the main contenders, and is generally better handled by the harness rather than framework. - Drop comparison point "Lazy test planning": this was supported by all frameworks that provide TAP output. Changes in v4: - Add link anchors for the framework comparison dimensions - Explain "Partial" results for each dimension - Use consistent dimension names in the section headers and comparison tables - Add "Project KLOC", "Adoption", and "Inline tests" dimensions - Fill in a few of the missing entries in the comparison table Changes in v3: - Expand the doc with discussion of desired features and a WIP comparison. - Drop all implementation patches until a framework is selected. - Link to v2: https://lore.kernel.org/r/20230517-unit-tests-v2-v2-0-21b5b60f4b32@xxxxxxxxxx Josh Steadmon (2): unit tests: Add a project plan document ci: run unit tests in CI Phillip Wood (1): unit tests: add TAP unit test framework .cirrus.yml | 2 +- Documentation/Makefile | 1 + Documentation/technical/unit-tests.txt | 220 +++++++++++++++++ Makefile | 24 +- ci/run-build-and-tests.sh | 2 + ci/run-test-slice.sh | 5 + t/Makefile | 15 +- t/t0080-unit-test-output.sh | 58 +++++ t/unit-tests/.gitignore | 2 + t/unit-tests/t-basic.c | 95 +++++++ t/unit-tests/t-strbuf.c | 75 ++++++ t/unit-tests/test-lib.c | 329 +++++++++++++++++++++++++ t/unit-tests/test-lib.h | 143 +++++++++++ 13 files changed, 966 insertions(+), 5 deletions(-) create mode 100644 Documentation/technical/unit-tests.txt create mode 100755 t/t0080-unit-test-output.sh create mode 100644 t/unit-tests/.gitignore create mode 100644 t/unit-tests/t-basic.c create mode 100644 t/unit-tests/t-strbuf.c create mode 100644 t/unit-tests/test-lib.c create mode 100644 t/unit-tests/test-lib.h Range-diff against v5: 1: c7dca1a805 ! 1: 81c5148a12 unit tests: Add a project plan document @@ Commit message preliminary comparison of several different frameworks. - Coauthored-by: Calvin Wan <calvinwan@xxxxxxxxxx> + Co-authored-by: Calvin Wan <calvinwan@xxxxxxxxxx> Signed-off-by: Calvin Wan <calvinwan@xxxxxxxxxx> Signed-off-by: Josh Steadmon <steadmon@xxxxxxxxxx> @@ Documentation/technical/unit-tests.txt (new) + +== Choosing a framework + -+=== Desired features & feature priority ++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@xxxxxxxxx/[1]. ++ ++ ++== 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. ++ ++ ++== Framework selection + +There are a variety of features we can use to rank the candidate frameworks, and +those features have different priorities: @@ Documentation/technical/unit-tests.txt (new) +** <<adoption,Adoption>> + +[[license]] -+==== License ++=== License + +We must be able to legally use the framework in connection with Git. As Git is +licensed only under GPLv2, we must eliminate any LGPLv3, GPLv3, or Apache 2.0 +projects. + +[[vendorable-or-ubiquitous]] -+==== Vendorable or ubiquitous ++=== Vendorable or ubiquitous + +We want to avoid forcing Git developers to install new tools just to run unit +tests. Any prospective frameworks and harnesses must either be vendorable @@ Documentation/technical/unit-tests.txt (new) +tools installed already. + +[[maintainable-extensible]] -+==== Maintainable / extensible ++=== Maintainable / extensible + +It is unlikely that any pre-existing project perfectly fits our needs, so any +project we select will need to be actively maintained and open to accepting @@ Documentation/technical/unit-tests.txt (new) +conditions holds. + +[[major-platform-support]] -+==== Major platform support ++=== Major platform support + +At a bare minimum, unit-testing must work on Linux, MacOS, and Windows. + @@ Documentation/technical/unit-tests.txt (new) +more platforms, but it is still usable in principle. + +[[tap-support]] -+==== TAP support ++=== TAP support + +The https://testanything.org/[Test Anything Protocol] is a text-based interface +that allows tests to communicate with a test harness. It is already used by @@ Documentation/technical/unit-tests.txt (new) +further. + +[[diagnostic-output]] -+==== Diagnostic output ++=== Diagnostic output + +When a test case fails, the framework must generate enough diagnostic output to +help developers find the appropriate test case in source code in order to debug +the failure. + +[[runtime-skippable-tests]] -+==== Runtime-skippable tests ++=== Runtime-skippable tests + +Test authors may wish to skip certain test cases based on runtime circumstances, +so the framework should support this. + +[[parallel-execution]] -+==== Parallel execution ++=== Parallel execution + +Ideally, we will build up a significant collection of unit test cases, most +likely split across multiple executables. It will be necessary to run these @@ Documentation/technical/unit-tests.txt (new) +parallelism can be handled by the test harness. + +[[mock-support]] -+==== Mock support ++=== Mock support + +Unit test authors may wish to test code that interacts with objects that may be +inconvenient to handle in a test (e.g. interacting with a network service). @@ Documentation/technical/unit-tests.txt (new) +for more convenient tests. + +[[signal-error-handling]] -+==== Signal & error handling ++=== Signal & error handling + +The test framework should fail gracefully when test cases are themselves buggy +or when they are interrupted by signals during runtime. + +[[project-kloc]] -+==== Project KLOC ++=== Project KLOC + +The size of the project, in thousands of lines of code as measured by +https://dwheeler.com/sloccount/[sloccount] (rounded up to the next multiple of +1,000). As a tie-breaker, we probably prefer a project with fewer LOC. + +[[adoption]] -+==== Adoption ++=== Adoption + +As a tie-breaker, we prefer a more widely-used project. We use the number of +GitHub / GitLab stars to estimate this. @@ Documentation/technical/unit-tests.txt (new) +https://libcheck.github.io/check/[Check],[lime-background]#LGPL v2.1#,[red-background]#False#,[yellow-background]#Partial#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[red-background]#False#,[red-background]#False#,[red-background]#False#,[lime-background]#True#,17,973 +|===== + -+==== Alternatives considered ++=== Additional framework candidates + +Several suggested frameworks have been eliminated from consideration: + @@ Documentation/technical/unit-tests.txt (new) +** https://github.com/siu/minunit[minunit] +** https://cunit.sourceforge.net/[CUnit] + -+==== Suggested framework -+ -+Considering the feature priorities and comparison listed above, a custom -+framework seems to be the best option. -+ -+ -+== Choosing a test harness -+ -+During upstream discussion, it was occasionally noted that `prove` provides many -+convenient features. While we already support the use of `prove` as a test -+harness for the shell tests, it is not strictly required. -+ -+IMPORTANT: It is an open question whether or not we wish to rely on `prove` as a -+strict dependency for running unit tests. -+ + +== Milestones + -+* Get upstream agreement on implementing a custom test framework -+* Determine if it's OK to rely on `prove` for running unit tests +* Add useful tests of library-like code -+* Integrate with Makefile -+* Integrate with CI +* Integrate with + https://lore.kernel.org/git/20230502211454.1673000-1-calvinwan@xxxxxxxxxx/[stdlib + work] -: ---------- > 2: ca284c575e unit tests: add TAP unit test framework -: ---------- > 3: ea33518d00 ci: run unit tests in CI base-commit: a9e066fa63149291a55f383cfa113d8bdbdaa6b3 -- 2.41.0.694.ge786442a9b-goog
