mirror of
https://github.com/Klipper4FlashForge/firmware.git
synced 2026-10-22 05:12:16 -05:00
Merge of qa-suite-bridgehead into the packaging line. The seven init.d/S*
scripts and the hand-written etc/s6 servicedirs are gone; anvil-core ships
etc/s6-rc/source/ and bin/patch.sh compiles it into etc/s6-rc/compiled/ after
the payload is installed. klipperDaemon joins firmwareExe and start.sh in
payload/prog/, all three linked into place by anvil-link-prog.sh.
WHAT IS KNOWN BROKEN. qa/replica/test_s6rc.py fails: the tree never comes up,
because s6-rc-init dies with
unable to supervise service directories in /run/s6-rc/servicedirs:
Value too large for defined data type
That is EOVERFLOW out of the readdir() loop in s6rc_servicedir_manage.c, which
checks errno after every entry. Measured, and these are ruled out:
* not the live directory's filesystem -- fails identically with -l on
/usr/data as with the default /run, and the inodes there are 3 and
1358496, nowhere near 32 bits
* not the database -- s6-rc-db lists all eleven services out of it, and a
database recompiled inside the replica by the TARGET s6-rc-compile fails
exactly like the one compiled natively on x86
* not a missing -D_FILE_OFFSET_BITS=64 -- both s6-svscan and s6-rc-init
reference readdir64 and never plain readdir
What is left is the toolchain. qa-suite-bridgehead built s6, s6-rc, execline
and skalibs with the Bootlin musl toolchain, where readdir has no EOVERFLOW
path at all; the packaging line builds them as recipes against the Ingenic
glibc, where it does. s6-svscan looked healthy only because it was started on
an EMPTY scandir, which never reaches the loop.
Everything else is green: 262 static tests, and bin/verify.sh passes on the
73MB package, including the graph's own "compiled s6-rc database" check.
Resolution notes: the five MOD_* switches stay deleted -- firmwareExe defaults
every gate to 1, so nothing changes -- but MOD_CAM is now worth having by hand
on a cameraless printer, because the camera is a longrun that waits out
timeout-up. anvil-service.sh went with init.d; nothing sourced it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VpUmBb4xtmnmtTosonqeNi
307 lines
14 KiB
Python
307 lines
14 KiB
Python
"""Every script parses, and none of them is secretly bash.
|
|
|
|
Ported from test/run-tests.py's check_shell_syntax and check_no_bashisms. The
|
|
logic is theirs; what changes is the granularity. There, one gate covered ~25
|
|
files and reported one bit, so a syntax error in S62moonraker and a syntax
|
|
error in patch.sh were the same red line. Here each file is its own test, named
|
|
after itself, and `pytest -k S62moonraker` runs exactly one.
|
|
|
|
The original's docstring made a deliberate choice worth preserving:
|
|
|
|
One line per check, not one per file: 25 green lines saying "syntax ok"
|
|
hide the two that matter.
|
|
|
|
That reasoning was about output noise, and it was right. pytest solves it a
|
|
different way -- passes collapse to dots, and only failures print -- so the
|
|
granularity and the quiet are no longer in tension.
|
|
|
|
WHY THE PAYLOAD IS CHECKED TWICE, WITH DIFFERENT DIALECTS
|
|
|
|
`bash -n` proves a file parses at all. shellcheck's dash dialect proves it uses
|
|
nothing bash-only, which is what matters for anything a recipe ships: the
|
|
printer runs busybox ash, not bash, and a bash-only construct parses perfectly
|
|
here and fails there. dash rather than sh as the dialect because busybox ash,
|
|
like dash, supports `local`, which the payload uses.
|
|
"""
|
|
import importlib.util
|
|
import os
|
|
import shutil
|
|
import subprocess
|
|
import sys
|
|
|
|
import pytest
|
|
|
|
from lib.paths import ROOT
|
|
|
|
pytestmark = pytest.mark.static
|
|
|
|
|
|
# ------------------------------------------------------------------ the sets
|
|
|
|
# Everything that must at least parse. bin/ and test/ run under bash on the
|
|
# build image; the printer's scripts run under its ash but bash parses them
|
|
# too, and a file that does not parse under either is broken beyond dialect.
|
|
#
|
|
# THE PRINTER'S SCRIPTS ARE ADDRESSED BY THE SHAPE OF A RECIPE, not by a
|
|
# top-level directory: pkgs/<recipe>/payload/ is what that recipe installs
|
|
# under $MODDIR, and pkgs/<recipe>/payload/prog/ is the part of that which
|
|
# anvil-link-prog.sh points the stock /usr/prog paths at. Neither is named
|
|
# recipe by recipe here, so a new recipe with scripts in it is covered the day
|
|
# it lands.
|
|
#
|
|
# The s6-rc `run` scripts are in the list because s6-supervise execs them on
|
|
# the printer, under the printer's ash, and a bashism in one is a service that
|
|
# never starts and says so only in s6's own log.
|
|
SYNTAX_GLOBS = ("bin/*.sh", "pkgs/*/payload/*.sh",
|
|
"pkgs/*/payload/prog/*.sh", "pkgs/*/payload/prog/firmwareExe",
|
|
"pkgs/*/payload/prog/klipperDaemon",
|
|
"pkgs/*/payload/etc/s6-rc/" + "source/*/run",
|
|
"installer/*.sh",
|
|
"test/integration/printer/*.sh",
|
|
"qa/replica/actions/*.sh",
|
|
# BOTH RECIPE LEVELS. pkgs/*/build.sh alone silently
|
|
# stopped covering thirty-four of the thirty-eight the day
|
|
# they moved under 3rdparty/ -- a glob that matches fewer
|
|
# files does not fail, it just checks less. _files() now
|
|
# refuses a pattern that matches nothing, so the next one
|
|
# fails instead of quietly shrinking.
|
|
"pkgs/*.sh", "pkgs/*/build.sh", "pkgs/3rdparty/*/build.sh")
|
|
|
|
# NOT the oneshots' `up`/`down`. Those are execline command lines, and the
|
|
# shell they carry is inside a `/bin/sh -c "..."` argument -- `bash -n` and
|
|
# shellcheck would both parse the wrapper, pass, and check none of the code
|
|
# that actually runs. Extracting the body and checking THAT belongs in the
|
|
# replica lane beside the rest of the s6-rc assertions.
|
|
|
|
# The subset executed by the printer's busybox ash. bin/ and test/ are
|
|
# deliberately absent: they run on the build image, where bash is the shell and
|
|
# a bashism is not a defect.
|
|
#
|
|
# qa/replica/actions/ IS here, and that is not a formality. Those scripts are
|
|
# handed to entrypoint.sh and run inside the chroot exactly as a case script
|
|
# is, so they are as exposed to ash as anything under payload/. A bashism in
|
|
# hold.sh would fail at container start and read as "the replica is broken on
|
|
# this machine" -- a harness failure wearing a machine failure's clothes, which
|
|
# is the hardest kind to diagnose.
|
|
# pkgs/*/build.sh is deliberately not here: the recipes are build-host bash,
|
|
# like everything in bin/, and a bashism in one is not a defect.
|
|
#
|
|
# installer/ is here too: run-pre.sh and run-append.sh are spliced into
|
|
# FlashForge's own run.sh and execute inside the stock installer's shell,
|
|
# which is the same busybox.
|
|
#
|
|
# NOT here, and it is a real gap rather than a decision:
|
|
# pkgs/*/payload/bin/*.sh. wifi-action.sh runs on the printer and no lane
|
|
# checks its dialect.
|
|
ASH_GLOBS = ("pkgs/*/payload/*.sh",
|
|
"pkgs/*/payload/prog/*.sh", "pkgs/*/payload/prog/firmwareExe",
|
|
"pkgs/*/payload/prog/klipperDaemon",
|
|
"pkgs/*/payload/etc/s6-rc/" + "source/*/run",
|
|
"installer/*.sh", "qa/replica/actions/*.sh")
|
|
|
|
PY_GLOBS = ("bin/*.py", "pkgs/*/payload/bin/*.py",
|
|
"pkgs/*/payload/klipper/klippy/extras/*.py", "test/*.py", "test/ffsim/*.py",
|
|
"test/integration/*.py", "qa/*.py", "qa/lib/*.py",
|
|
"qa/static/*.py", "qa/replica/*.py")
|
|
|
|
|
|
def _files(globs):
|
|
"""Expand the globs, and refuse one that matches nothing.
|
|
|
|
A glob that stops matching does not fail -- it checks less, and the lane
|
|
stays green while doing it. That is not hypothetical: moving thirty-four
|
|
recipes under pkgs/3rdparty/ left "pkgs/*/build.sh" matching the four that
|
|
had not moved, and the only visible sign was a test count dropping by
|
|
thirty-four in a run nobody was counting.
|
|
|
|
So an empty pattern is a collection error, which is as loud as this file
|
|
can be, and the message says the two things that cause it: a file that
|
|
moved, or a pattern that was never right.
|
|
"""
|
|
found = []
|
|
for pattern in globs:
|
|
hits = sorted(p for p in ROOT.glob(pattern) if p.is_file())
|
|
if not hits:
|
|
raise AssertionError(
|
|
"%r matches no files -- either something moved and this "
|
|
"pattern did not follow it, or the pattern is wrong. Either "
|
|
"way this lane was checking less than it claimed." % pattern)
|
|
found += hits
|
|
return found
|
|
|
|
|
|
def _ids(paths):
|
|
return [str(p.relative_to(ROOT)) for p in paths]
|
|
|
|
|
|
SHELL_FILES = _files(SYNTAX_GLOBS)
|
|
ASH_FILES = _files(ASH_GLOBS)
|
|
PY_FILES = _files(PY_GLOBS)
|
|
|
|
|
|
# --------------------------------------------------------- the globs are live
|
|
#
|
|
# Each lane below asserts its own inputs exist. This is not defensive noise: it
|
|
# is the exact bug the original guarded against, quoted from check_no_bashisms:
|
|
#
|
|
# With no targets shellcheck writes usage to stderr and exits 1, leaving
|
|
# `hits` empty -- a green gate that examined nothing. check_shell_syntax
|
|
# guards this; this one did not, so moving payload/ would have retired the
|
|
# ash-compatibility check silently.
|
|
#
|
|
# Parametrized tests fail the same way, and worse: zero parameters is zero
|
|
# tests, which is not even a red line. It is an empty run that reads as clean.
|
|
|
|
def test_shell_globs_match_something():
|
|
assert SHELL_FILES, "no scripts to parse -- the globs in SYNTAX_GLOBS rotted"
|
|
|
|
|
|
def test_ash_globs_match_something():
|
|
assert ASH_FILES, "no on-printer scripts -- has the recipe layout moved?"
|
|
|
|
|
|
def test_python_globs_match_something():
|
|
assert PY_FILES, "no python files -- have the globs in PY_GLOBS rotted?"
|
|
|
|
|
|
ACTIONS = sorted((ROOT / "qa" / "replica" / "actions").glob("*.sh"))
|
|
|
|
|
|
@pytest.mark.parametrize("script", ACTIONS, ids=_ids(ACTIONS))
|
|
def test_action_scripts_are_executable(script):
|
|
"""qa/replica/actions/*.sh are handed to docker, not sourced.
|
|
|
|
bake.sh is a `--entrypoint` and the others are mounted in and run, so a
|
|
missing executable bit is not a style problem -- the container dies at
|
|
init with `exec: permission denied` before a single test runs, and the
|
|
43 errors that follow all say "could not install the package", which
|
|
points at the installer rather than at a file mode.
|
|
|
|
Caught here because it happened: bake.sh was written without the bit and
|
|
the whole lane failed in 1.6 seconds with an error about the mod. It is
|
|
the same failure the payload's own s6 `run` scripts have a check for --
|
|
"s6 reports a non-executable run in its own log and nowhere else" -- and
|
|
it deserved the same treatment here.
|
|
"""
|
|
assert os.access(str(script), os.X_OK), (
|
|
"%s is not executable, so docker cannot run it. `chmod +x` it and "
|
|
"commit the mode." % script.name)
|
|
|
|
|
|
# ------------------------------------------------------------------- it parses
|
|
|
|
@pytest.mark.parametrize("script", SHELL_FILES, ids=_ids(SHELL_FILES))
|
|
def test_parses(script):
|
|
parsed = subprocess.run(["bash", "-n", str(script)],
|
|
capture_output=True, text=True)
|
|
assert parsed.returncode == 0, parsed.stderr.strip()
|
|
|
|
|
|
# ------------------------------------------------------ it is not secretly bash
|
|
|
|
@pytest.fixture(scope="session")
|
|
def bashisms():
|
|
"""shellcheck over every payload script at once, indexed by file.
|
|
|
|
One process, not one per file. shellcheck spends most of a run starting
|
|
up, so per-file invocation cost more than everything else in this lane
|
|
combined -- but per-file REPORTING is the whole point of the port. The
|
|
gcc output format is `path:line:col: level: message [SCxxxx]`, so the
|
|
findings can be split back apart by path afterwards and each test can ask
|
|
only about itself.
|
|
"""
|
|
# A failure, not a skip. There is no configuration in which running this
|
|
# suite without shellcheck is a complete run, so calling it "skipped" would
|
|
# describe a machine that is not set up as though it were a decision
|
|
# somebody made. See qa/conftest.py.
|
|
#
|
|
# pytest.fail rather than a bare assert on which(): `assert None` is what
|
|
# an assert would lead with, and the sentence explaining how to fix it
|
|
# would be the part that got truncated in the summary.
|
|
if not shutil.which("shellcheck"):
|
|
pytest.fail(
|
|
"shellcheck is not installed, so nothing checked that the payload "
|
|
"is free of bash-only constructs -- the printer runs busybox ash. "
|
|
"The build image has it: run `make qa`, or install it locally "
|
|
"(dnf install ShellCheck / apt install shellcheck).")
|
|
|
|
checked = subprocess.run(
|
|
["shellcheck", "-s", "dash", "-f", "gcc"]
|
|
+ [str(p) for p in ASH_FILES],
|
|
capture_output=True, text=True, cwd=str(ROOT))
|
|
|
|
# SC3xxx is shellcheck's "not supported in POSIX sh" family; SC2039 is its
|
|
# older spelling of the same thing. Everything else it says here is style,
|
|
# and a gate that goes red for style gets switched off.
|
|
found = {}
|
|
for line in (checked.stdout or "").splitlines():
|
|
if "SC3" not in line and "SC2039" not in line:
|
|
continue
|
|
path = line.split(":", 1)[0]
|
|
found.setdefault(path, []).append(line)
|
|
|
|
# shellcheck exits 1 for findings, which `found` covers, but also for a
|
|
# usage or file error -- which would otherwise pass unnoticed as "no
|
|
# findings". Raised here rather than returned so it fails every test in
|
|
# the lane instead of hiding behind whichever file happens to be clean.
|
|
if not found and checked.returncode not in (0, 1):
|
|
raise AssertionError(
|
|
"shellcheck failed to run (exit %d): %s"
|
|
% (checked.returncode, (checked.stderr or "").strip()[:200]))
|
|
return found
|
|
|
|
|
|
@pytest.mark.parametrize("script", ASH_FILES, ids=_ids(ASH_FILES))
|
|
def test_no_bash_only_constructs(script, bashisms):
|
|
# shellcheck echoes back the path it was given. It was given absolute
|
|
# paths, but match on both spellings so a future cwd-relative call site
|
|
# does not silently stop matching and report every file clean.
|
|
rel = str(script.relative_to(ROOT))
|
|
hits = bashisms.get(str(script), []) + bashisms.get(rel, [])
|
|
assert not hits, "bash-only constructs:\n " + "\n ".join(hits)
|
|
|
|
|
|
# ------------------------------------------------------- every name is bound
|
|
|
|
# One test, not one per file: pyflakes resolves names within a module, so the
|
|
# per-file split buys nothing here and costs an interpreter start each. The
|
|
# failure message names the files itself.
|
|
def test_no_undefined_names():
|
|
"""A name that does not exist anywhere it is read.
|
|
|
|
py_compile catches typos in syntax; nothing caught typos in names, so a
|
|
rename that renamed an assignment and missed one of its uses stayed green
|
|
through both lanes and waited for the line to execute on the printer. That
|
|
is how `_plate_check` came to reach for `z_trigger` when what it had bound
|
|
was `station_z` -- on the one path that runs BEFORE a nozzle is allowed to
|
|
descend.
|
|
"""
|
|
# The debian package ships the module without putting a pyflakes
|
|
# executable on PATH, so shutil.which() -- the shape the shellcheck test
|
|
# above uses -- reports it missing on the very image that has it.
|
|
#
|
|
# A failure, not a skip, for the same reason as shellcheck above.
|
|
if importlib.util.find_spec("pyflakes") is None:
|
|
pytest.fail(
|
|
"pyflakes is not installed, so nothing checked that every name is "
|
|
"bound -- which is the gate that catches a rename missing one of "
|
|
"its uses. The build image has it: run `make qa`, or install it "
|
|
"locally (apt install python3-pyflakes / pip install pyflakes).")
|
|
|
|
checked = subprocess.run(
|
|
[sys.executable, "-m", "pyflakes"] + [str(p) for p in PY_FILES],
|
|
capture_output=True, text=True, cwd=str(ROOT))
|
|
|
|
# Only the fatal findings count. pyflakes also reports unused imports and
|
|
# the like, which are untidy but cannot fail at runtime.
|
|
fatal = ("undefined name", "referenced before assignment",
|
|
"syntax error", "invalid syntax")
|
|
hits = [line for line in (checked.stdout or "").splitlines()
|
|
if any(marker in line.lower() for marker in fatal)]
|
|
|
|
assert hits or checked.returncode in (0, 1), (
|
|
"pyflakes failed to run (exit %d): %s"
|
|
% (checked.returncode, (checked.stderr or "").strip()[:200]))
|
|
|
|
assert not hits, "names used but never bound:\n " + "\n ".join(hits)
|