All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: Toon Claes <toon@iotcl.com>, git@vger.kernel.org
Subject: Re: [PATCH v2 2/2] t0450: add allowlist for builtins with missing .adoc
Date: Tue, 12 Aug 2025 14:52:31 +0200	[thread overview]
Message-ID: <aJs5D2jPh8Uom96p@pks.im> (raw)
In-Reply-To: <xmqq7bzdfoxe.fsf@gitster.g>

On Fri, Aug 08, 2025 at 03:07:09PM -0700, Junio C Hamano wrote:
> Toon Claes <toon@iotcl.com> writes:
> 
> > Before we were silently skipping all builtins that don't have a matching
> > .adoc file. This is overly loose and might skip documentation files
> > when it shouldn't, for example when there was a typo in the filename.
> >
> > To ensure no new builtins are added without documentation, add an
> > allowlist: t0450/adoc-missing. In this file only builtin commands that
> > do *not* have a corresponding .adoc file shall be listed. If there is a
> > mismatch, fail the test. This should force future contributions to
> > either add an .adoc, or add the builtin name to the allowlist file.
> >
> > Signed-off-by: Toon Claes <toon@iotcl.com>
> > ---
> >  t/t0450-txt-doc-vs-help.sh | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> Forgot to add something?

Indeed. Toon is currently out of office, so I had a look at what it
takes. The below patch is what I ended up with -- note that I also had
to reverse the `grep` condition to set the prereq in the else branch.

Let me know whether you're fine with just squashing these changes in or
whether I shall send another version.

Thanks!

Patrick

diff --git a/t/t0450-txt-doc-vs-help.sh b/t/t0450-txt-doc-vs-help.sh
index da2d0af5b0..e12e18f97f 100755
--- a/t/t0450-txt-doc-vs-help.sh
+++ b/t/t0450-txt-doc-vs-help.sh
@@ -112,10 +112,19 @@ do
 	adoc="$(builtin_to_adoc "$builtin")" &&
 	preq="$(echo BUILTIN_ADOC_$builtin | tr '[:lower:]-' '[:upper:]_')" &&
 
-	if test -f "$adoc"
+	# If and only if *.adoc is missing, builtin shall be listed in t0450/adoc-missing.
+	if grep -q "^$builtin$" "$TEST_DIRECTORY"/t0450/adoc-missing
 	then
+		test_expect_success "$builtin appropriately marked as not having .adoc" '
+			! test -f "$adoc"
+		'
+	else
 		test_set_prereq "$preq"
-	fi &&
+
+		test_expect_success "$builtin appropriately marked as having .adoc" '
+			test -f "$adoc"
+		'
+	fi
 
 	# *.adoc output assertions
 	test_expect_success "$preq" "$builtin *.adoc SYNOPSIS has dashed labels" '
diff --git a/t/t0450/adoc-missing b/t/t0450/adoc-missing
new file mode 100644
index 0000000000..1ec9f8dcf3
--- /dev/null
+++ b/t/t0450/adoc-missing
@@ -0,0 +1,9 @@
+checkout--worker
+merge-ours
+merge-recursive
+merge-recursive-ours
+merge-recursive-theirs
+merge-subtree
+pickaxe
+submodule--helper
+upload-archive--writer

  reply	other threads:[~2025-08-12 12:52 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-04  7:30 [PATCH] t0450: fix test for out-of-tree builds Toon Claes
2025-08-04  7:56 ` Patrick Steinhardt
2025-08-05 12:11   ` Toon Claes
2025-08-08  9:59 ` [PATCH v2 0/2] Harden test t0450-txt-doc-vs-help Toon Claes
2025-08-08  9:59 ` [PATCH v2 1/2] t0450: fix test for out-of-tree builds Toon Claes
2025-08-08  9:59 ` [PATCH v2 2/2] t0450: add allowlist for builtins with missing .adoc Toon Claes
2025-08-08 22:07   ` Junio C Hamano
2025-08-12 12:52     ` Patrick Steinhardt [this message]
2025-08-28 23:12       ` Junio C Hamano
2025-09-02 20:57         ` Junio C Hamano
2025-09-03 11:26           ` Toon Claes
2025-09-03 18:46             ` Junio C Hamano

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=aJs5D2jPh8Uom96p@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=toon@iotcl.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.