Tidy the test comments and shell portability

20 files changed, 310 insertions, 134 deletions

diff --git a/tests/Makefile b/tests/Makefile
index e159487..1047de5 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -1,17 +1,29 @@
+# Runs the cgit test suite. Every t[0-9][0-9][0-9][0-9]-*.sh script beside this
+# file is a self-contained test built on the test library in the bundled Git
+# tree, so all that is needed here is a shell to run each one under and a way
+# to pass extra options along. The top-level Makefile reaches this through its
+# test target, once cgit and the Git tree it links against have been built.
+
+# SHELL_PATH comes from Git's platform detection, and cgit.conf may override it,
+# so both are read before anything below uses it.
 include ../vendor/git/config.mak.uname
 -include ../cgit.conf
 
 SHELL_PATH ?= $(SHELL)
+
+# Escaped for the single quotes that the recipe below puts around the path.
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 
-T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
+TESTS = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
 
-all: $(T)
+all: $(TESTS)
 
-$(T):
+$(TESTS):
 	@'$(SHELL_PATH_SQ)' $@ $(CGIT_TEST_OPTS)
 
 clean:
 	$(RM) -r 'trash directory'.* test-results
 
-.PHONY: all $(T) clean
+# Each test is named after a file that already exists, so without this make
+# would decide there is nothing to do and run none of them.
+.PHONY: all $(TESTS) clean
diff --git a/tests/setup.sh b/tests/setup.sh
index 39819e4..93a663a 100755
--- a/tests/setup.sh
+++ b/tests/setup.sh
@@ -1,24 +1,15 @@
-# This file should be sourced by all test-scripts
-#
-# Main functions:
-#   prepare_tests(description) - setup for testing, i.e. create repos+config
-#   run_test(description, script) - run one test, i.e. eval script
-#
-# Helper functions
-#   cgit_query(querystring) - call cgit with the specified querystring
-#   cgit_url(url) - call cgit with the specified virtual url
-#
-# Example script:
-#
-# . setup.sh
-# prepare_tests "html validation"
-# run_test 'repo index' 'cgit_url "/" | tidy -e'
-# run_test 'repo summary' 'cgit_url "/foo" | tidy -e'
+# Shared groundwork sourced by every test script. It builds a handful of small
+# repositories under repos/ and writes a cgitrc pointing at them, then offers
+# the helpers that ask cgit for a page so a test only has to look at what comes
+# back. Sourcing it also brings in the test library from the bundled Git tree,
+# which is where test_expect_success and the trash directory come from. Set
+# CGIT_TEST_NO_CREATE_REPOS to get the helpers without paying for the fixtures.
 
-# We don't want to run Git commands through Valgrind, so we filter out the
-# --valgrind option here and handle it ourselves.  We copy the arguments
-# assuming that none contain a newline, although other whitespace is
-# preserved.
+# The Git test library would run every Git command under Valgrind if it saw
+# --valgrind, and only cgit itself is worth watching, so the option is taken out
+# here and acted on further down. The arguments are carried across as a newline
+# separated list, which keeps any other whitespace in them intact but assumes
+# that none holds a newline of its own.
 LF='
 '
 test_argv=
@@ -44,10 +35,13 @@ IFS=$OLDIFS
 
 : ${TEST_DIRECTORY=$(pwd)/../vendor/git/t}
 : ${TEST_OUTPUT_DIRECTORY=$(pwd)}
+# The library would otherwise leave a repository of its own in the trash
+# directory, and every repository these tests want is built by mkrepo below.
 TEST_NO_CREATE_REPO=YesPlease
 . "$TEST_DIRECTORY"/test-lib.sh
 
-# Prepend the directory containing cgit to PATH.
+# The tests run cgit by name, so the binary just built has to come ahead of any
+# copy already installed. Under Valgrind the wrappers take that place instead.
 if test -n "$cgit_valgrind"
 then
 	GIT_VALGRIND="$TEST_DIRECTORY/valgrind"
@@ -60,27 +54,33 @@ fi
 
 FILTER_DIRECTORY=$(cd ../filters && pwd)
 
+# Lua filters only work when cgit was compiled with Lua, so the tests that use
+# them have to ask the binary rather than assume.
 if cgit --version | grep -F -q "[+] Lua scripting"; then
 	export CGIT_HAS_LUA=1
 else
 	export CGIT_HAS_LUA=0
 fi
 
+# Creates a repository holding the given number of one file commits. A third
+# argument asks for a variant, either testplus for a file and a branch whose
+# names have to be escaped in a URL, or commit-graph for a written commit graph.
 mkrepo() {
-	name=$1
-	count=$2
-	test_create_repo "$name"
+	repo=$1
+	commits=$2
+	variant=$3
+	test_create_repo "$repo"
 	(
-		cd "$name"
+		cd "$repo"
 		n=1
-		while test $n -le $count
+		while test $n -le $commits
 		do
 			echo $n >file-$n
 			git add file-$n
 			git commit -m "commit $n"
 			n=$(expr $n + 1)
 		done
-		case "$3" in
+		case "$variant" in
 		testplus)
 			echo "hello" >a+b
 			git add a+b
@@ -170,6 +170,8 @@ cgit_url()
 	CGIT_CONFIG="$PWD/cgitrc" QUERY_STRING="url=$1" cgit
 }
 
+# The response headers run down to the first empty line, and a test comparing
+# two pages wants to see the bodies alone.
 strip_headers() {
 	while read -r line
 	do
diff --git a/tests/t0001-validate-git-versions.sh b/tests/t0001-validate-git-versions.sh
index 65c3c6a..2117764 100755
--- a/tests/t0001-validate-git-versions.sh
+++ b/tests/t0001-validate-git-versions.sh
@@ -1,5 +1,12 @@
 #!/bin/sh
 
+# Checks that the Git version cgit says it is built for is the one it is
+# actually built against. The top-level Makefile names a version, the tree
+# under vendor/git records its own, and the submodule is pinned to a tag, so
+# all three have to agree or cgit is being built on something other than what
+# it claims. Set CGIT_TEST_NO_GIT_VERSION to YesPlease when Git comes from
+# elsewhere and the comparison has nothing to say.
+
 if [ "${CGIT_TEST_NO_GIT_VERSION}" = "YesPlease" ]; then
 	exit 0
 fi
@@ -15,10 +22,9 @@ test_expect_success 'extract Git version from Makefile' '
 	}" ../../Makefile >makefile_version
 '
 
-# Note that Git's GIT-VERSION-GEN script applies "s/-/./g" to the version
-# string to produce the internal version in the GIT-VERSION-FILE, so we
-# must apply the same transformation to the version in the Makefile before
-# comparing them.
+# Git's GIT-VERSION-GEN script applies "s/-/./g" to the version string on its
+# way into the GIT-VERSION-FILE, so the same has to be done to the version in
+# the Makefile before the two can be compared.
 test_expect_success 'test Git version matches Makefile' '
 	( cat ../../vendor/git/GIT-VERSION-FILE || echo "No GIT-VERSION-FILE" ) |
 	sed -e "s/GIT_VERSION[ 	]*=[ 	]*//" -e "s/\\.dirty$//" >git_version &&
@@ -26,6 +32,8 @@ test_expect_success 'test Git version matches Makefile' '
 	test_cmp git_version makefile_git_version
 '
 
+# A tree unpacked from a tarball has no submodule pin to describe, so there is
+# nothing to compare against and the test says so rather than failing.
 test_expect_success 'test submodule version matches Makefile' '
 	if ! test -e ../../vendor/git/.git
 	then
@@ -33,12 +41,12 @@ test_expect_success 'test submodule version matches Makefile' '
 	else
 		(
 			cd ../.. &&
-			sm_oid=$(git ls-files --stage -- vendor/git |
+			submodule_oid=$(git ls-files --stage -- vendor/git |
 				sed -e "s/^[0-9]* \\([0-9a-f]*\\) [0-9]	.*$/\\1/") &&
 			cd vendor/git &&
-			git describe --match "v[0-9]*" $sm_oid
-		) | sed -e "s/^v//" -e "s/-/./" >sm_version &&
-		test_cmp sm_version makefile_version
+			git describe --match "v[0-9]*" $submodule_oid
+		) | sed -e "s/^v//" -e "s/-/./" >submodule_version &&
+		test_cmp submodule_version makefile_version
 	fi
 '
 
diff --git a/tests/t0010-validate-html.sh b/tests/t0010-validate-html.sh
index 91433f8..308cef7 100755
--- a/tests/t0010-validate-html.sh
+++ b/tests/t0010-validate-html.sh
@@ -1,20 +1,30 @@
 #!/bin/sh
 
+# Runs the tidy checker over one page of each kind cgit renders, so that markup
+# broken enough to confuse a browser is caught here rather than in the browser.
+# Only the shape of the markup matters, since what the pages actually say is
+# the business of the t01xx scripts. Tidy is optional and old versions of it
+# predate the elements cgit uses, so the whole file steps aside when a usable
+# one cannot be found.
+
 test_description='Validate html with tidy'
 . ./setup.sh
 
-
 test_url()
 {
-	tidy_opt="-eq"
-	test -z "$NO_TIDY_WARNINGS" || tidy_opt+=" --show-warnings no"
+	tidy_options="-eq"
+	test -z "$NO_TIDY_WARNINGS" ||
+		tidy_options="$tidy_options --show-warnings no"
 	cgit_url "$1" >tidy-$test_count.tmp || return
+	# Tidy reads what it is given as a whole document, so the response
+	# headers are dropped before it sees the page.
 	sed -e "1,4d" tidy-$test_count.tmp >tidy-$test_count || return
-	"$tidy" $tidy_opt tidy-$test_count
-	rc=$?
+	"$tidy" $tidy_options tidy-$test_count
+	status=$?
 
-	# tidy returns with exitcode 1 on warnings, 2 on error
-	if test $rc = 2
+	# Tidy leaves with 1 for warnings and 2 for errors, and only an error is
+	# worth failing the test over.
+	if test $status = 2
 	then
 		false
 	else
@@ -22,15 +32,15 @@ test_url()
 	fi
 }
 
-tidy=`which tidy 2>/dev/null`
+tidy=$(which tidy 2>/dev/null)
 test -n "$tidy" || {
 	skip_all='Skipping html validation tests: tidy not found'
 	test_done
 	exit
 }
 
-# Releases of tidy that predate HTML5 reject the semantic elements (header,
-# nav, main, footer) that cgit emits, reporting them as errors. Skip the
+# Releases of tidy that predate HTML5 reject the semantic elements cgit emits,
+# header, nav, main and footer among them, reporting each as an error. Skip the
 # validation unless tidy is new enough to understand them.
 printf '<!DOCTYPE html>\n<html lang="en"><head><title>t</title></head><body><header>x</header></body></html>\n' \
 	| "$tidy" -q -e >/dev/null 2>&1
diff --git a/tests/t0020-validate-cache.sh b/tests/t0020-validate-cache.sh
index 510e51c..0cf7433 100755
--- a/tests/t0020-validate-cache.sh
+++ b/tests/t0020-validate-cache.sh
@@ -1,5 +1,12 @@
 #!/bin/sh
 
+# Exercises the cache, which keeps a rendered page in a slot on disk and
+# replays it for the next request that asks for the same thing. The first three
+# tests set cache-size to nothing, to one slot and to the full table in turn,
+# then count what the requests left behind. The rest cover the two ways a slot
+# can come out wrong, a page longer than the output buffer and a key too long
+# to be read back.
+
 test_description='Validate cache'
 . ./setup.sh
 
@@ -70,15 +77,14 @@ test_expect_success 'verify cache-size=1021' '
 	cgit_url "foo/ls_cache" >output.full &&
 	strip_headers <output.full >output &&
 	test_line_count = 13 output &&
-	# Check that ls_cache output is cached correctly
 	cgit_url "foo/ls_cache" >output.second &&
 	test_cmp output.full output.second
 '
 
-# --- A cached page must hold everything the uncached one produced -----------
-# Page output is buffered, so the slot is only complete if the buffer is
-# emptied before the generated content is measured. The blob here is larger
-# than one buffer, so a missing flush would truncate the cached copy.
+# Page output is buffered, so a slot only holds the whole page if the buffer is
+# emptied before the size of what was generated is taken. The blob set up here
+# is larger than one buffer, so a missing flush would leave the cached copy cut
+# short.
 test_expect_success 'set up a repo with a page larger than the output buffer' '
 	mkrepo repos/bigpage 1 &&
 	(
@@ -98,12 +104,15 @@ test_expect_success 'set up a repo with a page larger than the output buffer' '
 	rm -rf cache2 && mkdir cache2
 '
 
-bigq() { CGIT_CONFIG="$PWD/bigrc" QUERY_STRING="$1" cgit; }
+bigpage_query()
+{
+	CGIT_CONFIG="$PWD/bigrc" QUERY_STRING="$1" cgit
+}
 
 test_expect_success 'the first request fills the slot and the second replays it' '
-	bigq "url=bigpage/tree/big.txt" >big.first &&
+	bigpage_query "url=bigpage/tree/big.txt" >big.first &&
 	test $(wc -c <big.first) -gt 65536 &&
-	bigq "url=bigpage/tree/big.txt" >big.second &&
+	bigpage_query "url=bigpage/tree/big.txt" >big.second &&
 	strip_headers <big.first >big.first.body &&
 	strip_headers <big.second >big.second.body &&
 	test_cmp big.first.body big.second.body
@@ -113,21 +122,21 @@ test_expect_success 'the cached body matches one generated with the cache off' '
 	sed -e "s/^cache-size=64$/cache-size=0/" bigrc >bignocache &&
 	CGIT_CONFIG="$PWD/bignocache" QUERY_STRING="url=bigpage/tree/big.txt" cgit >big.nocache &&
 	strip_headers <big.nocache >big.nocache.body &&
-	# The footer carries the time the page was generated, which differs
-	# between the two runs by construction.
-	sed -e "s/generated by .*//" big.nocache.body >big.a &&
-	sed -e "s/generated by .*//" big.second.body >big.b &&
-	test_cmp big.a big.b
+	# The footer carries the time the page was generated, which the two runs
+	# cannot agree on.
+	sed -e "s/generated by .*//" big.nocache.body >big.nocache.timeless &&
+	sed -e "s/generated by .*//" big.second.body >big.cached.timeless &&
+	test_cmp big.nocache.timeless big.cached.timeless
 '
 
 test_expect_success 'the page ends where it should, so nothing was dropped' '
 	tail -c 200 big.second.body | grep "</html>"
 '
 
-# --- A key too long to read back must not claim a slot ----------------------
-# A slot stores its key ahead of the content and only the first few kilobytes
-# are read back, so a longer key could never match. Such a request used to
-# write a slot on every visit that no later request could ever use.
+# A slot stores its key ahead of the content and only the first few kilobytes of
+# it are read back, so a key longer than that could never match. A request
+# carrying one used to leave a slot behind on every visit that nothing could
+# ever go on to use.
 test_expect_success 'an over-long cache key leaves no slot behind' '
 	rm -rf cache3 && mkdir cache3 &&
 	sed -e "s|^cache-root=.*|cache-root=$PWD/cache3|" bigrc >bigkeyrc &&
diff --git a/tests/t0101-index.sh b/tests/t0101-index.sh
index 82ef9b0..be382ad 100755
--- a/tests/t0101-index.sh
+++ b/tests/t0101-index.sh
@@ -1,5 +1,12 @@
 #!/bin/sh
 
+# The index page is what cgit serves at the root of a site, one row for every
+# repository named in cgitrc. These checks look for each repository the shared
+# setup builds along with its description, and for the escaping a name that
+# contains a plus or a space needs before it can go into a link. They also
+# confirm the index stays a plain list, without the tree and log links that
+# belong to a repository's own pages.
+
 test_description='Check content on index page'
 . ./setup.sh
 
diff --git a/tests/t0102-summary.sh b/tests/t0102-summary.sh
index f1c1e78..a953ce3 100755
--- a/tests/t0102-summary.sh
+++ b/tests/t0102-summary.sh
@@ -1,5 +1,12 @@
 #!/bin/sh
 
+# The summary page is the landing page for a single repository, holding its
+# most recent commits together with its branches and its tags. The shared
+# setup caps that log at five entries, so the checks against the fifty commit
+# repository are really checking that the cut lands where it should. They also
+# confirm the clone url template from cgitrc has had the repository name
+# substituted into it.
+
 test_description='Check content on summary page'
 . ./setup.sh
 
diff --git a/tests/t0103-log.sh b/tests/t0103-log.sh
index bdf1435..41b9769 100755
--- a/tests/t0103-log.sh
+++ b/tests/t0103-log.sh
@@ -1,5 +1,11 @@
 #!/bin/sh
 
+# The log page lists the commits on a branch and can narrow that list down
+# with a search. The searching checks run against the repository whose name
+# contains a space and search for a term containing a space, so every link
+# cgit writes back out has to escape both the path it points at and the query
+# it carries, and the two are escaped differently.
+
 test_description='Check content on log page'
 . ./setup.sh
 
diff --git a/tests/t0104-tree.sh b/tests/t0104-tree.sh
index 2e140f5..c5c5c4c 100755
--- a/tests/t0104-tree.sh
+++ b/tests/t0104-tree.sh
@@ -1,5 +1,11 @@
 #!/bin/sh
 
+# The tree page browses a repository at one revision, either as a listing of a
+# directory or as a single file with an anchor on every line. The checks
+# against the repository named foo+bar cover a file name and a branch name
+# that both contain a plus, which cgit has to spell one way inside a path and
+# another way inside a query.
+
 test_description='Check content on tree page'
 . ./setup.sh
 
diff --git a/tests/t0105-commit.sh b/tests/t0105-commit.sh
index 1a12ee3..47edc5f 100755
--- a/tests/t0105-commit.sh
+++ b/tests/t0105-commit.sh
@@ -1,5 +1,11 @@
 #!/bin/sh
 
+# The commit page shows a single commit, its message, the files it touched and
+# the diff for them. Most of these checks read the markup cgit emits for the
+# tree link, the parent link, the subject and the diffstat. The last few ask
+# for the root commit of a repository, which has no parent and so goes through
+# the code that compares a commit against an empty tree.
+
 test_description='Check content on commit page'
 . ./setup.sh
 
diff --git a/tests/t0106-diff.sh b/tests/t0106-diff.sh
index 4074276..175f609 100755
--- a/tests/t0106-diff.sh
+++ b/tests/t0106-diff.sh
@@ -1,5 +1,12 @@
 #!/bin/sh
 
+# The diff page renders the change a commit made, either as unified text or,
+# when dt=1 asks for it, as a side by side table. The first checks read the
+# markup for one added file in the repository the shared setup builds. The
+# rest build small repositories of their own, shaped so the side by side
+# renderer meets the two cases it used to get wrong, a hunk covering a single
+# line and a file where every line changed.
+
 test_description='Check content on diff page'
 . ./setup.sh
 
@@ -16,10 +23,10 @@ test_expect_success 'find added line' '
 	grep "<div class=.add.>+5</div>" tmp
 '
 
-# --- Side-by-side line numbers for a hunk that covers a single line ---------
-# git writes the length only when a hunk spans more than one line, so a header
-# reads "@@ -1 +1 @@". Requiring a comma after the number left those hunks
-# numbered from zero, which dropped the line-number link entirely.
+# git writes a hunk length only when the hunk spans more than one line, so the
+# header for a single line reads "@@ -1 +1 @@" with no comma in it. Requiring
+# that comma left those hunks numbered from zero, which dropped the line
+# number link altogether.
 test_expect_success 'set up a repo with single-line hunks' '
 	mkrepo repos/hunk1 1 &&
 	(
@@ -51,10 +58,11 @@ test_expect_success 'ssdiff still numbers a hunk that carries a length' '
 	grep "#n1.>1</a>" tmp
 '
 
-# --- Intra-line highlighting across many changed pairs ----------------------
-# The character-level highlight reuses one table across every changed pair.
-# Lines shrink down the file so a short comparison always follows a longer one,
-# which is where a stale cell would show up as misplaced del and add spans.
+# The character level highlight reuses one table across every changed pair, so
+# these lines get shorter as the file goes on and each comparison is smaller
+# than the one before it. That is where a cell left over from the previous
+# pair shows itself, as a del or an add span sitting over the wrong
+# characters.
 test_expect_success 'set up a repo with shrinking changed lines' '
 	mkrepo repos/lcs 1 &&
 	(
@@ -78,10 +86,10 @@ test_expect_success 'set up a repo with shrinking changed lines' '
 
 test_expect_success 'every changed pair gets both a del and an add span' '
 	CGIT_CONFIG="$PWD/lcsrc" QUERY_STRING="url=lcs/diff/&dt=1" cgit >tmp &&
-	dels=$(grep -o "<span class=.del.>" tmp | wc -l) &&
-	adds=$(grep -o "<span class=.add.>" tmp | wc -l) &&
-	test "$dels" -gt 0 &&
-	test "$dels" -eq "$adds"
+	deletions=$(grep -o "<span class=.del.>" tmp | wc -l) &&
+	additions=$(grep -o "<span class=.add.>" tmp | wc -l) &&
+	test "$deletions" -gt 0 &&
+	test "$deletions" -eq "$additions"
 '
 
 test_expect_success 'stripping the highlight leaves every line intact' '
diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh
index 0811ec4..0762ab2 100755
--- a/tests/t0107-snapshot.sh
+++ b/tests/t0107-snapshot.sh
@@ -1,5 +1,12 @@
 #!/bin/sh
 
+# The snapshot page hands a branch back as a compressed archive. Most of the
+# formats the shared setup enables are fetched here, then their headers are
+# read, their bodies unpacked and the files inside compared against the
+# repository that setup built. A format needing a helper program that is not
+# always installed sits behind a prerequisite, so a machine without lzip, xz,
+# zstd or unzip skips those checks instead of failing them.
+
 test_description='Verify snapshot'
 . ./setup.sh
 
diff --git a/tests/t0108-patch.sh b/tests/t0108-patch.sh
index ffcba96..3d38e55 100755
--- a/tests/t0108-patch.sh
+++ b/tests/t0108-patch.sh
@@ -1,5 +1,12 @@
 #!/bin/sh
 
+# The patch page serves a commit as a mail ready patch, so that a change read
+# from cgit can be fed straight to git am. The checks compare that output
+# against git format-patch for a single commit and for a range of them, once
+# the CGI headers have been stripped, which means the two have to agree line
+# for line. The last one sets max-patch-count so a range wider than the limit
+# comes back cut short.
+
 test_description='Check content on patch page'
 . ./setup.sh
 
diff --git a/tests/t0109-gitconfig.sh b/tests/t0109-gitconfig.sh
index 189ef28..7078596 100755
--- a/tests/t0109-gitconfig.sh
+++ b/tests/t0109-gitconfig.sh
@@ -1,8 +1,17 @@
 #!/bin/sh
 
+# Guards the promise that cgit reads nothing out of the home directory of
+# whichever account the web server happens to run as. Every page is fetched
+# under strace with HOME pointed at a path that is known not to exist, and
+# the run fails if any access call names that path, which is how a stray
+# read of a personal gitconfig would show up.
+
 test_description='Ensure that git does not access $HOME'
 . ./setup.sh
 
+# strace needs ptrace, which containers and hardened kernels refuse even
+# where the binary is installed, so a working run is checked as well as a
+# present binary.
 test -n "$(which strace 2>/dev/null)" || {
 	skip_all='Skipping access validation tests: strace not found'
 	test_done
@@ -16,16 +25,21 @@ strace true 2>/dev/null || {
 }
 
 test_no_home_access () {
-	non_existent_path="/path/to/some/place/that/does/not/possibly/exist"
-	while test -d "$non_existent_path"; do
-		non_existent_path="$non_existent_path/$(date +%N)"
+	# A home that happened to exist would be one git may legitimately
+	# read, leaving the check below with nothing to catch, so extend the
+	# path until nothing is there.
+	missing_home="/path/to/some/place/that/does/not/possibly/exist"
+	depth=0
+	while test -d "$missing_home"; do
+		depth=$((depth + 1))
+		missing_home="$missing_home/$depth"
 	done &&
 	strace \
-		-E HOME="$non_existent_path" \
+		-E HOME="$missing_home" \
 		-E CGIT_CONFIG="$PWD/cgitrc" \
 		-E QUERY_STRING="url=$1" \
 		-e access -f -o strace.out cgit &&
-	! grep "$non_existent_path" strace.out
+	! grep "$missing_home" strace.out
 }
 
 test_no_home_access_success() {
diff --git a/tests/t0110-rawdiff.sh b/tests/t0110-rawdiff.sh
index 66fa7d5..23d37a5 100755
--- a/tests/t0110-rawdiff.sh
+++ b/tests/t0110-rawdiff.sh
@@ -1,5 +1,11 @@
 #!/bin/sh
 
+# Checks the rawdiff page, which serves a diff as plain text with no markup
+# around it so that the result can be fed straight to patch. Every test runs
+# the git command the page is meant to mirror and compares the two byte for
+# byte, covering an ordinary commit, the initial commit that has no parent,
+# and a range spanning several commits.
+
 test_description='Check content on rawdiff page'
 . ./setup.sh
 
@@ -7,6 +13,9 @@ test_expect_success 'generate foo/rawdiff' '
 	cgit_query "url=foo/rawdiff" >tmp
 '
 
+# cgit writes its CGI headers and the blank line that ends them ahead of the
+# body while git writes only the diff, so the head of the page comes off
+# before the two are compared.
 test_expect_success 'compare with output of git-diff(1)' '
 	git --git-dir="$PWD/repos/foo/.git" diff HEAD^.. >tmp2 &&
 	sed "1,4d" tmp >tmp_ &&
diff --git a/tests/t0111-filter.sh b/tests/t0111-filter.sh
index da4172f..88533f4 100755
--- a/tests/t0111-filter.sh
+++ b/tests/t0111-filter.sh
@@ -1,5 +1,11 @@
 #!/bin/sh
 
+# Checks the source, about, commit and email filters, which hand a piece of
+# page content to an outside program before it is written out. The whole
+# body runs once per kind of filter, the exec kind always and the lua kind
+# only where cgit was built with lua, because both have to produce the same
+# page from the same repository.
+
 test_description='Check filtered content'
 . ./setup.sh
 
@@ -42,11 +48,13 @@ do
 		grep "<committer@example.com> commit C O MITTER &LT;COMMITTER@EXAMPLE.COM&GT;" tmp
 	'
 
-	# Page output is buffered, so anything written before a filter opens has
-	# to leave the buffer before the filter takes over stdout, and anything
-	# written while it is open has to leave before stdout is handed back.
-	# A missed flush reorders the page rather than losing it, so check that
-	# the markup around the filtered text is still on the right side of it.
+	# Page output is buffered, so whatever was written before a filter
+	# opens has to leave the buffer before the filter takes over stdout,
+	# and whatever was written while it was open has to leave before
+	# stdout is handed back, since those bytes are meant for the filter.
+	# A missed flush reorders the page rather than losing any of it,
+	# so the two tests below confirm that the markup around the filtered
+	# text is still on the side of it that it belongs on.
 	test_expect_success "the $prefix source filter output stays inside its cell" "
 		cgit_url 'filter-$prefix/tree/a%2bb' >tmp &&
 		tr -d '\n' <tmp >flat.out &&
diff --git a/tests/t0200-security.sh b/tests/t0200-security.sh
index 9238262..f475301 100755
--- a/tests/t0200-security.sh
+++ b/tests/t0200-security.sh
@@ -1,10 +1,19 @@
 #!/bin/sh
 
+# Collects the regression tests for the security fixes and for the behaviour
+# this fork adds on top of upstream cgit. Each case builds the smallest
+# repository and config that reproduce the original problem and then asks for
+# the page that used to mishandle it. The comment above a case says what the
+# page is being defended against, because a request that looks ordinary is
+# usually the whole point of the attack.
+
 test_description='Check security fixes and fork-specific behavior'
 . ./setup.sh
 
-# A repo with an oversized blob, readmes that carry markup, and a directory,
-# plus a config that pins a tiny blob limit and groups directories.
+# Most of what follows shares one repository and one config, so the fixture
+# carries everything they need at once, a blob over the size limit, readmes
+# holding markup that must not reach the page as markup, and a subdirectory
+# to sort ahead of the files.
 test_expect_success 'set up security fixtures' '
 	mkrepo repos/sec 1 &&
 	(
@@ -31,9 +40,9 @@ test_expect_success 'set up security fixtures' '
 
 secq() { CGIT_CONFIG="$PWD/seccgitrc" QUERY_STRING="$1" cgit; }
 
-# --- Argument injection through the log id= parameter -----------------------
-# A tip beginning with a dash would be parsed as a git option, and
-# id=--output=<path> would create or truncate an arbitrary file.
+# A revision beginning with a dash reaches git as an option rather than as a
+# tip, so a request for id=--output=<path> could create or truncate any file
+# the server is able to write.
 test_expect_success 'log id=--output does not write a file' '
 	rm -f pwned &&
 	cgit_query "url=foo/log&id=--output=$PWD/pwned" >tmp 2>&1 &&
@@ -55,7 +64,9 @@ test_expect_success 'a valid id= still renders the log' '
 	grep -i "commit 5" tmp
 '
 
-# --- max-blob-size is enforced before the object is read --------------------
+# max-blob-size is enforced before the object is read, so every view that
+# would otherwise inline a file has to turn the same one away rather than
+# inflate it first and think better of it afterwards.
 test_expect_success 'tree view refuses an oversized blob' '
 	secq "url=sec/tree/big.txt" | grep -iE "exceeds|too large"
 '
@@ -72,7 +83,9 @@ test_expect_success 'a small blob is still served' '
 	secq "url=sec/plain/afile" | grep -F "top"
 '
 
-# --- Readme rendering escapes untrusted repository content ------------------
+# A readme is repository content, so with no about filter configured it has
+# to reach the page escaped instead of as live markup, whatever its name
+# suggests about the format.
 test_expect_success 'markdown readme without a filter is escaped as plain text' '
 	{
 		echo "virtual-root=/" &&
@@ -104,9 +117,9 @@ test_expect_success 'non-markdown readme keeps its line structure' '
 	grep "pre class=.plaintext." tmp
 '
 
-# --- Auto-submitting selects carry no inline handlers ------------------------
-# A Content-Security-Policy without unsafe-inline blocks inline onchange
-# handlers, so the forms mark their selects and cgit.js wires them up.
+# A Content-Security-Policy without unsafe-inline stops an inline onchange
+# handler from ever running, so the option forms only mark their selects and
+# cgit.js wires the submit up from outside the page.
 test_expect_success 'diff option selects use the autosubmit marker' '
 	sha=$(git -C repos/foo rev-parse HEAD) &&
 	cgit_query "url=foo/commit&id=$sha" >tmp &&
@@ -114,7 +127,8 @@ test_expect_success 'diff option selects use the autosubmit marker' '
 	! grep "onchange" tmp
 '
 
-# --- Fork feature: directories are grouped before files in the tree ---------
+# Grouping directories ahead of files is behaviour this fork adds, so nothing
+# upstream covers it.
 test_expect_success 'tree groups directories before files' '
 	secq "url=sec/tree/" >tmp &&
 	dirline=$(grep -n "tree/zsub" tmp | head -1 | cut -d: -f1) &&
@@ -124,9 +138,9 @@ test_expect_success 'tree groups directories before files' '
 	test "$dirline" -lt "$fileline"
 '
 
-# --- Side-by-side diff percent-encodes a file path into its links -----------
-# A file name is repository content and may contain a quote, which would
-# otherwise break out of the href attribute of the line-number links.
+# A file name is repository content and may hold a quote, which would break
+# out of the href attribute on the line number links of a side by side diff,
+# so the path is percent-encoded on its way into them.
 test_expect_success 'ssdiff percent-encodes a quoted file path' '
 	mkrepo repos/xss 1 &&
 	name=$(printf "x\047y.txt") &&
@@ -150,7 +164,8 @@ test_expect_success 'ssdiff percent-encodes a quoted file path' '
 	! grep "href=.[^>]*x.y.txt.[^>]*>" tmp
 '
 
-# --- A commit with no message must not crash the history views --------------
+# git itself will write a commit with an empty message, so the log and the
+# summary both have to have something to print where the subject goes.
 test_expect_success 'a message-less commit renders without crashing' '
 	mkrepo repos/nomsg 1 &&
 	(
@@ -172,12 +187,12 @@ test_expect_success 'a message-less commit renders without crashing' '
 	grep "no commit message" tmp
 '
 
-# --- A branch need not point at a commit ------------------------------------
-# struct refinfo keeps taginfo and commitinfo in a union and fills only the one
-# matching the object type. Sorting branches by reading the commit member read
-# past the end of the smaller taginfo, and read NULL for a tree, which crashed.
-# git update-ref refuses to write these, so the refs go in as loose files: a
-# repository is just files on disk and cgit reads whatever is there.
+# A branch need not point at a commit. struct refinfo keeps taginfo and
+# commitinfo in a union and fills only the member matching the object type,
+# so sorting branches through the commit member read past the end of the
+# smaller taginfo, and read NULL for a tree, which crashed. git update-ref
+# refuses to create such a ref, hence the loose files written by hand below,
+# and a repository is only files on disk so cgit meets whatever is there.
 test_expect_success 'set up a repo whose branches point at odd objects' '
 	mkrepo repos/oddref 2 &&
 	(
@@ -229,10 +244,11 @@ test_expect_success 'name-sorted branches are unaffected' '
 	grep "</html>" tmp
 '
 
-# --- A repository cannot supply a printf format string ----------------------
-# module-link is a template, and scan-path lets a repository set it through its
-# own cgitrc. Handing it to printf let a repo owner crash the process, or read
-# stack memory into the served page, with surplus conversions.
+# module-link is a template and scan-path lets a repository set its own from
+# a cgitrc in the tree, so handing that string to printf let the owner of a
+# scanned repository crash the process, or read stack memory into the served
+# page, merely by adding conversions past the two that are filled. The tests
+# after the fixture also pin down the templates that must keep working.
 test_expect_success 'set up a submodule fixture with a hostile module-link' '
 	mkrepo repos/modlink 1 &&
 	(
diff --git a/tests/t0201-limits.sh b/tests/t0201-limits.sh
index 72d8a23..f09c290 100755
--- a/tests/t0201-limits.sh
+++ b/tests/t0201-limits.sh
@@ -1,5 +1,14 @@
 #!/bin/sh
 
+# Checks the ceilings a config can put on a page, that is the caps on refs
+# listed, on the lines and the files a diff renders inline, and on the size
+# of a blob any view will inflate, along with the clamp on how long an index
+# query may be. Crossing one of these has to trim the page or point the
+# reader at somewhere better suited, never drop the request, so each case
+# watches what survives as closely as what is left out. The last case is the
+# same idea applied to a filter, which degrades to escaped text rather than
+# failing when its highlighting library is absent.
+
 test_description='Check the ref listing and diff size limits'
 . ./setup.sh
 
@@ -7,8 +16,10 @@ if [ $CGIT_HAS_LUA -eq 1 ]; then
 	test_set_prereq LUA
 fi
 
-# A repo with several branches and tags, one commit with an oversized file
-# diff beside a small one, and configs that pin tiny limits.
+# The limits only show themselves against content that crosses them, so the
+# fixture carries more branches and tags than max-ref-count allows and a
+# commit whose oversized file diff sits beside a small one, which is what
+# tells a per file limit apart from a whole page one.
 test_expect_success 'set up limit fixtures' '
 	mkrepo repos/limits 1 &&
 	(
@@ -43,10 +54,10 @@ test_expect_success 'set up limit fixtures' '
 
 limitq() { CGIT_CONFIG="$PWD/limitrc" QUERY_STRING="$1" cgit; }
 
-# --- A request cannot ask for an unbounded amount of matching ---------------
-# The query is compared against every repository the index lists, so an
-# enormous one would multiply out across the whole listing. It is clamped
-# rather than rejected, so an ordinary query still narrows the page.
+# An index query is compared against every repository the listing holds, so
+# an enormous one multiplies out across the whole index. Clamping it rather
+# than refusing it keeps the cost bounded without making an ordinary search
+# behave any differently.
 test_expect_success 'an enormous query is clamped, not rejected' '
 	long=$(awk "BEGIN{s=\"\";for(i=0;i<4000;i++)s=s \"a\"; print s}") &&
 	test ${#long} -eq 4000 &&
@@ -62,7 +73,9 @@ test_expect_success 'an ordinary query still filters the index' '
 	! grep ">bar<" tmp
 '
 
-# --- The refs page caps each section and links to the category pages --------
+# The combined refs page caps branches and tags separately, and the pages it
+# hands the overflow to page on their own, so a limit that leaked between the
+# two sections would show up as the wrong link or the wrong count here.
 test_expect_success 'refs page lists max-ref-count branches and tags' '
 	limitq "url=limits/refs/" >tmp &&
 	test $(grep -c "log/?h=" tmp) -eq 2 &&
@@ -90,7 +103,10 @@ test_expect_success 'tag page paginates independently' '
 	! grep "log/?h=" tmp
 '
 
-# --- Oversized file diffs link out instead of rendering inline --------------
+# A file that busts max-diff-lines is replaced by a link to its own page,
+# where the reader asked for that one file and the limit no longer applies.
+# The rest of the commit still renders, so one huge file cannot hide the
+# small change beside it.
 test_expect_success 'oversized file diff is replaced by a link' '
 	limitq "url=limits/commit/" >tmp &&
 	grep "too large to be rendered inline" tmp &&
@@ -107,7 +123,9 @@ test_expect_success 'the single-file diff page always renders in full' '
 	! grep "too large to be rendered inline" tmp
 '
 
-# --- Commits over max-diff-files fall back to the diffstat ------------------
+# max-diff-files counts the files in a commit rather than the lines in one,
+# so it drops the whole body back to the diffstat while a request naming a
+# single file stays unaffected.
 test_expect_success 'a commit changing too many files shows stat only' '
 	CGIT_CONFIG="$PWD/limitfilesrc" QUERY_STRING="url=limits/commit/" cgit >tmp &&
 	grep "too large to be rendered inline" tmp &&
@@ -119,9 +137,9 @@ test_expect_success 'the single-file page is not limited by max-diff-files' '
 	grep "class=.hunk." tmp
 '
 
-# --- max-blob-size also bounds the diff path --------------------------------
-# The diff family must not inflate a blob larger than max-blob-size just to
-# render it. Such a file is reported as binary instead of inlined.
+# The diff views must not inflate a blob past max-blob-size merely to render
+# it, which is the same defence the tree and plain views make and is easy to
+# miss on this path. Such a file is reported as binary instead.
 test_expect_success 'a diff of an oversized blob is not inlined' '
 	mkrepo repos/blobdiff 1 &&
 	(
@@ -145,7 +163,9 @@ test_expect_success 'a diff of an oversized blob is not inlined' '
 	! grep "aaaaaaaa" tmp
 '
 
-# --- The shipped highlight filter degrades to escaped passthrough -----------
+# A missing scintillua leaves the shipped filter with nothing to highlight
+# with, and a filter that failed there would take the whole page down with
+# it, so it has to hand the source back escaped instead.
 test_expect_success LUA 'highlight filter passes text through without scintillua' '
 	{
 		echo "virtual-root=/" &&
diff --git a/tests/t0202-stats.sh b/tests/t0202-stats.sh
index 1589776..2ada672 100755
--- a/tests/t0202-stats.sh
+++ b/tests/t0202-stats.sh
@@ -1,9 +1,15 @@
 #!/bin/sh
 
+# Checks the per repository statistics page, which stays off until a config
+# turns it on and then reports commits per author beside a breakdown of the
+# tree by language. A second config caps the period so that what max-stats
+# takes out of the period selector is covered as well.
+
 test_description='Check the statistics page'
 . ./setup.sh
 
-# The default test config carries no enable-stats.
+# The shared test config sets no enable-stats, so this is what an untouched
+# install does.
 test_expect_success 'stats are off by default' '
 	cgit_url "foo/stats" >tmp &&
 	grep "Status: 404" tmp &&
@@ -11,10 +17,11 @@ test_expect_success 'stats are off by default' '
 	! grep ">stats</a>" tmp
 '
 
-# A repo with one commit dated now, so the authors table has something
-# to count beside the 2005-dated fixture commits. The commit graph
-# makes the language walk take the graph lookup path, which once
-# returned another commit after the history walk released memory.
+# The authors table counts only recent periods, so the fixture needs a commit
+# dated now to sit beside the 2005 dates the shared repositories carry. The
+# commit graph is written on purpose, because it sends the language walk down
+# the graph lookup path, which once handed back another commit after the
+# history walk had released its memory.
 test_expect_success 'set up a stats config' '
 	mkrepo repos/pulse 2 &&
 	(
diff --git a/tests/t0203-dates.sh b/tests/t0203-dates.sh
index 1bd6253..02f12f1 100755
--- a/tests/t0203-dates.sh
+++ b/tests/t0203-dates.sh
@@ -1,10 +1,16 @@
 #!/bin/sh
 
+# Checks how cgit shows a commit date, which is either an age relative to now
+# or a calendar date in the format the config names. The config is rewritten
+# between cases so that each one names only the settings it is about and
+# leaves the rest at their defaults.
+
 test_description='Check the date display options'
 . ./setup.sh
 
-# An old commit and a fresh one, so a single log page exercises both the
-# relative rendering and the calendar fallback past the two week cutoff.
+# One old commit and one fresh one, so a single page shows both the relative
+# rendering and the calendar fallback that takes over past the two week
+# cutoff.
 test_expect_success 'set up a dated repo' '
 	mkrepo repos/dated 1 &&
 	(
@@ -18,7 +24,8 @@ test_expect_success 'set up a dated repo' '
 	)
 '
 
-# Each argument is written as one extra config line.
+# Writes the config afresh, one extra line per argument, which is what lets a
+# test below name a setting or two and say nothing about the rest.
 daterc() {
 	{
 		echo "virtual-root=/"