The acceptance scripts only run on macOS

[akiomik]

Split out of #396, which fixed the bugs that stopped scripts/acceptance from
working at all. Two things it deliberately left, both outside that diff.

sed -i '' is BSD-only

scripts/acceptance/test.sh truncates its output with

sed -i '' -e "/^Found /d" $TEMP_PATH/mado.txt > /dev/null

GNU sed takes the suffix attached to the flag (-i.bak), so the standalone
'' is read as an input filename and each call fails with
sed: can't read : No such file or directory. #396 made dash -n pass on the
file, but that is only the parser; the script still cannot run on Linux.

Worth deciding rather than patching: either commit to macOS and say so in the
script, or make it portable — a temporary file and mv, or perl -i -pe,
avoids the BSD/GNU split entirely.

While there, the > /dev/null on those three lines does nothing: -i writes
to the file, and the errors that do appear go to stderr.

Quoting

shellcheck reports SC2086 and friends across
scripts/acceptance/*.sh and scripts/benchmarks/*.sh — 32 findings after
#396. None misbehaves today, because no fixture filename contains a space or a
glob character, and find output is word-split by the for loop regardless,
so quoting alone would not make that loop robust.

This is the mechanical half deferred from #396, and it should land with
whatever linter enforces it rather than before. Whether that linter is
shellcheck is part of #397.

[akiomik]

#399 and #400, split out of #396, are the limit of what a linter answers here. shellcheck's only finding on the code in #399 is SC2164, and taking that advice produces the guard that does not work. Neither issue argues against adopting shellcheck — it would still have caught the SC2086 and SC3030 classes — but it does mean the linter is not the fix for those two.

[akiomik]

One concrete instance of the SC2086 class, from reviewing #396:

scripts/acceptance/setup.sh builds its work list as
style_files=$(find test/rule_tests -name '*_style.rb') and iterates
for style_file in $style_files. That expansion is subject to word splitting
and pathname expansion, so a fixture whose name contains whitespace or a glob
character derives the wrong document name, falls through to the "no document
for" branch, and leaves the real fixture in the corpus — the thing the loop
exists to prevent. The non-empty guard added in #396 cannot catch it, because
$style_files is not empty in that case.

No upstream name triggers this today (all 37 checked), so it is hardening
rather than a live bug. Worth noting because quoting alone does not fix it:
for over a captured find splits regardless, so the real repair is
restructuring to find -exec or while read, which is a rewrite of the loop
rather than a pair of quotes. That distinction is worth keeping in view when
deciding what adopting a linter here actually buys.

[akiomik]

Follow-up to my note above, from the last review of #396: that pull request added mv ... || exit 1, which changes what the unquoted for style_file in $style_files would cost. A split fragment used to produce one failed mv and carry on; now it aborts setup.sh. Aborting is the better failure of the two, but it means the word-splitting turns a fixture name with a space into a setup that cannot run at all rather than one bad iteration — worth knowing when deciding how to restructure that loop.

[akiomik]

Two refinements to the for style_file in $style_files note above, from the
last review of #396, both about how it fails rather than whether:

• It fails silently and wrongly, not loudly. A fixture named
  my doc_style.rb splits into two iterations, both of which print
  no document for … and neither of which sets the document aside — so
  my doc.md, a parameter-sensitive fixture, stays in the corpus and gets
  compared. Exit status is 0 whenever at least one other fixture was set aside,
  so the counter guard fix: repair the acceptance and benchmark shell scripts #396 added does not catch it either.
• Quoting genuinely does not fix it. find … -print0 | while IFS= read -r -d ''
  or find … -exec sh -c is the repair, which is a restructure of the loop.

Also confirmed on the sed -i '' item: the > /dev/null on those three lines
is a no-op, since sed -i writes in place and prints nothing on stdout.

[akiomik]

Correction to my note above, after testing it rather than reasoning about it.

A review of #396 suggested that the unquoted $style_files also globs, so a
fixture named with a * or a [ would make the loop set aside "an arbitrary
file", and that set -f would close that half. I could not reproduce it, and
on inspection it cannot happen: every field find produces ends in
_style.rb, so pathname expansion of that field can only match files that are
themselves *_style.rb under test/rule_tests — exactly the set find already
returned. Globbing therefore yields duplicate iterations, and duplicates are
idempotent because the second pass sees the .bak and skips. Verified with a
file literally named *_style.rb alongside real fixtures: identical results
with and without set -f.

So set -f was tried in #396 and taken back out — it changes nothing reachable
here, and a line whose stated purpose does not hold is worse than no line.

The word-splitting half stands as originally reported: a fixture name with a
space really does split into fragments, leaves the document in the corpus, and
exits 0. That is the part find -print0 | while IFS= read -r -d '' or
find -exec fixes, and it is why quoting alone is not the answer.