|
| 1 | += Unit Testing |
| 2 | + |
| 3 | +In our current testing environment, we spend a significant amount of effort |
| 4 | +crafting end-to-end tests for error conditions that could easily be captured by |
| 5 | +unit tests (or we simply forgo some hard-to-setup and rare error conditions). |
| 6 | +Unit tests additionally provide stability to the codebase and can simplify |
| 7 | +debugging through isolation. Writing unit tests in pure C, rather than with our |
| 8 | +current shell/test-tool helper setup, simplifies test setup, simplifies passing |
| 9 | +data around (no shell-isms required), and reduces testing runtime by not |
| 10 | +spawning a separate process for every test invocation. |
| 11 | + |
| 12 | +We believe that a large body of unit tests, living alongside the existing test |
| 13 | +suite, will improve code quality for the Git project. |
| 14 | + |
| 15 | +== Definitions |
| 16 | + |
| 17 | +For the purposes of this document, we'll use *test framework* to refer to |
| 18 | +projects that support writing test cases and running tests within the context |
| 19 | +of a single executable. *Test harness* will refer to projects that manage |
| 20 | +running multiple executables (each of which may contain multiple test cases) and |
| 21 | +aggregating their results. |
| 22 | + |
| 23 | +In reality, these terms are not strictly defined, and many of the projects |
| 24 | +discussed below contain features from both categories. |
| 25 | + |
| 26 | +For now, we will evaluate projects solely on their framework features. Since we |
| 27 | +are relying on having TAP output (see below), we can assume that any framework |
| 28 | +can be made to work with a harness that we can choose later. |
| 29 | + |
| 30 | + |
| 31 | +== Summary |
| 32 | + |
| 33 | +We believe the best way forward is to implement a custom TAP framework for the |
| 34 | +Git project. We use a version of the framework originally proposed in |
| 35 | +https://lore.kernel.org/git/ [email protected]/[1]. |
| 36 | + |
| 37 | +See the <<framework-selection,Framework Selection>> section below for the |
| 38 | +rationale behind this decision. |
| 39 | + |
| 40 | + |
| 41 | +== Choosing a test harness |
| 42 | + |
| 43 | +During upstream discussion, it was occasionally noted that `prove` provides many |
| 44 | +convenient features, such as scheduling slower tests first, or re-running |
| 45 | +previously failed tests. |
| 46 | + |
| 47 | +While we already support the use of `prove` as a test harness for the shell |
| 48 | +tests, it is not strictly required. The t/Makefile allows running shell tests |
| 49 | +directly (though with interleaved output if parallelism is enabled). Git |
| 50 | +developers who wish to use `prove` as a more advanced harness can do so by |
| 51 | +setting DEFAULT_TEST_TARGET=prove in their config.mak. |
| 52 | + |
| 53 | +We will follow a similar approach for unit tests: by default the test |
| 54 | +executables will be run directly from the t/Makefile, but `prove` can be |
| 55 | +configured with DEFAULT_UNIT_TEST_TARGET=prove. |
| 56 | + |
| 57 | + |
| 58 | +[[framework-selection]] |
| 59 | +== Framework selection |
| 60 | + |
| 61 | +There are a variety of features we can use to rank the candidate frameworks, and |
| 62 | +those features have different priorities: |
| 63 | + |
| 64 | +* Critical features: we probably won't consider a framework without these |
| 65 | +** Can we legally / easily use the project? |
| 66 | +*** <<license,License>> |
| 67 | +*** <<vendorable-or-ubiquitous,Vendorable or ubiquitous>> |
| 68 | +*** <<maintainable-extensible,Maintainable / extensible>> |
| 69 | +*** <<major-platform-support,Major platform support>> |
| 70 | +** Does the project support our bare-minimum needs? |
| 71 | +*** <<tap-support,TAP support>> |
| 72 | +*** <<diagnostic-output,Diagnostic output>> |
| 73 | +*** <<runtime-skippable-tests,Runtime-skippable tests>> |
| 74 | +* Nice-to-have features: |
| 75 | +** <<parallel-execution,Parallel execution>> |
| 76 | +** <<mock-support,Mock support>> |
| 77 | +** <<signal-error-handling,Signal & error-handling>> |
| 78 | +* Tie-breaker stats |
| 79 | +** <<project-kloc,Project KLOC>> |
| 80 | +** <<adoption,Adoption>> |
| 81 | + |
| 82 | +[[license]] |
| 83 | +=== License |
| 84 | + |
| 85 | +We must be able to legally use the framework in connection with Git. As Git is |
| 86 | +licensed only under GPLv2, we must eliminate any LGPLv3, GPLv3, or Apache 2.0 |
| 87 | +projects. |
| 88 | + |
| 89 | +[[vendorable-or-ubiquitous]] |
| 90 | +=== Vendorable or ubiquitous |
| 91 | + |
| 92 | +We want to avoid forcing Git developers to install new tools just to run unit |
| 93 | +tests. Any prospective frameworks and harnesses must either be vendorable |
| 94 | +(meaning, we can copy their source directly into Git's repository), or so |
| 95 | +ubiquitous that it is reasonable to expect that most developers will have the |
| 96 | +tools installed already. |
| 97 | + |
| 98 | +[[maintainable-extensible]] |
| 99 | +=== Maintainable / extensible |
| 100 | + |
| 101 | +It is unlikely that any pre-existing project perfectly fits our needs, so any |
| 102 | +project we select will need to be actively maintained and open to accepting |
| 103 | +changes. Alternatively, assuming we are vendoring the source into our repo, it |
| 104 | +must be simple enough that Git developers can feel comfortable making changes as |
| 105 | +needed to our version. |
| 106 | + |
| 107 | +In the comparison table below, "True" means that the framework seems to have |
| 108 | +active developers, that it is simple enough that Git developers can make changes |
| 109 | +to it, and that the project seems open to accepting external contributions (or |
| 110 | +that it is vendorable). "Partial" means that at least one of the above |
| 111 | +conditions holds. |
| 112 | + |
| 113 | +[[major-platform-support]] |
| 114 | +=== Major platform support |
| 115 | + |
| 116 | +At a bare minimum, unit-testing must work on Linux, MacOS, and Windows. |
| 117 | + |
| 118 | +In the comparison table below, "True" means that it works on all three major |
| 119 | +platforms with no issues. "Partial" means that there may be annoyances on one or |
| 120 | +more platforms, but it is still usable in principle. |
| 121 | + |
| 122 | +[[tap-support]] |
| 123 | +=== TAP support |
| 124 | + |
| 125 | +The https://testanything.org/[Test Anything Protocol] is a text-based interface |
| 126 | +that allows tests to communicate with a test harness. It is already used by |
| 127 | +Git's integration test suite. Supporting TAP output is a mandatory feature for |
| 128 | +any prospective test framework. |
| 129 | + |
| 130 | +In the comparison table below, "True" means this is natively supported. |
| 131 | +"Partial" means TAP output must be generated by post-processing the native |
| 132 | +output. |
| 133 | + |
| 134 | +Frameworks that do not have at least Partial support will not be evaluated |
| 135 | +further. |
| 136 | + |
| 137 | +[[diagnostic-output]] |
| 138 | +=== Diagnostic output |
| 139 | + |
| 140 | +When a test case fails, the framework must generate enough diagnostic output to |
| 141 | +help developers find the appropriate test case in source code in order to debug |
| 142 | +the failure. |
| 143 | + |
| 144 | +[[runtime-skippable-tests]] |
| 145 | +=== Runtime-skippable tests |
| 146 | + |
| 147 | +Test authors may wish to skip certain test cases based on runtime circumstances, |
| 148 | +so the framework should support this. |
| 149 | + |
| 150 | +[[parallel-execution]] |
| 151 | +=== Parallel execution |
| 152 | + |
| 153 | +Ideally, we will build up a significant collection of unit test cases, most |
| 154 | +likely split across multiple executables. It will be necessary to run these |
| 155 | +tests in parallel to enable fast develop-test-debug cycles. |
| 156 | + |
| 157 | +In the comparison table below, "True" means that individual test cases within a |
| 158 | +single test executable can be run in parallel. We assume that executable-level |
| 159 | +parallelism can be handled by the test harness. |
| 160 | + |
| 161 | +[[mock-support]] |
| 162 | +=== Mock support |
| 163 | + |
| 164 | +Unit test authors may wish to test code that interacts with objects that may be |
| 165 | +inconvenient to handle in a test (e.g. interacting with a network service). |
| 166 | +Mocking allows test authors to provide a fake implementation of these objects |
| 167 | +for more convenient tests. |
| 168 | + |
| 169 | +[[signal-error-handling]] |
| 170 | +=== Signal & error handling |
| 171 | + |
| 172 | +The test framework should fail gracefully when test cases are themselves buggy |
| 173 | +or when they are interrupted by signals during runtime. |
| 174 | + |
| 175 | +[[project-kloc]] |
| 176 | +=== Project KLOC |
| 177 | + |
| 178 | +The size of the project, in thousands of lines of code as measured by |
| 179 | +https://dwheeler.com/sloccount/[sloccount] (rounded up to the next multiple of |
| 180 | +1,000). As a tie-breaker, we probably prefer a project with fewer LOC. |
| 181 | + |
| 182 | +[[adoption]] |
| 183 | +=== Adoption |
| 184 | + |
| 185 | +As a tie-breaker, we prefer a more widely-used project. We use the number of |
| 186 | +GitHub / GitLab stars to estimate this. |
| 187 | + |
| 188 | + |
| 189 | +=== Comparison |
| 190 | + |
| 191 | +:true: [lime-background]#True# |
| 192 | +:false: [red-background]#False# |
| 193 | +:partial: [yellow-background]#Partial# |
| 194 | + |
| 195 | +:gpl: [lime-background]#GPL v2# |
| 196 | +:isc: [lime-background]#ISC# |
| 197 | +:mit: [lime-background]#MIT# |
| 198 | +:expat: [lime-background]#Expat# |
| 199 | +:lgpl: [lime-background]#LGPL v2.1# |
| 200 | + |
| 201 | +:custom-impl: https://lore.kernel.org/git/ [email protected]/[Custom Git impl.] |
| 202 | +:greatest: https://github.com/silentbicycle/greatest[Greatest] |
| 203 | +:criterion: https://github.com/Snaipe/Criterion[Criterion] |
| 204 | +:c-tap: https://github.com/rra/c-tap-harness/[C TAP] |
| 205 | +:check: https://libcheck.github.io/check/[Check] |
| 206 | + |
| 207 | +[format="csv",options="header",width="33%",subs="specialcharacters,attributes,quotes,macros"] |
| 208 | +|===== |
| 209 | +Framework,"<<license,License>>","<<vendorable-or-ubiquitous,Vendorable or ubiquitous>>","<<maintainable-extensible,Maintainable / extensible>>","<<major-platform-support,Major platform support>>","<<tap-support,TAP support>>","<<diagnostic-output,Diagnostic output>>","<<runtime--skippable-tests,Runtime- skippable tests>>","<<parallel-execution,Parallel execution>>","<<mock-support,Mock support>>","<<signal-error-handling,Signal & error handling>>","<<project-kloc,Project KLOC>>","<<adoption,Adoption>>" |
| 210 | +{custom-impl},{gpl},{true},{true},{true},{true},{true},{true},{false},{false},{false},1,0 |
| 211 | +{greatest},{isc},{true},{partial},{true},{partial},{true},{true},{false},{false},{false},3,1400 |
| 212 | +{criterion},{mit},{false},{partial},{true},{true},{true},{true},{true},{false},{true},19,1800 |
| 213 | +{c-tap},{expat},{true},{partial},{partial},{true},{false},{true},{false},{false},{false},4,33 |
| 214 | +{check},{lgpl},{false},{partial},{true},{true},{true},{false},{false},{false},{true},17,973 |
| 215 | +|===== |
| 216 | + |
| 217 | +=== Additional framework candidates |
| 218 | + |
| 219 | +Several suggested frameworks have been eliminated from consideration: |
| 220 | + |
| 221 | +* Incompatible licenses: |
| 222 | +** https://github.com/zorgnax/libtap[libtap] (LGPL v3) |
| 223 | +** https://cmocka.org/[cmocka] (Apache 2.0) |
| 224 | +* Missing source: https://www.kindahl.net/mytap/doc/index.html[MyTap] |
| 225 | +* No TAP support: |
| 226 | +** https://nemequ.github.io/munit/[µnit] |
| 227 | +** https://github.com/google/cmockery[cmockery] |
| 228 | +** https://github.com/lpabon/cmockery2[cmockery2] |
| 229 | +** https://github.com/ThrowTheSwitch/Unity[Unity] |
| 230 | +** https://github.com/siu/minunit[minunit] |
| 231 | +** https://cunit.sourceforge.net/[CUnit] |
| 232 | + |
| 233 | + |
| 234 | +== Milestones |
| 235 | + |
| 236 | +* Add useful tests of library-like code |
| 237 | +* Integrate with |
| 238 | + https://lore.kernel.org/git/ [email protected]/[stdlib |
| 239 | + work] |
| 240 | +* Run alongside regular `make test` target |
0 commit comments