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
prev parent 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).