git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Fernando Ramos" <greenfoo@u92.eu>,
	"Matthias Aßhauer" <mha1993@live.de>,
	"Lars Schneider" <larsxschneider@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH] tests: do roundtrip builtin doc & sanity checking
Date: Sun, 27 Mar 2022 15:18:20 +0200	[thread overview]
Message-ID: <patch-1.1-4a5d219dfe7-20220327T131751Z-avarab@gmail.com> (raw)
In-Reply-To: <xmqq4k3k6xw3.fsf@gitster.g>

Replace the part of "make check-docs" first added in [1] and the
entirety of "make check-builtins" first added in [2] with checks that
do checking against --list-cmds=main (see [3]) and that our generated
"man" docs instead.

The "check-docs" part of this was mostly broken anyway, in that we
were ignoring the exit code. Since b6d8887d3d6 (documentation: add
documentation for 'git version', 2021-09-14) we started emitting:

	$ make -s check-docs
	removed but documented: git-version

But as noted in [4] the check didn't fail.

By checking that we get the man page when running "git $cmd --help" we
can be sure that we don't miss anything. To do this add a
GIT_TEST_MANPATH, as we don't want to append to the manpath as we'd
pick up the MANPATH on the system.

There is no replacement provided for the "$base is builtin but
$builtin.$sfx still exists" part of the "check-builtins" check being
removed here. Back when it was added various commands were being
migrated to C at a rapid rate, now we have just a few stragglers
in *.sh and *.perl. It's unlikely that we'd forget to remove the old
code.

1. 8c989ec5288 (Makefile: $(MAKE) check-docs, 2006-04-13)
2. c74390e4a1d (cherry is built-in, do not ship git-cherry.sh,
   2006-11-05)
3. 6bb2dc0b947 (completion: implement and use --list-cmds=main,others,
   2018-05-20)
4. https://lore.kernel.org/git/87o88i2keu.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile     |  3 --
 Makefile                   | 39 +----------------
 builtin/help.c             | 12 +++++-
 check-builtins.sh          | 34 ---------------
 ci/install-dependencies.sh |  3 ++
 t/t0012-help.sh            | 86 +++++++++++++++++++++++++++++++++++++-
 6 files changed, 101 insertions(+), 76 deletions(-)
 delete mode 100755 check-builtins.sh

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 1eb9192dae8..a2c6ef479e0 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -431,9 +431,6 @@ require-htmlrepo::
 quick-install-html: require-htmlrepo
 	'$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) $(DESTDIR)$(htmldir) $(GIT_MAN_REF)
 
-print-man1:
-	@for i in $(MAN1_TXT); do echo $$i; done
-
 ## Lint: gitlink
 LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))
 $(LINT_DOCS_GITLINK): lint-gitlink.perl
diff --git a/Makefile b/Makefile
index e8aba291d7f..c5b83f80a3a 100644
--- a/Makefile
+++ b/Makefile
@@ -3331,46 +3331,11 @@ ALL_COMMANDS += gitweb
 .PHONY: check-docs
 check-docs::
 	$(MAKE) -C Documentation lint-docs
-	@(for v in $(patsubst %$X,%,$(ALL_COMMANDS)); \
-	do \
-		case "$$v" in \
-		git-merge-octopus | git-merge-ours | git-merge-recursive | \
-		git-merge-resolve | git-merge-subtree | \
-		git-fsck-objects | git-init-db | \
-		git-remote-* | git-stage | git-legacy-* | \
-		git-?*--?* ) continue ;; \
-		esac ; \
-		test -f "Documentation/$$v.txt" || \
-		echo "no doc: $$v"; \
-		sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt | \
-		grep -q "^$$v[ 	]" || \
-		case "$$v" in \
-		git) ;; \
-		*) echo "no link: $$v";; \
-		esac ; \
-	done; \
-	( \
-		sed -e '1,/^### command list/d' \
-		    -e '/^#/d' \
-		    -e '/guide$$/d' \
-		    -e 's/[ 	].*//' \
-		    -e 's/^/listed /' command-list.txt; \
-		$(MAKE) -C Documentation print-man1 | \
-		grep '\.txt$$' | \
-		sed -e 's|^|documented |' \
-		    -e 's/\.txt//'; \
-	) | while read how cmd; \
-	do \
-		case " $(patsubst %$X,%,$(ALL_COMMANDS) $(BUILT_INS) $(EXCLUDED_PROGRAMS)) " in \
-		*" $$cmd "*)	;; \
-		*) echo "removed but $$how: $$cmd" ;; \
-		esac; \
-	done ) | sort
 
 ### Make sure built-ins do not have dups and listed in git.c
 #
-check-builtins::
-	./check-builtins.sh
+check-builtins:: all man
+	$(MAKE) -C t T=t0012-help.sh
 
 ### Test suite coverage testing
 #
diff --git a/builtin/help.c b/builtin/help.c
index 222f994f863..775eee76c05 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -434,7 +434,7 @@ static const char *cmd_to_page(const char *git_cmd)
 		return xstrfmt("git%s", git_cmd);
 }
 
-static void setup_man_path(void)
+static void setup_real_man_path(void)
 {
 	struct strbuf new_path = STRBUF_INIT;
 	const char *old_path = getenv("MANPATH");
@@ -455,6 +455,16 @@ static void setup_man_path(void)
 	strbuf_release(&new_path);
 }
 
+static void setup_man_path(void)
+{
+	const char *test_manpath = getenv("GIT_TEST_MANPATH");
+
+	if (test_manpath)
+		xsetenv("MANPATH", test_manpath, 1);
+	else
+		setup_real_man_path();
+}
+
 static void exec_viewer(const char *name, const char *page)
 {
 	const char *info = get_man_viewer_info(name);
diff --git a/check-builtins.sh b/check-builtins.sh
deleted file mode 100755
index a0aaf3a3473..00000000000
--- a/check-builtins.sh
+++ /dev/null
@@ -1,34 +0,0 @@
-#!/bin/sh
-
-{
-	cat <<\EOF
-sayIt:
-	$(foreach b,$(BUILT_INS),echo XXX $(b:$X=) YYY;)
-EOF
-	cat Makefile
-} |
-make -f - sayIt 2>/dev/null |
-sed -n -e 's/.*XXX \(.*\) YYY.*/\1/p' |
-sort |
-{
-    bad=0
-    while read builtin
-    do
-	base=$(expr "$builtin" : 'git-\(.*\)')
-	x=$(sed -ne 's/.*{ "'$base'", \(cmd_[^, ]*\).*/'$base'	\1/p' git.c)
-	if test -z "$x"
-	then
-		echo "$base is builtin but not listed in git.c command list"
-		bad=1
-	fi
-	for sfx in sh perl py
-	do
-		if test -f "$builtin.$sfx"
-		then
-			echo "$base is builtin but $builtin.$sfx still exists"
-			bad=1
-		fi
-	done
-    done
-    exit $bad
-}
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index dbcebad2fb2..1df7c908db0 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -71,6 +71,9 @@ Documentation)
 
 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
 	sudo gem install --version 1.5.8 asciidoctor
+
+	sudo apt-get -q -y install libcurl4-openssl-dev libssl-dev \
+		libexpat-dev gettext make
 	;;
 linux-gcc-default)
 	sudo apt-get -q update
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 6c3e1f7159d..522eb1b492d 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -234,7 +234,9 @@ test_expect_success "'git help -g' section spacing" '
 
 test_expect_success 'generate builtin list' '
 	mkdir -p sub &&
-	git --list-cmds=builtins >builtins
+	git --list-cmds=builtins >builtins &&
+	uniq builtins >builtins.uniq &&
+	test_cmp builtins builtins.uniq
 '
 
 while read builtin
@@ -249,4 +251,86 @@ do
 	'
 done <builtins
 
+
+symlink_test_manpath () {
+	local doc="$GIT_BUILD_DIR"/Documentation &&
+	test_path_exists "$doc"/git-add.1 &&
+	test_when_finished "rm -f man1" &&
+	ln -s "$doc" man1
+}
+
+# Use "git-add" as a guinea pig, and check the basic sanity of the
+# output.
+test_lazy_prereq HAVE_BUILT_DOCS '
+	symlink_test_manpath &&
+	test_when_finished "rm -f man.txt" &&
+	GIT_TEST_MANPATH="$PWD" git add --help >man.txt &&
+	grep GIT-ADD man.txt &&
+	grep ^SYNOPSIS man.txt
+'
+
+is_documented () {
+	cat >undocumented <<-\EOF
+	add--interactive
+	bisect--helper
+	checkout--worker
+	difftool--helper
+	env--helper
+	merge-octopus
+	merge-ours
+	merge-recursive
+	merge-recursive-ours
+	merge-recursive-theirs
+	merge-resolve
+	merge-subtree
+	pickaxe
+	remote-ftp
+	remote-ftps
+	remote-http
+	remote-https
+	submodule--helper
+	upload-archive--writer
+	EOF
+	! grep -q "^$1$" undocumented
+}
+
+test_undocumented () {
+	cmd=$1 &&
+	test_expect_success HAVE_BUILT_DOCS "$cmd does not have --help documentation" '
+		symlink_test_manpath &&
+		test_when_finished "rm -f man.txt" &&
+		test_must_fail env GIT_TEST_MANPATH="$PWD" git $cmd --help >man.txt
+	'
+}
+
+test_documented () {
+	cmd=$1 &&
+	test_expect_success HAVE_BUILT_DOCS "$cmd can handle --help" '
+		symlink_test_manpath &&
+		test_when_finished "rm -f man.txt" &&
+		GIT_TEST_MANPATH="$PWD" git $cmd --help >man.txt &&
+		grep "git-$cmd" man.txt
+	'
+}
+
+test_expect_success 'generate main list' '
+	mkdir -p sub &&
+	git --list-cmds=main >main
+'
+
+while read cmd
+do
+	case "$cmd" in
+	*.sh|*.perl|*.py)
+		    continue
+		    ;;
+	esac &&
+	if is_documented "$cmd"
+	then
+		test_documented "$cmd"
+	else
+		test_undocumented "$cmd"
+	fi
+done <main
+
 test_done
-- 
2.35.1.1535.gc79a163ff4c


      reply	other threads:[~2022-03-27 13:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-24  3:13 What's cooking in git.git (Mar 2022, #05; Wed, 23) Junio C Hamano
2022-03-25 13:38 ` ab/commit-plug-leaks (was: What's cooking in git.git (Mar 2022, #05; Wed, 23)) Ævar Arnfjörð Bjarmason
2022-03-26 16:05 ` fr/vimdiff-layout " Ævar Arnfjörð Bjarmason
2022-03-26 22:55   ` Fernando Ramos
2022-03-27  0:29     ` fr/vimdiff-layout Junio C Hamano
2022-03-27 13:18       ` Ævar Arnfjörð Bjarmason [this message]

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=patch-1.1-4a5d219dfe7-20220327T131751Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=greenfoo@u92.eu \
    --cc=larsxschneider@gmail.com \
    --cc=mha1993@live.de \
    /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).