[v6,0/3] Add unit test framework and project plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit

Message

Josh Steadmon Aug. 16, 2023, 11:50 p.m. UTC

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==[e-mail]/
[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/[e-mail]


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 <[e-mail]>
    +    Co-authored-by: Calvin Wan <[e-mail]>
         Signed-off-by: Calvin Wan <[e-mail]>
         Signed-off-by: Josh Steadmon <[e-mail]>
     
    @@ 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/[e-mail]/[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/[e-mail]/[stdlib
     +  work]
-:  ---------- > 2:  ca284c575e unit tests: add TAP unit test framework
-:  ---------- > 3:  ea33518d00 ci: run unit tests in CI

base-commit: a9e066fa63149291a55f383cfa113d8bdbdaa6b3