On 2023.10.27 22:12, Christian Couder wrote: > On Tue, Oct 10, 2023 at 12:22 AM Josh Steadmon <steadmon@xxxxxxxxxx> wrote: > > > > 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. Fixed in v9, thanks. > > 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. > > I agree with that. > > > +== 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@xxxxxxxxx/[1]. > > Nit: Logically I would think that our opinion should come after the > comparison and be backed by it. I intended this to be a quick summary for those who don't want to read the whole doc. I clarified that and added a link to the selection rationale. > > +== 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. Thanks for the review!