'[PATCH v4] unit tests: Add a project plan document'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    [PATCH v4] unit tests: Add a project plan document
From:       Josh Steadmon <steadmon () google ! com>
Date:       2023-06-30 22:51:19
Message-ID: 0169ce6fb9ccafc089b74ae406db0d1a8ff8ac65.1688165272.git.steadmon () google ! com
[Download RAW message or body]

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.

Coauthored-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Josh Steadmon <steadmon@google.com>
---
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 patch adds a project document describing our goals for adding unit
tests, as well as a discussion of features needed from prospective test
frameworks or harnesses. It also includes a WIP comparison of various
proposed frameworks. Later iterations of this series will probably
include a sample unit test and Makefile integration once we've settled
on a framework. A rendered preview of this doc can be found at [2].

In addition to reviewing the document itself, reviewers can help this
series progress by helping to fill in the framework comparison table.

[1] https://lore.kernel.org/git/CAJoAoZ=Cig_kLocxKGax31sU7Xe4==BGzC__Bg2_pr7krNq6MA@mail.gmail.com/
 [2] https://github.com/steadmon/git/blob/unit-tests-asciidoc/Documentation/technical/unit-tests.adoc


TODOs remaining:
- List rough priorities across comparison dimensions
- Group dimensions into sensible categories
- Discuss pre-existing harnesses for the current test suite
- Discuss harness vs. framework features, particularly for parallelism
- Figure out how to evaluate frameworks on additional OSes such as *BSD
  and NonStop
- Add more discussion about desired features (particularly mocking)
- Add dimension for test timing
- Evaluate remaining missing comparison table entries

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@google.com


 Documentation/Makefile                 |   1 +
 Documentation/technical/unit-tests.txt | 196 +++++++++++++++++++++++++
 2 files changed, 197 insertions(+)
 create mode 100644 Documentation/technical/unit-tests.txt

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..e302a0e40f
--- /dev/null
+++ b/Documentation/technical/unit-tests.txt
@@ -0,0 +1,196 @@
+= 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.
+
+== Definitions
+
+For the purposes of this document, we'll use *test framework* to refer to
+projects that support writing test cases and running tests within the context
+of a single executable. *Test harness* will refer to projects that manage
+running multiple executables (each of which may contain multiple test cases) and
+aggregating their results.
+
+In reality, these terms are not strictly defined, and many of the projects
+discussed below contain features from both categories.
+
+
+== Choosing a framework & harness
+
+=== Desired features
+
+[[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
+Git's integration test suite. Supporting TAP output is a mandatory feature for
+any prospective test framework.
+
+In the comparison table below, "True" means this is natively supported.
+"Partial" means TAP output must be generated by post-processing the native
+output.
+
+Frameworks that do not have at least Partial support will not be evaluated
+further.
+
+[[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.
+
+[[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
+tests in parallel to enable fast develop-test-debug cycles.
+
+In the comparison table below, "True" means that individual test cases within a
+single test executable can be run in parallel. "Partial" means that test cases
+are run serially within a single executable, but multiple test executables can
+be run at once (with proper harness support).
+
+[[vendorable-or-ubiquitous]]
+==== Vendorable or ubiquitous
+
+If possible, we want to avoid forcing Git developers to install new tools just
+to run unit tests. So any prospective frameworks and harnesses must either be
+vendorable (meaning, we can copy their source directly into Git's repository),
+or so ubiquitous that it is reasonable to expect that most developers will have
+the tools installed already.
+
+[[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
+changes. Alternatively, assuming we are vendoring the source into our repo, it
+must be simple enough that Git developers can feel comfortable making changes as
+needed to our version.
+
+In the comparison table below, "True" means that the framework seems to have
+active developers, that it is simple enough that Git developers can make changes
+to it, and that the project seems open to accepting external contributions (or
+that it is vendorable). "Partial" means that at least one of the above
+conditions holds.
+
+[[major-platform-support]]
+==== Major platform support
+
+At a bare minimum, unit-testing must work on Linux, MacOS, and Windows.
+
+In the comparison table below, "True" means that it works on all three major
+platforms with no issues. "Partial" means that there may be annoyances on one or
+more platforms, but it is still usable in principle.
+
+[[lazy-test-planning]]
+==== Lazy test planning
+
+TAP supports the notion of _test plans_, which communicate which test cases are
+expected to run, or which tests actually ran. This allows test harnesses to
+detect if the TAP output has been truncated, or if some tests were skipped due
+to errors or bugs.
+
+The test framework should handle creating plans at runtime, rather than
+requiring test developers to manually create plans, which leads to both human-
+and merge-errors.
+
+[[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.
+
+[[scheduling-re-running]]
+==== Scheduling / re-running
+
+The test harness scheduling should be configurable so that e.g. developers can
+choose to run slow tests first, or to run only tests that failed in a previous
+run.
+
+"True" means that the framework supports both features, "Partial" means it
+supports only one (assuming proper harness 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).
+Mocking allows test authors to provide a fake implementation of these objects
+for more convenient tests.
+
+[[signal-error-handling]]
+==== Signal & error handling
+
+The test framework must fail gracefully when test cases are themselves buggy or
+when they are interrupted by signals during runtime.
+
+[[coverage-reports]]
+==== Coverage reports
+
+It may be convenient to generate coverage reports when running unit tests
+(although it may be possible to accomplish this regardless of test framework /
+harness support).
+
+[[project-kloc]]
+==== Project KLOC
+
+WIP: The size of the project, in thousands of lines of code. All else being
+equal, we probably prefer a project with fewer LOC.
+
+[[adoption]]
+==== Adoption
+
+WIP: we prefer a more widely-used project. We'll need to figure out the best way
+to measure this.
+
+[[inline-tests]]
+==== Inline tests
+
+Can the tests live alongside production code in the same source files? This can
+be a useful reminder for developers to add new tests, and keep existing ones
+synced with new changes.
+
+=== Comparison
+
+[format="csv",options="header"]
+|=====
+Framework,"<<tap-support,TAP support>>","<<diagnostic-output,Diagnostic \
output>>","<<parallel-execution,Parallel \
execution>>","<<vendorable-or-ubiquitous,Vendorable or \
ubiquitous>>","<<maintainable-extensible,Maintainable / \
extensible>>","<<major-platform-support,Major platform \
support>>","<<lazy-test-planning,Lazy test \
planning>>","<<runtime--skippable-tests,Runtime- skippable \
tests>>","<<scheduling-re-running,Scheduling / re-running>>","<<mock-support,Mock \
support>>","<<signal-error-handling,Signal & error \
handling>>","<<coverage-reports,Coverage reports>>","<<project-kloc,Project \
KLOC>>","<<adoption,Adoption>>","<<inline-tests,Inline tests>>" \
+https://lore.kernel.org/git/c902a166-98ce-afba-93f2-ea6027557176@gmail.com/[Custom \
Git impl.],[lime-background]#True#,[lime-background]#True#,?,[lime-background]#True#,[ \
lime-background]#True#,[lime-background]#True#,[lime-background]#True#,?,?,[red-background]#False#,?,?,?,?,?
 +https://cmocka.org/[cmocka],[lime-background]#True#,[lime-background]#True#,?,[red-b \
ackground]#False#,[yellow-background]#Partial#,[yellow-background]#Partial#,?,?,?,[lime-background]#True#,?,?,?,?,?
 +https://libcheck.github.io/check/[Check],[lime-background]#True#,[lime-background]#T \
rue#,?,[red-background]#False#,[yellow-background]#Partial#,[lime-background]#True#,?,?,?,[red-background]#False#,?,?,?,?,?
 +https://github.com/rra/c-tap-harness/[C \
TAP],[lime-background]#True#,[red-background]#False#,?,[lime-background]#True#,[yellow \
-background]#Partial#,[yellow-background]#Partial#,?,?,?,[red-background]#False#,?,?,?,?,?
 +https://github.com/silentbicycle/greatest[Greatest],[yellow-background]#Partial#,?,? \
,[lime-background]#True#,[yellow-background]#Partial#,?,?,?,?,[red-background]#False#,?,?,?,?,?
 +https://github.com/Snaipe/Criterion[Criterion],[lime-background]#True#,?,?,[red-background]#False#,?,[lime-background]#True#,?,?,?,[red-background]#False#,?,?,?,?,?
 +https://github.com/zorgnax/libtap[libtap],[lime-background]#True#,?,?,?,?,?,?,?,?,?,?,?,?,?,?
 +https://www.kindahl.net/mytap/doc/index.html[MyTAP],[lime-background]#True#,?,?,?,?,?,?,?,?,?,?,?,?,?,?
 +https://nemequ.github.io/munit/[ \
ľnit],[red-background]#False#,-,-,-,-,-,-,-,-,-,-,-,-,-,- \
+https://github.com/google/cmockery[cmockery],[red-background]#False#,-,-,-,-,-,-,-,-,-,-,-,-,-,-
 +https://github.com/lpabon/cmockery2[cmockery2],[red-background]#False#,-,-,-,-,-,-,-,-,-,-,-,-,-,-
 +https://github.com/ThrowTheSwitch/Unity[Unity],[red-background]#False#,-,-,-,-,-,-,-,-,-,-,-,-,-,-
 +https://github.com/siu/minunit[minunit],[red-background]#False#,-,-,-,-,-,-,-,-,-,-,-,-,-,-
 +https://cunit.sourceforge.net/[CUnit],[red-background]#False#,-,-,-,-,-,-,-,-,-,-,-,-,-,-
 +|=====
+
+== Milestones
+
+* Settle on final framework
+* Add useful tests of library-like code
+* Integrate with Makefile
+* Integrate with CI
+* Integrate with
+  https://lore.kernel.org/git/20230502211454.1673000-1-calvinwan@google.com/[stdlib
+  work]
+* Run alongside regular `make test` target

base-commit: a9e066fa63149291a55f383cfa113d8bdbdaa6b3
-- 
2.41.0.255.g8b1d071c50-goog


[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic