fix: repair the acceptance and benchmark shell scripts

[akiomik]

Three bugs that stop the development scripts from doing what they say. None is
user-visible, so there is no changelog entry.

The fixture loop has never run

scripts/acceptance/setup.sh sets aside the fixtures that cannot be compared.
Upstream pairs some with an X_style.rb, and test/test_rules.rb
(do_lint, do_fix_lint) loads it to select the rules that fixture is checked
against and to set their parameters:

# long_lines_100_style.rb, beside a document written to 100 columns
rule 'MD013', :line_length => 100

test.sh cannot reproduce that. It runs each tool once with a config of its
own — mdl under scripts/acceptance/.mdlrc, mado under whatever mado.toml
the working directory yields — and neither of those is the fixture's style, so
the document is read at whatever width they say. The fixtures with a style
file are exactly the parameter-sensitive ones (MD003, MD004, MD013, MD035,
MD046 and so on), which is why they are set aside rather than compared.

(Reviewing this turned up that those two configs do not match each other either
— mado is given indent = 4 for MD007 where mdl uses its default of 3, among
others — which makes test.sh working-directory dependent and puts part of
#401's divergence down to the harness. Recorded there; not something this pull
request changes.)

The loop that does this did nothing, for two reasons that hid each other:

1. find data/markdownlint/test/rule_tests is relative, and the script has
   already cded into that clone, so it searched
   .../markdownlint/data/markdownlint/test/rule_tests and found nothing.
2. Fixing the path exposes [ $style_file -eq "..." ], an integer comparison,
   which raises Illegal number on every iteration.

default_test_style.rb is skipped because it is the shared fallback
test_rules.rb uses for every fixture without a style of its own — which is
why it is the only style file with no .md beside it.

A bash array in #!/bin/sh

test.sh and comparison.sh both had PROJECT_ROOT=($SCRIPT_DIR/../..).
dash -n rejects them at line 4 with Syntax error: "(" unexpected. They ran
on macOS only because /bin/sh there is bash, where expanding an array without
a subscript yields its first element.

Making the loop run

Most of the diff is what running it for the first time requires:

state	result
document present	set aside
document gone, .bak present	nothing, exit 0
document restored by git restore, .bak present	set aside again
neither present	named on stderr, exit 0
nothing set aside at all	named on stderr, exit 1
mv fails	the run fails, exit 1

Keyed on the document rather than on the .bak: this loop leaves the clone
permanently dirty, so git restore inside it is a natural thing to do, and it
brings the documents back while leaving the untracked .bak files alone.

The last row is not incidental hardening — a silent, successful exit having
done nothing is how this bug looked from outside for nine months. It counts
what was set aside rather than checking that find matched something, because
matching something is not the claim: a corpus holding only the fallback style
file matches and still sets nothing aside.

It claims nothing about why the corpus is wrong, so both cds are checked as
well. Switching find to a clone-relative path makes arriving there
load-bearing in a way it was not before, and until now a failed clone let the
two git commands after it loose on mado's own repository. That closes the
clone-failed case; a markdownlint/ that exists without being a repository
still passes cd, and that is #399.

The line every path derives from

SCRIPT_DIR=$(cd $(dirname $0); pwd) is unquoted in each of these scripts, and
its cd is unguarded twice over. A checkout path containing a space
word-splits $(dirname $0); and $(dirname "$0") is a relative operand when
the script is invoked by a relative path, so cd resolves it through CDPATH
and echoes where it landed — into the substitution:

$ cd /tmp/sp && CDPATH=. dash x/s.sh
[/tmp/sp/x
/tmp/sp/x]                      # the path twice, newline-separated
$ cd /tmp/sp && CDPATH=/tmp/decoy dash x/s.sh
[/tmp/decoy/x ...]              # the wrong tree

Now SCRIPT_DIR=$(CDPATH= cd -- "$(dirname "$0")" && pwd) || exit 1. setup.sh reaches
its clone by absolute path for the same reason: cd markdownlint with CDPATH
set lands in an unrelated directory that happens to hold one, successfully, so
the guard above it never fires.

test.sh could not run on a fresh clone

tmp/ at the repository root is gitignored and nothing creates it, so both
redirects failed and the seds ran against files that were not there. It works
in an existing checkout only because the directory happens to be there.

It now checks what it needs before producing anything, each of these being
separable from what the tools reported:

	without it
mkdir -p "$TEMP_PATH" || exit 1	a failed redirection does not end a non-interactive shell
cd "$PROJECT_ROOT"	cargo run exits 101 into an already-created empty mado.txt
cargo build || exit 1	a failed build leaves the same empty file
command -v mdl, command -v cargo (git in setup.sh; hyperfine, node, cargo in comparison.sh)	the gem nothing here installs leaves mdl.txt reading as mdl finding nothing; cargo fails loudly, but a contributor without a toolchain should hear it here
[ -d "$DOC_PATH" ]	mado check on a missing path exits 0 saying "All checks passed!"
< /dev/null on the mado run	mado lints stdin instead of the given paths when stdin is neither a terminal nor empty, so piping a single newline into this script made it report on that and call the corpus clean — non-empty output, zero status, every guard satisfied. An empty stdin falls through to the paths, which is why comparison.sh needs no redirect
no document beside any style file	after a git restore in the clone, the set-aside documents are back and get compared under one shared config
SCRIPT_DIR non-empty	a failed cd there makes every derived path root-relative — DOC_PATH becomes /data/markdownlint/test/rule_tests
the corpus exists, then is not empty	nothing to lint produces two files that compare equal; the order matters, find otherwise failing on a missing directory in its own words before the message meant for it
each tool wrote something	mdl says nothing both when its config names a rule it lacks and when the corpus is clean; either way there is nothing to compare
both config files exist	mado --config on a missing file exits 1 with empty stdout, which is what violations look like; mdl exits 3 the same way. Named once, so the guard and the runs cannot drift apart
each benchmarked tool starts	--version on three of them and a no-match glob for markdownlint-cli2, which has no version flag; that one reads its config on the way, where markdownlint accepts a broken one without complaint when nothing matches (#400): present and executable is not the same as working, and a node_modules from a stale cache dies on MODULE_NOT_FOUND in milliseconds, which --ignore-failure prints as the fastest tool

The set-aside check looks for the absence of a document, not for the .bak
files: those are untracked, so a git restore leaves all of them in place
while bringing the documents back. Same reasoning as setup.sh's own loop, and
it searches recursively as that one does, so the two cannot disagree about the
corpus if upstream ever stops keeping it flat. It collects find's output rather than
piping it into a loop. $DOC_PATH is absolute, so a space anywhere in the
checkout path split every result into pieces that exist nowhere and the guard
passed on a corpus it never looked at; and an exit in the last stage of a
pipeline ends the script outright on shells that run that stage in the current
one, taking the diagnostic with it. setup.sh's loop is handed to find -exec
for the same reason, having had the opposite half of the problem: zsh does not
split an unquoted expansion at all, so zsh scripts/acceptance/setup.sh made
the whole list one iteration and set nothing aside. Both scripts read the
corpus the same way now, and all three scripts behave identically under dash,
bash and zsh — test.sh only after status=$? became rc=$?, status being
read-only in zsh, where assigning to it killed the script the moment mdl
returned and left an unfiltered mdl.txt with no mado.txt beside it:

$ DOC_PATH="/tmp/sp ace/rt"       # foo_style.rb and foo.md, not set aside
before: exit 0, silent            # tested /tmp/sp and ace/rt/foo.md
after:  exit 1, "fixtures are not set aside in /tmp/sp ace/rt"

The three cheap ones run before the build, so a contributor without the gem or
without a corpus is told so immediately rather than after it.

test.sh is quoted throughout, not only on line 3 — its sed operands split
too, and since sed is unchecked the script exited 0 having left mado.txt
unfiltered, a wrong answer rather than a missing one. (Their > /dev/null went
with them; sed -i prints nothing.) comparison.sh needed the same
treatment inside its command strings. An earlier revision claimed quoting could
not help there, since hyperfine hands each string to a shell that splits it
again — wrong, and the file disproved it: the markdownlint-cli2 line already
protected its glob with quotes inside the string, which only works because that
shell honours them.

Quoting them in place is not the fix either, though — it moves the problem from
spaces to apostrophes, and /Users/o'brien/src/mado closes the quote early.
The paths go through the environment instead, so the shell that expands them
does the quoting — and each run gets a -n name, hyperfine otherwise labelling
its results table with the variable references rather than with what ran:

/Users/o'brien/…   quoted in place: fails to start   via environment: runs
/Users/sp ace/…    quoted in place: runs             via environment: runs

It does get the same
cd as test.sh, though, and the checks that go with it: cargo build --release found its project through the working directory there too, and
--ignore-failure treats a command that could not start as a result rather
than an error, so it would have timed a stale binary or a command not found
at microsecond speed and printed either as this tree's number. It now asks for
hyperfine, mdl, node and both markdownlint commands up front — the
README's prerequisites name all three now — naming npm ci as
the remedy for the latter two, and testing those two with -x rather than
command -v — given a path rather than a name, dash reports an existing file
as found whether or not it can be run. Its corpus has to hold documents rather
than merely exist, none of benchmarks/setup.sh's git commands being checked,
and mado's own binary has to be where cargo put it — a successful
cargo build says nothing about that. CARGO_TARGET_DIR and
CARGO_BUILD_TARGET are honoured for both the check and the command hyperfine
times; their cargo-config equivalents, which a script cannot see, are named in
the failure message instead of the binary being reported as "not built". node is on the list because both markdownlint
commands are #!/usr/bin/env node wrappers, which -x finds startable while
they cannot run.
Nothing installs
node_modules, so those two were absent on a fresh clone as the rule rather
than the exception — along with its corpus and its three config files, as
test.sh does.

Neither output file is left half-written

The pair is removed before the guards run, so stopping at one of them leaves
nothing behind: a previous run's complete pair would otherwise sit there
looking finished and be read as this one's. That costs a good pair whenever a
precondition fails, which is the trade taken on purpose: the two files are
regenerable by re-running, and being misled by them is not. Past the guards, both outputs are
built under working names and moved into place only once both are ready, the
trap asking whether the pair is there rather than whether the run reached a
flag — so no signal has a window to catch: after the first rename only one
exists and both go, after the second both exist and both stay — which covers
the exits after the tools run — a mado failure used to leave a partial
mado.txt beside an unfiltered mdl.txt, the filtering happening last.

Verified both ways: tripping the corpus guard, and failing the mado
invocation, each leave the directory empty where the earlier arrangement left
last run's files in place. This matters more now that the README documents
diff tmp/mdl.txt tmp/mado.txt as a separate step.

The clone is checked for being one

cd "$CLONE_DIR" proves the directory is there, not that it is the clone, and
git resolves .git upward — so a directory that exists without being a
repository sent sparse-checkout set and git checkout into mado's own
repository and emptied its working tree.

Asking git for the prefix — empty only when the working directory is a
repository's own top level — is the first proposal that survives every case the
earlier ones failed:

	cd guard	.git test	GIT_CEILING_DIRECTORIES	--show-prefix
directory exists, not a repository	passes	passes	rejects	rejects
directory holds a bare .git	passes	passes	rejects	rejects
directory in no repository at all	passes	passes	rejects	rejects
GIT_DIR exported to the outer repo	passes	passes	passes	rejects
a real clone	passes	passes	passes	passes

The prefix rather than comparing --show-toplevel against pwd -P: those are
different spellings of the same directory under Git for Windows, where the
comparison would reject a perfectly good clone and re-cloning would never help.

Deferred through four reviews on the grounds that #399 should settle it once
for both scripts; landed here because this pull request is what made arriving
in the clone load-bearing, and the check is now settled rather than guessed.
benchmarks/setup.sh still needs it and stays with the issue.

What is deliberately left unchecked

What the two linters report: both exit non-zero on violations, the normal
case here, so gating on that would abort the comparison this exists to produce.
Their statuses are still looked at — anything above 1 is the tool failing
rather than finding something — and each output has to be non-empty, since mado
prints "All checks passed!" when it genuinely finds none.

The seds are no longer among them. They write through a temporary file rather
than sed -i '', whose empty suffix GNU sed reads as another input file: the
same line exits non-zero having edited the file correctly, so the three of them
could not be checked, and a filter that genuinely failed left the trailers in
place under a successful exit. The closing exit 0 went with them, having
existed only to paper over that status on Linux. That closes the sed -i ''
half of #398.

setup.sh says how many fixtures are set aside. The dots its loop prints are
the count, captured rather than shown, so a successful re-run's only output was
git clone reporting the directory already exists — success and failure looked
alike. It is phrased about the corpus rather than about the run, since the
count includes what an earlier run set aside and this one had nothing to do
for. (Sending the dots to stderr instead, as suggested, empties the capture and
makes the guard below fire every time; the count line is the version that
works.)

test.sh clears its working names from a trap rather than at the end. Every
exit between the first tool and the final rename left some of them in tmp/
until the next run cleaned up — the published names were always right, the
debris was not. It takes two traps, because an EXIT trap runs when a signal
is caught but not when the shell dies from an uncaught one:

	dash	bash	zsh
INT, EXIT trap alone	cleaned	cleaned	cleaned
TERM, EXIT trap alone	debris	cleaned	debris
either, plus trap 'exit 1' INT TERM HUP	cleaned	cleaned	cleaned

TERM is a CI cancelling a job, so the second trap is the one that matters.

Raised in review and not changed

Moving the rm -f of the published pair below the preconditions, so that a
missing mdl does not cost a good pair. Raised twice, and it is a real trade
either way: below them, a precondition failure leaves the previous run's files
for the README's diff step to read as current. Kept above, because those two
are regenerable by re-running and being misled by them is not — written down in
the script so it stops being re-decided.

The trailing blank line in tmp/mado.txt, said to make diff tmp/mdl.txt tmp/mado.txt permanently dirty. Both files end with exactly one blank line, so
they cancel: 132 differing lines either way, and 132 again with every blank
stripped from both.

Scope

This reached 122 lines across four review rounds before being cut back. What
came out needs decisions rather than patches, and is tracked:

• setup.sh can empty mado's working tree #399 — the same hazard in benchmarks/setup.sh, which this does not
  touch: its two cds are unquoted and git reset --hard <sha> sits below
  them, so that file wants the whole treatment rather than one check. The
  acceptance side is fixed here — see below.
• The development scripts report results without having run anything #400 — the ways these scripts produce an answer without having run
  anything.
• mado and mdl disagree on 36 of the 52 acceptance fixtures #401 — the comparison this restores does not come out clean: 132
  differing lines (diff | grep -c '^[<>]'; the full diff output is 182,
  the rest being its own separators) over the 52 remaining fixtures. 43 of those are MD007, where
  benchmarks/mado.toml says indent = 4 and mdl defaults to 3 — the one
  parameter in that file that does not match mdl, checked against
  lib/mdl/rules.rb. Not changed here, because that file is also what
  comparison.sh hands mado, so the value is not the acceptance harness's
  alone to pick.
• The acceptance scripts only run on macOS #398 — the remaining SC2086 quoting. The three instances that issue
  opened with are fixed here: the word-splitting loops go through find -exec,
  sed -i '' is gone, and hyperfine's command strings carry their own quotes.

The README gains named prerequisites for the benchmark — hyperfine, mdl,
node, npm, cargo, git, all of which its steps need and none of which it
listed — plus the npm ci those steps left out, and an Acceptance Testing
section — that flow was documented nowhere and
now fails hard without mdl: node_modules is
gitignored and nothing installed it, so the documented setup.sh then
comparison.sh flow could not work on a fresh clone. Developer docs, so no
changelog entry, same as the rest of this.

All four are the same point as #397: shellcheck finds none of them, because
they are facts about git, hyperfine and cargo rather than about shell
syntax.

Two things reviewers raised that are deliberately not here:

• The identical unchecked cd pair in benchmarks/setup.sh. This pull request
  guards the ones in acceptance/setup.sh because changing find to a
  clone-relative path made arriving there newly load-bearing; nothing in the
  benchmark script changed, and setup.sh can empty mado's working tree #399's actual fix replaces those cds rather
  than guarding them. Noted on that issue so the omission is not read as the
  sibling being safe — it is the worse of the two. An earlier revision quoted
  its SCRIPT_DIR on its own; that made things worse, because DATA_ROOT then
  genuinely carried the space that the still-unquoted cd $DATA_ROOT below
  split on. The file wants quoting, both cds guarded, and setup.sh can empty mado's working tree #399's treatment
  together or not at all, so it is back out of this diff.
• What mdl and cargo run report is still unchecked, deliberately: both
  exit non-zero when they find violations, which is the normal case here, so
  gating on their status would abort the comparison this script exists to
  produce. Whether they ran at all is checked; what they found is not.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JWNoFSfDhBzXEX9UL68oqf

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.78%. Comparing base (4fd61ae) to head (c5c3fcf).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #396   +/-   ##
=======================================
  Coverage   99.78%   99.78%           
=======================================
  Files          72       72           
  Lines        7485     7485           
=======================================
  Hits         7469     7469           
  Misses         16       16           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.