git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: [PATCH 2/2] t/Makefile: get UNIT_TESTS list from C sources
Date: Sun, 28 Jan 2024 22:19:33 -0500	[thread overview]
Message-ID: <20240129031933.GB2433899@coredump.intra.peff.net> (raw)
In-Reply-To: <20240129031540.GA2433764@coredump.intra.peff.net>

We decide on the set of unit tests to run by asking make to expand the
wildcard "t/unit-tests/bin/*". One unfortunate outcome of this is that
we'll run anything in that directory, even if it is leftover cruft from
a previous build. This isn't _quite_ as bad as it sounds, since in
theory the unit test executables are self-contained (so if they passed
before, they'll pass even though they now have nothing to do with the
checked out version of Git). But at the very least it's wasteful, and if
they _do_ fail it can be quite confusing to understand why they are
being run at all.

This wildcarding presumably came from our handling of the regular
shell-script tests, which match "t[0-9][0-9][0-9][0-9]-*.sh".  But the
difference there is that those are actual tracked files. So if you
checkout a different commit, they'll go away. Whereas the contents of
unit-tests/bin are ignored (so not only do they stick around, but you
are not even warned of the stale files via "git status").

This patch fixes the situation by looking for the actual unit-test
source files and then massaging those names into the final executable
names. This has two additional benefits:

  1. It will notice if we failed to build one or more unit-tests for
     some reason (wheras the current code just runs whatever made it to
     the bin/ directory).

  2. The wildcard should avoid other build cruft, like the pdb files we
     worked around in 0df903d402 (unit-tests: do not mistake `.pdb`
     files for being executable, 2023-09-25).

Our new wildcard does make an assumption that unit tests are build from
C sources. It would be a bit cleaner if we consulted UNIT_TEST_PROGRAMS
from the top-level Makefile. But doing so is tricky unless we reorganize
that Makefile to split the source file lists into include-able subfiles.
That might be worth doing in general, but in the meantime, the
assumptions made by the wildcard here seems reasonable.

Signed-off-by: Jeff King <peff@peff.net>
---
I of course hit this when moving between "next" and "master" for an
up-and-coming unit-test file which sometimes failed.

 t/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/t/Makefile b/t/Makefile
index b7a6fefe28..c5c6e2ef6b 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -42,7 +42,9 @@ TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))
 TINTEROP = $(sort $(wildcard interop/i[0-9][0-9][0-9][0-9]-*.sh))
 CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))
 CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl
-UNIT_TESTS = $(sort $(filter-out %.pdb unit-tests/bin/t-basic%,$(wildcard unit-tests/bin/t-*)))
+UNIT_TEST_SOURCES = $(wildcard unit-tests/t-*.c)
+UNIT_TEST_PROGRAMS = $(patsubst unit-tests/%.c,unit-tests/bin/%,$(UNIT_TEST_SOURCES))
+UNIT_TESTS = $(sort $(filter-out unit-tests/bin/t-basic%,$(UNIT_TEST_PROGRAMS)))
 
 # `test-chainlint` (which is a dependency of `test-lint`, `test` and `prove`)
 # checks all tests in all scripts via a single invocation, so tell individual
-- 
2.43.0.797.g29b680fc68

  parent reply	other threads:[~2024-01-29  3:19 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-29  3:15 [PATCH 0/2] some unit-test Makefile polishing Jeff King
2024-01-29  3:18 ` [PATCH 1/2] Makefile: use order-only prereq for UNIT_TEST_BIN Jeff King
2024-01-29 20:22   ` SZEDER Gábor
2024-01-29 22:06     ` Junio C Hamano
2024-01-30  5:21     ` Jeff King
2024-01-29  3:19 ` Jeff King [this message]
2024-01-29 11:26   ` [PATCH 2/2] t/Makefile: get UNIT_TESTS list from C sources Patrick Steinhardt
2024-01-29 17:49     ` Jeff King
2024-01-29 21:31       ` Adam Dinwoodie
2024-01-30  0:27         ` Junio C Hamano
2024-01-30  5:25           ` Jeff King
2024-01-31 19:13           ` Adam Dinwoodie
2024-01-30  5:23         ` Jeff King
2024-01-29 21:51   ` Junio C Hamano
2024-01-30  5:37 ` [PATCH v2 0/3] some unit-test Makefile polishing Jeff King
2024-01-30  5:37   ` [PATCH v2 1/3] Makefile: use mkdir_p_parent_template for UNIT_TEST_BIN Jeff King
2024-01-30  5:38   ` [PATCH v2 2/3] Makefile: remove UNIT_TEST_BIN directory with "make clean" Jeff King
2024-01-30  5:40   ` [PATCH v2 3/3] t/Makefile: get UNIT_TESTS list from C sources Jeff King
2024-01-31 22:58     ` Junio C Hamano
2024-02-01 10:50     ` Phillip Wood
2024-02-02  1:20   ` [PATCH v2 0/3] some unit-test Makefile polishing Junio C Hamano
2024-02-02 23:52     ` Johannes Schindelin
2024-02-03  1:32       ` Junio C Hamano
2024-02-04  4:41         ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240129031933.GB2433899@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).