From patchwork Thu Aug 17 18:37:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Josh Steadmon X-Patchwork-Id: 13356875 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4F46C3DA6C for ; Thu, 17 Aug 2023 18:38:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1354487AbjHQSiF (ORCPT ); Thu, 17 Aug 2023 14:38:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1354474AbjHQShc (ORCPT ); Thu, 17 Aug 2023 14:37:32 -0400 Received: from mail-yw1-x114a.google.com (mail-yw1-x114a.google.com [IPv6:2607:f8b0:4864:20::114a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0DEB726BB for ; Thu, 17 Aug 2023 11:37:30 -0700 (PDT) Received: by mail-yw1-x114a.google.com with SMTP id 00721157ae682-58d799aa369so1778317b3.0 for ; Thu, 17 Aug 2023 11:37:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1692297449; x=1692902249; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=Jv1CYGCQ/GYDbNU7BzaVeMqNGjml1pGVKJVwlO2E7Zg=; b=hF/vYOUsxHgxLfPNgXmCTTGmduPh/YJ3TTqJljId6vToPZnpKJBj71CIFOS8qFEH7B 3aB/9s/t2AWywjjneKEOGmpu2YW8momQKls7rtLF7DKti2lYKrBWYWglRQzrALLtKdCy T0jpRgdM7XrnuzVPFDLu0dyM8QtHh5yxZvNpAQweDhO+VBHmhGsQdMKCe7jp9f8ha1Vx Ddrg0BBVikoLx0+R6bwcLA18Hhpe6TWx2CFExYJRgBOlPULOtusuNGVsRJfulAKNGtij xdkVZMyZrrJTftnGmPF+vFCmomT8n56h0zc5N1UjLd+UXpNKw3xUiaFKiEXWjiFc9uG2 aaaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692297449; x=1692902249; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=Jv1CYGCQ/GYDbNU7BzaVeMqNGjml1pGVKJVwlO2E7Zg=; b=Eu10lsJKo4GTokPzRiEbOM6lpxopKRP+3TbxmPen18vPaxyLlRu233yRVZZTpmMmHx vKzqR2vWXwxoLBVbA/F8kWIG2qEIg+YEt1kz0K2qcXQaDKbqnTUnSQC0tVqhkMwlgu1J V51cU3jFZFGLKGvZyeM6avh9DXyaWKQjhcgImC+beENpH2dsDd1mYB8nvuOhpB+u+duU M42pIii18ICUob9t/FEnmLLUi1aZUXevf+YEA2QJaO6Tc5CUXlVIjrxxmZY6A6rz181F rj2E59fPnTgLkNOEY8RdeqdXSolQs6biHyQlyldd/DG+xOYTNbIBilmY1gRMORc9EXRm yGng== X-Gm-Message-State: AOJu0Yz1rB6ZD/UDRaZCoVmV5DpQedISjPo7+tSUEg/JyVd9MUJaOGlW 1lCIozzM5SVscf5mwtym21PBmeJERUpKPFqOYyBXQSjdTNNrLkjDvsXAYAXXMSX10nArUdZtBOU 7jIGFI0YnxMif+6tGCGKScojo9eoARgKZbos0+S1QGAf+Tb4GDOstSE+agiOPI6A= X-Google-Smtp-Source: AGHT+IG/ZfMzLwE+6fnGn7M65rKsVp7oRIDmgOGq1yLnPNRKYr5KlMjkSyB0ysra2Otf4b78Eq2SzfJfmjI/ig== X-Received: from lunarfall.svl.corp.google.com ([2620:15c:2d3:204:e60a:f80e:4b7a:6509]) (user=steadmon job=sendgmr) by 2002:a81:ac4e:0:b0:583:a3c1:6b5a with SMTP id z14-20020a81ac4e000000b00583a3c16b5amr3068ywj.4.1692297449297; Thu, 17 Aug 2023 11:37:29 -0700 (PDT) Date: Thu, 17 Aug 2023 11:37:21 -0700 In-Reply-To: Mime-Version: 1.0 References: <0169ce6fb9ccafc089b74ae406db0d1a8ff8ac65.1688165272.git.steadmon@google.com> X-Mailer: git-send-email 2.42.0.rc1.204.g551eb34607-goog Message-ID: <81c5148a1267b8f9ce432a950340f0fa16b4d773.1692297001.git.steadmon@google.com> Subject: [PATCH v7 1/3] unit tests: Add a project plan document From: Josh Steadmon To: git@vger.kernel.org Cc: linusa@google.com, calvinwan@google.com, phillip.wood123@gmail.com, gitster@pobox.com, rsbecker@nexbridge.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org 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. Co-authored-by: Calvin Wan Signed-off-by: Calvin Wan Signed-off-by: Josh Steadmon --- Documentation/Makefile | 1 + Documentation/technical/unit-tests.txt | 220 +++++++++++++++++++++++++ 2 files changed, 221 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..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. + +== 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. + +For now, we will evaluate projects solely on their framework features. Since we +are relying on having TAP output (see below), we can assume that any framework +can be made to work with a harness that we can choose later. + + +== 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@gmail.com/[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: + +* Critical features: we probably won't consider a framework without these +** Can we legally / easily use the project? +*** <> +*** <> +*** <> +*** <> +** Does the project support our bare-minimum needs? +*** <> +*** <> +*** <> +* Nice-to-have features: +** <> +** <> +** <> +* Tie-breaker stats +** <> +** <> + +[[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 + +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 +(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. + +[[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. + +[[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 + +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. We assume that executable-level +parallelism can be handled by the test harness. + +[[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 should fail gracefully when test cases are themselves buggy +or when they are interrupted by signals during runtime. + +[[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 + +As a tie-breaker, we prefer a more widely-used project. We use the number of +GitHub / GitLab stars to estimate this. + + +=== Comparison + +[format="csv",options="header",width="33%"] +|===== +Framework,"<>","<>","<>","<>","<>","<>","<>","<>","<>","<>","<>","<>" +https://lore.kernel.org/git/c902a166-98ce-afba-93f2-ea6027557176@gmail.com/[Custom Git impl.],[lime-background]#GPL v2#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[red-background]#False#,[red-background]#False#,[red-background]#False#,1,0 +https://github.com/silentbicycle/greatest[Greatest],[lime-background]#ISC#,[lime-background]#True#,[yellow-background]#Partial#,[lime-background]#True#,[yellow-background]#Partial#,[lime-background]#True#,[lime-background]#True#,[red-background]#False#,[red-background]#False#,[red-background]#False#,3,1400 +https://github.com/Snaipe/Criterion[Criterion],[lime-background]#MIT#,[red-background]#False#,[yellow-background]#Partial#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[red-background]#False#,[lime-background]#True#,19,1800 +https://github.com/rra/c-tap-harness/[C TAP],[lime-background]#Expat#,[lime-background]#True#,[yellow-background]#Partial#,[yellow-background]#Partial#,[lime-background]#True#,[red-background]#False#,[lime-background]#True#,[red-background]#False#,[red-background]#False#,[red-background]#False#,4,33 +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 +|===== + +=== Additional framework candidates + +Several suggested frameworks have been eliminated from consideration: + +* Incompatible licenses: +** https://github.com/zorgnax/libtap[libtap] (LGPL v3) +** https://cmocka.org/[cmocka] (Apache 2.0) +* Missing source: https://www.kindahl.net/mytap/doc/index.html[MyTap] +* No TAP support: +** https://nemequ.github.io/munit/[µnit] +** https://github.com/google/cmockery[cmockery] +** https://github.com/lpabon/cmockery2[cmockery2] +** https://github.com/ThrowTheSwitch/Unity[Unity] +** https://github.com/siu/minunit[minunit] +** https://cunit.sourceforge.net/[CUnit] + + +== Milestones + +* Add useful tests of library-like code +* Integrate with + https://lore.kernel.org/git/20230502211454.1673000-1-calvinwan@google.com/[stdlib + work] +* Run alongside regular `make test` target