diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c439207..c7cf035 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,7 +71,7 @@ jobs: # shellcheck could report anything it liked and the job stayed green. # payload scripts run under busybox ash on the printer. shellcheck -s sh -S error pkgs/*/payload/*.sh pkgs/*/payload/init.d/S* pkgs/*/prog/*.sh pkgs/*/prog/firmwareExe installer/*.sh - shellcheck -s bash -S error bin/*.sh test/integration/*.sh + shellcheck -s bash -S error bin/*.sh tools/replica/*.sh # Warnings are advisory: printed, not enforced. shellcheck -s sh -S warning pkgs/*/payload/*.sh pkgs/*/payload/init.d/S* pkgs/*/prog/*.sh pkgs/*/prog/firmwareExe installer/*.sh || true diff --git a/Makefile b/Makefile index 3dcd52c..2c206a3 100644 --- a/Makefile +++ b/Makefile @@ -233,7 +233,7 @@ verify: image # it only exists inside the stock package's kernel component. rootfs: image config.env @$(RUN) ./bin/unpack.sh >/dev/null - @$(RUN) ./test/integration/extract-rootfs.py + @$(RUN) ./tools/replica/extract-rootfs.py # `make test` used to be here, running test/run-tests.py -- a bespoke harness # that wrapped pytest, re-parsed its JUnit XML, and drove thirteen replica case @@ -294,10 +294,10 @@ qa-replica: image # # The image contains proprietary FlashForge firmware. printer-image: image - @$(RUNSIM) ./test/integration/build-printer-image.sh + @$(RUNSIM) ./tools/replica/build-printer-image.sh printer-image-push: image - @$(RUNSIM) ./test/integration/build-printer-image.sh --push + @$(RUNSIM) ./tools/replica/build-printer-image.sh --push # The Python gate. The checks that read the printer's rootfs skip without one; # everything else runs on any checkout. @@ -310,7 +310,7 @@ boot-screen: # The same frames, drawn by the PRINTER's python inside the replica. Slower, # and the only render that proves what the panel would really show. boot-screen-sim: image - @$(RUNSIM) ./test/integration/sim-boot-screen.py + @$(RUNSIM) ./tools/replica/sim-boot-screen.py clean: @rm -rf work/stage work/out work/modpayload-root diff --git a/bin/build-payload.py b/bin/build-payload.py index 20dc895..6333756 100755 --- a/bin/build-payload.py +++ b/bin/build-payload.py @@ -22,14 +22,14 @@ from pathlib import Path for _p in Path(__file__).resolve().parents: if (_p / "bin" / "common.sh").is_file(): ROOT = _p - sys.path.insert(0, str(_p / "test")) + sys.path.insert(0, str(_p / "tools" / "replica")) break from ffsim import cli # noqa: E402 from ffsim.config import Config # noqa: E402 from ffsim.replica import Replica # noqa: E402 -CASE = "test/integration/printer/case-build-payload.sh" +CASE = "tools/replica/printer/case-build-payload.sh" def _sh(var, default=""): diff --git a/docs/building.md b/docs/building.md index 0cb1664..3cf0499 100644 --- a/docs/building.md +++ b/docs/building.md @@ -315,33 +315,33 @@ pkgs/ package recipes: one directory per component, each a ``` qa/ THE SUITE. static/ needs nothing but a checkout; replica/ needs docker and the firmware. `make qa` -test/ the shared pytest fixtures, the host-side tests of our own - Python, and the machinery that builds the replica - ffsim/ the host half of the replica, as a python package: config - loading and the docker plumbing. Not a test framework any - more -- see docs/qa-migration.md - integration/ the host-side tests. `make test-py` - test_startup.py bin/ff-startup.py -- the whole first boot - test_ffscreen.py the lines the first boot puts on /dev/fb0 - test_tool_transform.py the per-tool G-code frame in ff_toolchange - test_chamber.py the Klipper config gate -- no firmware needed - test_gcode.py gcode/*.gcode against the macros we define - printer/ the replica itself: binfmt, mount layout, its two - Dockerfiles, and the entrypoint that runs inside it on - the printer's own binaries -- SHELL, because the printer's - busybox ash is the only interpreter that matters there - extract-rootfs.py pulls the real rootfs out of the stock package - sim-boot-screen.py renders the boot frames, via the docker socket - build-printer-image.sh bakes a prebuilt replica image +test/ 28 host-side unit tests over our own Python, and the two + fixtures they share. `make test-py` + conftest.py the merged config tree the config gates read + ffcfg.py Klipper config parsing, as klippy's own parser sees it + integration/ test_startup.py, test_tool_transform.py, test_ffscreen.py, + test_chamber.py, test_gcode.py -- see docs/testing.md + test-chelper.py in neither suite: bin/patch.sh and bin/verify.sh call it +tools/replica/ THE REPLICA, which is a build tool as much as a test one -- + bin/patch.sh assembles the payload inside it + printer/ the machine: binfmt, the mount layout, its two Dockerfiles + and the entrypoint that runs inside it on the printer's own + binaries -- SHELL, because the printer's busybox ash is the + only interpreter that matters there + ffsim/ the host half: config loading and the docker plumbing + extract-rootfs.py pulls the real rootfs out of the stock package + sim-boot-screen.py renders the boot frames, via the docker socket + build-printer-image.sh bakes a prebuilt replica image test.env replica settings only -- factory image, partition sizes docs/ the documentation ``` -Nothing left under `test/` needs the firmware: the config gate needs only -python3 and jinja2, and the rest test our own Python against fakes. It briefly -had a directory of its own, `test/unit`, so that a pull request had something -to run; with one maintainer who always has the firmware, that was a boundary -kept in sync for nobody. Everything that needs a real machine is in `qa/`. +Nothing left under `test/` needs the firmware, or docker, or a network: the +config gates need python3 and jinja2, and the rest exercise our own Python +directly. It briefly had a directory of its own, `test/unit`, for exactly that +distinction; with one maintainer who always has the firmware, that was a +boundary kept in sync for nobody. Everything that needs a real machine is in +`qa/`, and the machine itself is in `tools/replica/`. The line between Python and shell here is not taste. Everything that runs on YOUR machine is Python; everything executed by the printer's own busybox under diff --git a/docs/printer-replica.md b/docs/printer-replica.md index fca0090..02eb665 100644 --- a/docs/printer-replica.md +++ b/docs/printer-replica.md @@ -1,6 +1,6 @@ # The printer replica -Everything in `test/integration/printer/` exists to answer one question honestly: **would +Everything in `tools/replica/printer/` exists to answer one question honestly: **would this package brick the machine?** A Debian container with hand-written stand-ins cannot answer it. The stock @@ -20,7 +20,7 @@ work/rootfs/ rootfs.squashfs, extracted from the stock package's (busybox 1.31.1, glibc 2.33, MIPS32r2 nan2008, mipsel) ``` -`test/integration/printer-exec.py` starts a privileged container that: +`tools/replica/ffsim/replica.py` starts a privileged container that: 1. registers a `binfmt_misc` handler so MIPS binaries execute (see below), 2. builds the machine's mount layout, @@ -83,7 +83,7 @@ the Ingenic toolchain produced for this printer has `3` there: So the stock handler never matched, and every attempt to run a printer binary came back `exec format error` — with qemu installed, registered and enabled. -`test/integration/printer/binfmt.sh` registers a handler with that byte masked out. +`tools/replica/printer/binfmt.sh` registers a handler with that byte masked out. Everything else is the stock mipsel matcher. The handler is registered with the `F` (fix-binary) flag, so the kernel holds @@ -115,7 +115,7 @@ Two things supply it to the replica, and there is no third: There used to be a third: stubs, for whatever neither of those provided. They are gone. A green run that came from a hand-written `python3` and an OpenSSL 3 -pretending to be 1.0.2d is reassurance, not evidence — `test/integration/printer/seed-prog.sh` +pretending to be 1.0.2d is reassurance, not evidence — `tools/replica/printer/seed-prog.sh` now hard-fails when the prog partition is not real. A full-filesystem factory image works too, and one is published: diff --git a/docs/qa-migration.md b/docs/qa-migration.md index 53fb666..97a0b5d 100644 --- a/docs/qa-migration.md +++ b/docs/qa-migration.md @@ -2,8 +2,8 @@ `qa/` is the test suite. It was built beside `test/` rather than inside it, and the migration is now done: every `case-*.sh` has been ported or retired, and -what remains of `test/` is 136 host-side pytest tests and the machinery that -builds the replica. +what remains of `test/` is 28 host-side unit tests. The machinery that builds +the replica has moved to `tools/replica/`, because it was never a test. This document says why it exists, what it proves, and what is still owed. @@ -236,7 +236,7 @@ this tree: |---|---| | `qa/static` | **246 tests, 8s**, no docker | | `qa/replica` | 118 tests, 3m40s -- 110 pass, 8 blocked on absent hardware | -| `test/` pytest | 136 tests, 0.3s | +| `test/` pytest | 28 tests, 0.1s | `test/run-tests.py` is **322 lines** and `test/ffsim/` **483**, from 1,657 between them. What is left of the old harness runs the packaging build on a @@ -482,20 +482,25 @@ a stock package. ### What survives in test/ -- **`test/integration/test_*.py`** -- 136 host-side tests in five files. - Already pytest, and adoptable by pointing `qa/` at them whenever that seems - worth doing. Eight files were dropped after the migration; see *The second - cut* below. -- **`test/ffsim/`** -- 678 lines, down from 1,657, and no longer a test - framework. `extract_rootfs` is what `make rootfs` runs; `Replica` and the - python-tarball helper are what `make boot-screen-sim` uses to render the boot - frames with the cross-built interpreter. -- **`test/integration/printer/`** -- the `Dockerfile`, `entrypoint.sh`, - `assemble.sh`, `binfmt.sh` and `seed-prog.sh` that BUILD the replica. `qa/` - uses them unmodified; they were never the problem. +- **`test/integration/test_*.py`** -- 28 host-side unit tests in five files, + down from 186; see *The second cut* below. - **`test/test-chelper.py`** -- in neither suite. `bin/patch.sh` and `bin/verify.sh` call it directly at build time. +And two things that were under `test/` and are not tests, now at +**`tools/replica/`**: + +- **`ffsim/`** -- 678 lines, down from 1,657, and no longer a test framework. + `extract_rootfs` is what `make rootfs` runs; `Replica` is what + `make boot-screen-sim` and, more to the point, **`bin/patch.sh`** use -- + the payload is assembled by running the printer's own `opkg` inside a + replica, so this is build-path code that happened to live in the test tree. +- **`printer/`** -- the `Dockerfile`, `entrypoint.sh`, `assemble.sh`, + `binfmt.sh` and `seed-prog.sh` that BUILD the replica. `qa/lib/replica.py` + uses them unmodified; they were never the problem. Moving them out of + `test/` changed no content, only `qa/lib/replica.py`'s build directory and + three launcher paths. + ### The second cut The migration left `test/` at 186 tests across twelve files. Seven of those @@ -516,6 +521,30 @@ The four rows in bold-ish -- the executable bit, the include set, the rootfs paths -- are static questions that a replica cannot answer any better, so if they come back they belong in `qa/static`, not here. +The five survivors were then cut from 136 tests to 28 -- the 20% worth +keeping -- on one question: **would this failure be silent?** + +| file | was | now | what the survivors are | +|---|---|---|---| +| `test_startup.py` | 49 | 8 | the stamp discipline, and the handover order. A stamp written before the values are verifiably saved is a printer that has lost its factory calibration for good, and no later boot will try again | +| `test_ffscreen.py` | 33 | 5 | the four ways it can do harm -- scribble outside the buffer, scribble a format it does not understand, raise into the migration, or turn the picture | +| `test_tool_transform.py` | 24 | 7 | every kept test is a move to the WRONG PLACE: a sign, a frame left applied while probing, a stale gcode_move cache, a klippy that will not parse its config on a calibrated printer | +| `test_chamber.py` | 20 | 4 | the three chamber mistakes that are silent until they have cost a print, plus the parse gate they rest on | +| `test_gcode.py` | 10 | 4 | a renamed macro, and the safe file's promise that it is cold and stays above Z50 | + +What went with them, in one sentence each: the panel narration (words a person +reads while waiting, and `qa/replica/test_boot_screen.py` renders the real +thing on the real machine); the geometry probe and `parse_geometry` (a panel +that looks wrong, not a printer that breaks); `z_adjust` staging and the +per-job Z term (wrong numbers reported to a person); the config layout checks +(a config that does not load, which the printer says loudly the first time); +and the G-code bounds checks (Klipper refuses the move at the first offending +line, before the toolhead has gone anywhere). + +Each of the five files says in its own header what it dropped and why. That is +deliberate: a test file that has been cut hard should say so where the next +person opens it, not only in a document they may not read. + ### Still owed - **The build path**, above. The largest gap. Its fixture generator, diff --git a/docs/testing.md b/docs/testing.md index 661cc85..75b1443 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -45,28 +45,51 @@ unreachable. See [qa-migration.md](qa-migration.md). Everything else is green. ## What is left in `test/` `test/run-tests.py` is **gone**, and `make test` with it. What remains is not a -suite: +suite -- it is 28 host-side unit tests over our own Python, which need neither +docker nor the firmware: ```sh -make test-py # the host-side pytest tree +make test-py # 28 tests, well under a second ``` -- **`test/integration/test_*.py`** -- 185 host-side tests over the Klipper - configs, the gcode, `ff-startup.py`, `ffscreen.py`, the toolchange transform - and the offset sampling. Already pytest, already well-seamed; they were never - part of the migration and can be adopted by pointing `qa/` at them. -- **`test/ffsim/`** -- 678 lines and no longer a framework. `extract_rootfs` is - what `make rootfs` runs; `Replica` is what `make boot-screen-sim` uses. -- **`test/integration/printer/`** -- the `Dockerfile`, `entrypoint.sh`, - `assemble.sh`, `binfmt.sh` and `seed-prog.sh` that BUILD the replica. `qa/` - uses them unmodified; they were never the problem. -- **`test/test-chelper.py`** -- in neither suite: `bin/patch.sh` and - `bin/verify.sh` call it directly at build time. +| file | tests | what it is for | +|---|---|---| +| `test_startup.py` | 8 | the stamp discipline in `ff-startup.py`: no stamp is written unless the values are verifiably saved, because a stamp written early is a printer that has silently lost its factory calibration for good. Plus the handover order -- the boards, then klipper, again on every retry | +| `test_tool_transform.py` | 7 | the per-tool frame arithmetic in `ff_toolchange`, whose failures are a nozzle in the wrong place | +| `test_ffscreen.py` | 5 | restraint: never write outside the framebuffer, never guess at a pixel format, never raise into the migration it decorates | +| `test_chamber.py` | 4 | the chamber macros branching on the model, whose failures are silent until they have cost a print | +| `test_gcode.py` | 4 | the two `gcode/` files name commands we ship, and the safe one is cold and stays above its floor | -> **The build path is not tested.** The packaging build on a synthetic stock -> package -- `unpack`/`patch`/`pack`/`verify` -- went with `run-tests.py` and -> has no counterpart in `qa/`. That is the path producing the `.tgz` a user -> flashes. See [qa-migration.md](qa-migration.md). +This was 186 tests in twelve files as recently as the qa migration. The cut to +28 was deliberate and is recorded in +[qa-migration.md](qa-migration.md#the-second-cut): what went was everything +whose subject had become the fake it was built on, or whose failure the printer +reports loudly the first time. What is kept is the mistakes that RUN +PERFECTLY and are wrong. + +`test/test-chelper.py` sits outside both suites: `bin/patch.sh` and +`bin/verify.sh` call it directly at build time. + +## The replica is a tool, not a test + +`tools/replica/` builds the machine both suites run against, and the build +uses it too -- `bin/patch.sh` assembles the payload by running the printer's +own `opkg` inside it. It is not under `test/` because nothing in it is a test: + +``` +tools/replica/ + printer/ the replica itself: the Dockerfile, binfmt.sh, + assemble.sh, entrypoint.sh, seed-prog.sh, bake.sh + ffsim/ the host half -- config loading and the docker + plumbing. NOT a test framework; it was one, and + what is left of it launches containers + build-printer-image.sh bakes a prebuilt replica image + extract-rootfs.py `make rootfs` + sim-boot-screen.py `make boot-screen-sim` +``` + +`qa/lib/replica.py` drives `tools/replica/printer/` unmodified; it holds a +container open where `ffsim` runs one case script and exits. ## Why the harness is Python @@ -122,7 +145,7 @@ Almost all of a replica run used to be setup, repeated per test case: | **the test itself** (three boots, install, re-install) | ~30s | `make printer-image` does both of those once, at build time, and publishes the -result. `test/integration/printer/bake.sh` is the part that cannot be a `docker build` +result. `tools/replica/printer/bake.sh` is the part that cannot be a `docker build` step — the stock install needs `binfmt_misc` and `chroot`, so it runs in a privileged container and the result is committed. The md5 of the package that was installed is recorded in `/usr/prog/.BASELINE`; `entrypoint.sh` reinstalls @@ -184,7 +207,7 @@ Tests that cannot fail are worse than no tests, because they read as coverage: - **The hand-rolled bashism grep in `run-tests.sh`.** It knew five constructs; `shellcheck -s dash` knows the whole SC3xxx family and now stands in its place. The same pass gave every replica launcher one shared home for the - docker plumbing — now `test/ffsim/replica.py` — which also fixed two `make + docker plumbing — now `tools/replica/ffsim/replica.py` — which also fixed two `make test-ash` bugs: it ran without the docker socket (so it always silently skipped), and it never read `test.env`, so it ignored `PRINTER_IMAGE` and rebuilt the local sim image every run. diff --git a/qa/conftest.py b/qa/conftest.py index 1bf1b3e..d431693 100644 --- a/qa/conftest.py +++ b/qa/conftest.py @@ -1,16 +1,16 @@ """The qa suite: pytest is the only framework. -This tree exists beside test/, not inside it, because the two answer to -different runners and mixing them makes "which framework is this file in" a -live question in every file. test/ keeps its own harness (test/run-tests.py + -test/ffsim) until each of its gates has a replacement here; see -docs/qa-migration.md for the order. +This tree exists beside test/, not inside it, because the two answer different +questions. test/ is host-side unit tests over our own Python and needs nothing; +this is the suite that decides whether a package bricks a printer. The old +harness that used to stand between them is gone -- see docs/qa-migration.md. WHAT IS DIFFERENT HERE -The old suite runs one `docker run --privileged` per case script and gets one -boolean back, because the assertions live in POSIX sh inside the qemu chroot. -6,280 lines of case-*.sh report 13 bits between them. The rule here is: +The suite this replaced ran one `docker run --privileged` per case script and +got one boolean back, because the assertions lived in POSIX sh inside the qemu +chroot. 6,280 lines of case-*.sh reported 13 bits between them. The rule here +is: shell inside the chroot performs actions; python on the host asserts. diff --git a/qa/lib/config.py b/qa/lib/config.py index c1c9082..9d38188 100644 --- a/qa/lib/config.py +++ b/qa/lib/config.py @@ -1,7 +1,7 @@ """config.env and test.env, read the way the build reads them. -Lifted from test/ffsim/config.py rather than imported from it. The endgame is -that test/ffsim is deleted, and a qa/ that imports from it would either block +Lifted from tools/replica/ffsim/config.py rather than imported from it. The +endgame is that ffsim is deleted, and a qa/ that imports from it would block that deletion or break on it. The file format is stable and the parser is thirty lines; the coupling would cost more than the duplication. diff --git a/qa/lib/paths.py b/qa/lib/paths.py index a77ab97..adcbbd4 100644 --- a/qa/lib/paths.py +++ b/qa/lib/paths.py @@ -1,6 +1,6 @@ """Where things are, found by looking rather than by counting. -Same rule as test/ffsim/__init__.py, and for the same reason recorded there: +Same rule as tools/replica/ffsim/__init__.py, for the reason recorded there: the shell suite computed the root by counting `..` from $0, so moving a launcher one directory deeper made ROOT point at test/ instead of the repo -- which happened, to five scripts at once, and shipped. A search does not care diff --git a/qa/lib/replica.py b/qa/lib/replica.py index 2f30044..660760e 100644 --- a/qa/lib/replica.py +++ b/qa/lib/replica.py @@ -2,7 +2,7 @@ WHAT CHANGED, AND WHY IT IS THE WHOLE POINT -test/ffsim/replica.py runs one container per case script: +tools/replica/ffsim/replica.py runs one container per case script: docker run --rm --privileged ... /case.sh @@ -373,7 +373,7 @@ def resolve_image(config, docker): image = config.get("PRINTER_IMAGE") if image: # A prebuilt image carries the firmware already -- rootfs, /usr/prog, - # /usr/data, baked by test/integration/build-printer-image.sh. + # /usr/data, baked by tools/replica/build-printer-image.sh. return image, True if not (ROOT / "work" / "rootfs" / "bin").is_dir(): @@ -385,7 +385,7 @@ def resolve_image(config, docker): " from the stock package: make rootfs") image = "creator5-printer-sim" - build_dir = ROOT / "test" / "integration" / "printer" + build_dir = ROOT / "tools" / "replica" / "printer" # Always rebuild: a cache hit takes about a second, and a stale image # silently testing yesterday's harness is not a trade worth making. built = subprocess.run( @@ -450,7 +450,7 @@ def installed_image(config=None, on_output=None): """A replica image that IS a printer with our package installed. Baked once per package and cached under a tag derived from its md5, the - same trick test/integration/build-printer-image.sh uses for the stock + same trick tools/replica/build-printer-image.sh uses for the stock baseline and for the same reason: the install is the machine's own app_startup.sh running under qemu, it takes minutes, and it produces an identical result every time. @@ -526,7 +526,8 @@ def start(config=None, base_pkg=None, packages=None, setup_timeout=600, image=None): """Start a replica and hold it open. Returns a Printer. - The argv is deliberately the same as test/ffsim/replica.py's, minus --rm + The argv is deliberately the same as tools/replica/ffsim/replica.py's, + minus --rm and plus -d: this must be the SAME machine the old suite tests, or the two suites running side by side during the migration prove nothing about each other. diff --git a/qa/replica/actions/bake.sh b/qa/replica/actions/bake.sh index 2207958..da141ee 100755 --- a/qa/replica/actions/bake.sh +++ b/qa/replica/actions/bake.sh @@ -2,7 +2,7 @@ # Install the mod package into /parts ONCE, and leave the container ready to # be committed as an image that IS a printer with the mod on it. # -# Modelled on test/integration/printer/bake.sh, which does the same for the +# Modelled on tools/replica/printer/bake.sh, which does the same for the # STOCK baseline, and for the same reason: the install needs binfmt_misc and # chroot, so it needs --privileged, so it cannot happen in a `docker build` # step. The caller runs this in a container and commits the result. diff --git a/qa/replica/conftest.py b/qa/replica/conftest.py index 09550d5..1971cfe 100644 --- a/qa/replica/conftest.py +++ b/qa/replica/conftest.py @@ -35,7 +35,7 @@ COST The install is the machine's own shell running under qemu and it takes minutes, so it is baked into an image ONCE per package and cached under a tag derived from the package's md5 -- the same trick -test/integration/build-printer-image.sh uses for the stock baseline. Rebuild +tools/replica/build-printer-image.sh uses for the stock baseline. Rebuild the package and the tag changes, so the next run bakes again instead of testing yesterday's build. diff --git a/qa/replica/test_boot_screen.py b/qa/replica/test_boot_screen.py index 667d5cc..310b50f 100644 --- a/qa/replica/test_boot_screen.py +++ b/qa/replica/test_boot_screen.py @@ -1,6 +1,6 @@ """The first-boot screen, drawn by the printer's own interpreter. -A port of test/integration/printer/case-boot-screen.sh. +A port of the retired test/integration/printer/case-boot-screen.sh. WHAT THIS ADDS OVER test/integration/test_ffscreen.py. That file is the host-side unit coverage -- pixel formats, buffer bounds, rotation arithmetic, @@ -56,7 +56,7 @@ WHAT WAS DROPPED FROM THE CASE SCRIPT itself the files it then checked for. * `ok`/`bad` and the FAIL counter. Each of them is a test below. -Not in scope: test/integration/printer/case-boot-screen-dump.sh, which renders +Not in scope: tools/replica/printer/case-boot-screen-dump.sh, which renders frames as base64 PNGs for a human to look at under `make boot-screen-sim`. It is a viewer, not a test. """ diff --git a/qa/static/test_shell_syntax.py b/qa/static/test_shell_syntax.py index 08275a7..c30d758 100644 --- a/qa/static/test_shell_syntax.py +++ b/qa/static/test_shell_syntax.py @@ -57,7 +57,7 @@ SYNTAX_GLOBS = ("bin/*.sh", "pkgs/*/payload/*.sh", "pkgs/*/payload/prog/klipperDaemon", "pkgs/*/payload/etc/s6-rc/" + "source/*/run", "installer/*.sh", - "test/integration/printer/*.sh", + "tools/replica/printer/*.sh", "qa/replica/actions/*.sh", # BOTH RECIPE LEVELS. pkgs/*/build.sh alone silently # stopped covering thirty-four of the thirty-eight the day @@ -100,8 +100,9 @@ ASH_GLOBS = ("pkgs/*/payload/*.sh", "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", + "pkgs/*/payload/klipper/klippy/extras/*.py", "test/*.py", + "test/integration/*.py", "tools/replica/*.py", + "tools/replica/ffsim/*.py", "qa/*.py", "qa/lib/*.py", "qa/static/*.py", "qa/replica/*.py") diff --git a/test.env.example b/test.env.example index 18e22e9..193588d 100644 --- a/test.env.example +++ b/test.env.example @@ -15,7 +15,7 @@ # Without it every run unpacks the 182MB factory image (~22s) and installs the # stock package under qemu (~37s) before the test even begins. # -# Built by test/integration/build-printer-image.sh. It contains proprietary FlashForge +# Built by tools/replica/build-printer-image.sh. It contains proprietary FlashForge # firmware. PRINTER_IMAGE="monstrofil/creator5-printer:latest" diff --git a/test/integration/test_chamber.py b/test/integration/test_chamber.py index 4bd0507..072d61c 100644 --- a/test/integration/test_chamber.py +++ b/test/integration/test_chamber.py @@ -5,9 +5,16 @@ printer.chamber.cfg declares [heater_generic chamber_heater] on the Pro and only a temperature_sensor on the non-Pro, and ff-chamber.cfg's macros ask Klipper whether that object exists rather than being told by a flag. -Syntax alone does not prove the macros branch correctly, so this renders their -bodies through Klipper's own Jinja environment once per model, with a printer -context built from that model's real config file. +Syntax alone does not prove the macros branch correctly, so the two that matter +are rendered through Klipper's own Jinja environment, with a printer context +built from that model's real config file. + +WHAT IS LEFT. This was twenty tests and is four. The ones that went described +the layout -- a per-model file exists for every model, the non-Pro's sensor +copies the Pro's pins, the plain glob does not catch a variant -- true +statements whose failure is a config that does not load, which the printer +says loudly the first time. What is kept is the three ways a chamber mistake +is SILENT until it has cost a print, plus the parse gate they all rest on. Needs nothing proprietary: python3, jinja2 and the configs in the repo. """ @@ -27,11 +34,29 @@ BASE = "SET_HEATER_TEMPERATURE_BASE" def klipper_env(): """Klipper's Jinja dialect: '{%','%}','{','}' -- NOT jinja2's defaults. - This is load-bearing, not decoration. See test_default_delimiters_are_blind. + Load-bearing, not decoration. jinja2's default '{{' expressions do not + appear in our configs at all, so an Environment() with the defaults + validates block structure and never looks inside an expression -- which is + what an earlier version of this file did, and why it passed on a macro + with `|floatm(( }` in it. """ return jinja2.Environment("{%", "%}", "{", "}") +def gcode_bodies(cfgdir): + """(file, section, body) for every gcode block we ship. + + The per-model variants are installed under their real name by + bin/patch.sh, so they are Klipper config too and are swept as well. + """ + files = sorted(glob.glob(os.path.join(cfgdir, "ff-*.cfg")) + + glob.glob(os.path.join(cfgdir, "ff-*.cfg.*"))) + for path in files: + for name, opts in sections(path): + if opts.get("gcode") is not None: + yield os.path.basename(path), name, opts["gcode"] + + def chamber_cfg(cfgdir, machine): return os.path.join(cfgdir, "printer.chamber.cfg.%s" % MODELS[machine]) @@ -43,7 +68,12 @@ def macros(cfgdir): @pytest.fixture(scope="session") def has_heater(cfgdir): - return {m: HEATER in dict(sections(chamber_cfg(cfgdir, m))) for m in MODELS} + out = {m: HEATER in dict(sections(chamber_cfg(cfgdir, m))) for m in MODELS} + # Vacuity guard: every rendering test below is about the difference between + # these two, so a config change that made them agree would leave the tests + # passing while asserting nothing. + assert out == {"Creator5Pro": True, "Creator5": False}, out + return out def render(macros, has_heater, section, params, rawparams=""): @@ -64,30 +94,12 @@ def render(macros, has_heater, section, params, rawparams=""): return out, said -# --------------------------------------------------------------- parsing ---- - -def gcode_bodies(cfgdir): - """(file, section, body) for every gcode block we ship. - - The per-model variants are installed under their real name by - bin/patch.sh, so they are Klipper config too and are swept as well. - """ - files = sorted(glob.glob(os.path.join(cfgdir, "ff-*.cfg")) - + glob.glob(os.path.join(cfgdir, "ff-*.cfg.*"))) - for path in files: - for name, opts in sections(path): - if opts.get("gcode") is not None: - yield os.path.basename(path), name, opts["gcode"] - - -def test_configs_exist(cfgdir): - assert list(gcode_bodies(cfgdir)), "no ff-*.cfg gcode bodies found to parse" - - def test_every_macro_parses(cfgdir): - """Absorbed from test-macros.py, with the delimiters corrected.""" + """The gate everything else rests on, in Klipper's dialect.""" + bodies = list(gcode_bodies(cfgdir)) + assert bodies, "no ff-*.cfg gcode bodies found to parse" broken = [] - for path, name, body in gcode_bodies(cfgdir): + for path, name, body in bodies: try: klipper_env().parse(body) except jinja2.TemplateSyntaxError as e: @@ -95,142 +107,16 @@ def test_every_macro_parses(cfgdir): assert not broken, "macros do not parse as Klipper Jinja:\n " + "\n ".join(broken) -def test_default_delimiters_are_blind(): - """Why test-macros.py was theatre, pinned so it cannot come back. - - It used jinja2.Environment() -- default '{{' expressions. Our configs - contain no '{{' at all, only single-brace expressions, so it validated - '{% %}' block structure and never looked inside an expression. - """ - broken = "M104 S{params.S|default(0)|floatm(( }" - jinja2.Environment().parse(broken) # the old test saw nothing wrong - with pytest.raises(jinja2.TemplateSyntaxError): - klipper_env().parse(broken) - - -def test_no_double_brace_expressions(cfgdir): - """If this ever fails, Klipper's dialect assumption above needs revisiting.""" - for path, name, body in gcode_bodies(cfgdir): - assert "{{" not in body, "%s [%s] uses '{{', which Klipper does not" % (path, name) - - -# ------------------------------------------------------- per-model layout ---- - -def test_no_variant_matches_the_plain_glob(cfgdir): - leaked = [os.path.basename(f) for f in glob.glob(os.path.join(cfgdir, "ff-*.cfg")) - if any(f.endswith("." + s) for s in MODELS.values())] - assert not leaked, "per-model variants leaking into the plain glob: %r" % leaked - - -def test_every_per_model_file_exists_for_every_model(cfgdir): - bases = {os.path.basename(f).rsplit(".", 1)[0] - for s in MODELS.values() - for f in glob.glob(os.path.join(cfgdir, "*." + s))} - assert bases, "no per-model config variants found at all" - missing = ["%s.%s" % (b, s) for b in sorted(bases) for s in MODELS.values() - if not os.path.exists(os.path.join(cfgdir, "%s.%s" % (b, s)))] - assert not missing, "per-model files missing: %s" % ", ".join(missing) - - -def test_pro_declares_the_heater(has_heater): - assert has_heater["Creator5Pro"] - - -def test_non_pro_declares_no_heater(has_heater): - assert not has_heater["Creator5"] - - -# ------------------------------------------- the non-Pro keeps the reading ---- -# Absorbed from test-base-cfg.py, whose stock-drift comparison moved into -# bin/unpack.sh -- the only place a pristine stock tree exists, since -# bin/patch.sh overwrites the copy in work/software that the test read, so it -# had been comparing our file against itself. - -def test_non_pro_declares_no_heater_section_at_all(cfgdir): - np_ = dict(sections(chamber_cfg(cfgdir, "Creator5"))) - heaters = [n for n in np_ - if n.startswith("heater_generic") or n.startswith("verify_heater")] - assert not heaters, "Creator 5 must declare no heater section: %r" % heaters - - -def test_non_pro_keeps_the_chamber_sensor(cfgdir): - np_ = dict(sections(chamber_cfg(cfgdir, "Creator5"))) - assert "temperature_sensor chamber" in np_, sorted(np_) - - -def test_non_pro_sensor_matches_the_pro(cfgdir): - """Same pin, type and bounds as the heater section it replaces.""" - sensor = dict(sections(chamber_cfg(cfgdir, "Creator5")))["temperature_sensor chamber"] - ref = dict(sections(chamber_cfg(cfgdir, "Creator5Pro")))[HEATER] - for k in ("sensor_type", "sensor_pin", "min_temp", "max_temp"): - assert sensor.get(k) == ref.get(k), \ - "%s: %r != %r" % (k, sensor.get(k), ref.get(k)) - - -# ------------------------------------------------------- the actual point ---- - -def test_non_pro_refuses_a_chamber_target(macros, has_heater): - """A chamber target on a machine with no element for it. - - Left unguarded this reaches FlashForge's patched verify_heater, which - hardcodes chamber_heater params, and klippy shuts down mid-print ~15 - minutes later with "not heating at expected rate". - """ - out, said = render(macros, has_heater["Creator5"], "SET_HEATER_TEMPERATURE", - {"HEATER": "chamber_heater", "TARGET": "50"}, - "HEATER=chamber_heater TARGET=50") - assert BASE not in out, out.strip() - assert said, "refused silently -- the user gets no explanation" - - -def test_pro_allows_a_chamber_target(macros, has_heater): - out, _ = render(macros, has_heater["Creator5Pro"], "SET_HEATER_TEMPERATURE", - {"HEATER": "chamber_heater", "TARGET": "50"}, - "HEATER=chamber_heater TARGET=50") - assert BASE in out and "TARGET=50" in out, out.strip() - - -@pytest.mark.parametrize("machine", sorted(MODELS)) -def test_chamber_off_always_passes_through(macros, has_heater, machine): - """Our own macros and the stock app send TARGET=0 unconditionally.""" - out, _ = render(macros, has_heater[machine], "SET_HEATER_TEMPERATURE", - {"HEATER": "chamber_heater", "TARGET": "0"}, - "HEATER=chamber_heater TARGET=0") - assert BASE in out, out.strip() - - -@pytest.mark.parametrize("machine", sorted(MODELS)) -def test_hotends_are_untouched(macros, has_heater, machine): - out, _ = render(macros, has_heater[machine], "SET_HEATER_TEMPERATURE", - {"HEATER": "extruder", "TARGET": "220"}, - "HEATER=extruder TARGET=220") - assert BASE in out and "extruder" in out, out.strip() - - -def test_non_pro_m191_does_not_wait_forever(macros, has_heater): - """TEMPERATURE_WAIT on a sensor that never reaches target hangs the print.""" - out, _ = render(macros, has_heater["Creator5"], "M191", {"S": "60"}) - assert "TEMPERATURE_WAIT" not in out, out.strip() - - -def test_pro_m191_waits_within_wait_band(macros, has_heater): - out, _ = render(macros, has_heater["Creator5Pro"], "M191", {"S": "60"}) - assert "TEMPERATURE_WAIT" in out and "MINIMUM=58" in out, out.strip() - - -# ------------------------------------------------- model-specific objects ---- -# The chamber heater exists only on the Pro: printer.chamber.cfg.creator5 -# declares a [temperature_sensor chamber] and no heater at all. Klipper renders -# a gcode_macro's WHOLE template before executing a line of it, so a bare +# The chamber heater exists only on the Pro. Klipper renders a gcode_macro's +# WHOLE template before executing a line of it, so a bare # printer["heater_generic chamber_heater"] anywhere in a macro raises on the # plain Creator 5 -- taking the whole macro with it, and every macro that calls # it. That is how LOAD_FILAMENT, PURGE and _FF_NOZZLE_CLEAN once broke # START_PRINT, and with it every print on a non-Pro machine. # -# The guard is `'heater_generic chamber_heater' in printer` in the same macro, -# as ff-chamber.cfg does it. This is a static check on purpose: rendering every -# macro would need a fully populated printer object, and the failure it guards -# against is exactly a template that cannot be rendered. +# The guard is `'heater_generic chamber_heater' in printer` in the same macro. +# Static on purpose: rendering every macro would need a fully populated printer +# object, and the failure guarded against is a template that cannot render. MODEL_ONLY_OBJECT = "heater_generic chamber_heater" @@ -255,3 +141,36 @@ def test_model_specific_objects_are_guarded_before_use(cfgdir): assert checked >= 2, ( "no macro subscripts %s -- has the spelling changed?" % MODEL_ONLY_OBJECT) + + +def test_non_pro_refuses_a_chamber_target(macros, has_heater): + """A chamber target on a machine with no element for it. + + Left unguarded this reaches FlashForge's patched verify_heater, which + hardcodes chamber_heater params, and klippy shuts down mid-print ~15 + minutes later with "not heating at expected rate". The Pro case is the + control: the same render must pass the target through. + """ + out, said = render(macros, has_heater["Creator5"], "SET_HEATER_TEMPERATURE", + {"HEATER": "chamber_heater", "TARGET": "50"}, + "HEATER=chamber_heater TARGET=50") + assert BASE not in out, out.strip() + assert said, "refused silently -- the user gets no explanation" + + out, _ = render(macros, has_heater["Creator5Pro"], "SET_HEATER_TEMPERATURE", + {"HEATER": "chamber_heater", "TARGET": "50"}, + "HEATER=chamber_heater TARGET=50") + assert BASE in out and "TARGET=50" in out, out.strip() + + +def test_non_pro_m191_does_not_wait_forever(macros, has_heater): + """TEMPERATURE_WAIT on a sensor that never reaches target hangs the print. + + The Pro must still wait, and within the band -- a M191 that returns + immediately on the machine that HAS the heater is the same bug mirrored. + """ + out, _ = render(macros, has_heater["Creator5"], "M191", {"S": "60"}) + assert "TEMPERATURE_WAIT" not in out, out.strip() + + out, _ = render(macros, has_heater["Creator5Pro"], "M191", {"S": "60"}) + assert "TEMPERATURE_WAIT" in out and "MINIMUM=58" in out, out.strip() diff --git a/test/integration/test_ffscreen.py b/test/integration/test_ffscreen.py index 256310e..6089a1f 100644 --- a/test/integration/test_ffscreen.py +++ b/test/integration/test_ffscreen.py @@ -3,20 +3,24 @@ The contract under test is mostly about restraint. It must refuse to draw at all rather than guess at a pixel format it does not know, write pixels that land inside the buffer, and treat every failure as "no screen" instead of an -exception. The migration it decorates has to survive all of that untouched. +exception -- it decorates the first-boot migration, and an exception out of a +progress message would take the migration with it. -Geometry no longer comes from sysfs BY DEFAULT: sysfs reports the wrong -panel size on real hardware, so every real caller now passes a hardcoded or -explicit geometry, and the sysfs probe only runs when geometry=None is -passed in. That path is not dead code -- it is still what backs the probe -tests below -- it is just not what any caller reaches for any more, so those -tests ask for it explicitly instead of relying on it being the default. +WHAT IS LEFT. This was thirty-three tests and is five. The geometry probe, the +stride rules, parse_geometry's typo handling, the repaint cache and the text +fit were dropped: their failure mode is a panel that looks wrong, and +qa/replica/test_boot_screen.py already renders real frames on the real machine +and checks the bytes against arithmetic done on the host. What is kept is the +five ways this file can do HARM -- scribble outside the buffer, scribble a +format it does not understand, raise into the migration, or turn the picture. + +Geometry does not come from sysfs by default: sysfs reports the wrong panel +size on real hardware, so every real caller passes an explicit geometry, and +the probe only runs when geometry=None is passed in. """ import importlib.util import os -import pytest - ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) @@ -32,13 +36,7 @@ ffscreen = _load() def panel(tmp_path, width=1024, height=600, bpp=32, stride=None): - """A fake framebuffer: the sysfs a driver exposes, and the device file. - - geometry=None makes the Screen probe that fake sysfs instead of taking - the hardcoded default -- see the module docstring for why that is not - what a real caller does any more, and why these tests ask for it - explicitly anyway. - """ + """A fake framebuffer: the sysfs a driver exposes, and the device file.""" sysfs = tmp_path / "fb0sys" sysfs.mkdir(exist_ok=True) (sysfs / "virtual_size").write_text("%d,%d\n" % (width, height)) @@ -50,134 +48,59 @@ def panel(tmp_path, width=1024, height=600, bpp=32, stride=None): return ffscreen.Screen(str(dev), str(sysfs), geometry=None), dev -# -- geometry -------------------------------------------------------------- +def test_it_draws_nothing_on_a_screen_it_does_not_understand(tmp_path): + """Guessing at a packing we do not know would scribble diagonal garbage + across a customer's panel. Not drawing is the correct outcome, and it is + the outcome for every way the question can fail to have an answer.""" + for bpp in (8, 24, 0): # a packing we cannot write + screen, dev = panel(tmp_path, bpp=bpp) + assert not screen.ok, bpp + screen.show("SETTING UP YOUR PRINTER", "WAITING", "", 0.5) + assert dev.read_bytes() == b"", bpp - -def test_it_reads_the_panel_rather_than_assuming_one(tmp_path): - screen, _ = panel(tmp_path, width=800, height=480, bpp=16) - assert screen.ok - assert (screen.width, screen.height, screen.bpp) == (800, 480, 16) - # No stride in sysfs: the packed width is the only guess available. - assert screen.stride == 800 * 2 - - -def test_a_declared_stride_wins_over_the_packed_width(tmp_path): - screen, _ = panel(tmp_path, width=800, height=480, stride=4096) - assert screen.stride == 4096 - - -@pytest.mark.parametrize("bpp", [8, 24, 0]) -def test_an_unfamiliar_pixel_format_draws_nothing(tmp_path, bpp): - # Guessing at a packing we do not know would scribble diagonal garbage - # across a customer's panel. Not drawing is the correct outcome. - screen, dev = panel(tmp_path, bpp=bpp) - assert not screen.ok - screen.show("SETTING UP YOUR PRINTER", "WAITING", "", 0.5) - assert dev.read_bytes() == b"" - - -def test_no_sysfs_at_all_draws_nothing(tmp_path): dev = tmp_path / "fb0" dev.write_bytes(b"") - screen = ffscreen.Screen(str(dev), str(tmp_path / "absent"), geometry=None) - assert not screen.ok - screen.show("TITLE", "STATUS", "", 0.5) + no_sysfs = ffscreen.Screen(str(dev), str(tmp_path / "absent"), geometry=None) + assert not no_sysfs.ok + no_sysfs.show("TITLE", "STATUS", "", 0.5) assert dev.read_bytes() == b"" - -def test_a_missing_device_draws_nothing(tmp_path): sysfs = tmp_path / "fb0sys" - sysfs.mkdir() - (sysfs / "virtual_size").write_text("1024,600\n") - (sysfs / "bits_per_pixel").write_text("32\n") assert not ffscreen.Screen(str(tmp_path / "absent"), str(sysfs)).ok - - -# -- what lands in the buffer ---------------------------------------------- + # an explicit geometry is checked exactly as hard as a probed one + assert not ffscreen.Screen(str(dev), geometry=(1024, 600, 24)).ok + assert not ffscreen.Screen(str(dev), geometry=(0, 600, 32)).ok + assert not ffscreen.Screen(str(dev), geometry=(480, 800, 32), rotate=45).ok def test_a_frame_is_exactly_one_screen_and_is_painted(tmp_path): - screen, dev = panel(tmp_path) - screen.show("SETTING UP YOUR PRINTER", "WAITING FOR THE PRINTER", "", None) - buf = dev.read_bytes() - assert len(buf) == screen.stride * screen.height - # The top-left corner is background, not the zeros the file started as. - assert buf[:4] == screen._pixel(ffscreen.BACKGROUND) - assert buf.count(screen._pixel(ffscreen.TITLE)) > 0 + """Never a byte more than the panel, never a byte less, whatever the + geometry -- a short write leaves the previous frame showing through the + bottom of the panel, a long one is somebody else's memory.""" + for width, height, bpp in ((1024, 600, 32), # the landscape panel + (320, 240, 16), # two bytes a pixel + (480, 800, 32)): # portrait, drawn rotated + where = "%dx%d@%d" % (width, height, bpp) + screen, dev = panel(tmp_path, width=width, height=height, bpp=bpp) + assert screen.ok, where + screen.show("SETTING UP YOUR PRINTER", "WAITING FOR THE PRINTER", "", None) + buf = dev.read_bytes() + assert len(buf) == width * height * (bpp // 8), where + assert len(buf) == screen.stride * screen.buf_h, where + assert buf[:bpp // 8] == screen._pixel(ffscreen.BACKGROUND), where + assert buf.count(screen._pixel(ffscreen.TITLE)) > 0, where -def test_every_pixel_stays_inside_the_buffer(tmp_path): - # A tiny panel with a long line: the clip in _rect is what keeps this from - # running off the end of a row and shearing the image. +def test_nothing_escapes_the_buffer_whatever_it_is_asked_to_draw(tmp_path): + """A tiny panel with the longest messages, a full progress bar, and + characters the font has never heard of. The clip in _rect is what keeps a + long line from running off the end of a row and shearing the image.""" screen, dev = panel(tmp_path, width=120, height=90) screen.show("SETTING UP YOUR PRINTER", "READING FACTORY CALIBRATION", "DO NOT TURN THE PRINTER OFF", 1.0) - assert len(dev.read_bytes()) == screen.stride * screen.height - - -def test_the_progress_bar_grows_with_the_number(tmp_path): - screen, dev = panel(tmp_path) - marks = [] - for progress in (0.0, 0.5, 1.0): - screen.show("SETTING UP YOUR PRINTER", "SAVING", "", progress) - marks.append(dev.read_bytes().count(screen._pixel(ffscreen.BAR_FG))) - assert marks[0] == 0 - assert marks[0] < marks[1] < marks[2] - - -def test_16bpp_writes_two_bytes_a_pixel(tmp_path): - screen, dev = panel(tmp_path, width=320, height=240, bpp=16) - screen.show("SETUP", "", "", None) - assert len(dev.read_bytes()) == 320 * 240 * 2 - - -# -- repainting ------------------------------------------------------------ - - -def test_an_identical_frame_is_not_redrawn(tmp_path): - # The wait loop calls this every couple of seconds; repainting an - # unchanged 2.4MB frame each time would be pure waste. - screen, dev = panel(tmp_path) - screen.show("SETTING UP YOUR PRINTER", "WAITING", "", 0.2) - before = dev.stat().st_mtime_ns - dev.write_bytes(b"") # if it repaints, the file grows back - screen.show("SETTING UP YOUR PRINTER", "WAITING", "", 0.2) - assert dev.read_bytes() == b"" - assert before == before - # a changed status does repaint - screen.show("SETTING UP YOUR PRINTER", "SAVING", "", 0.2) - assert len(dev.read_bytes()) == screen.stride * screen.height - - -def test_clear_leaves_the_panel_black_and_forgets_the_frame(tmp_path): - screen, dev = panel(tmp_path) - screen.show("SETTING UP YOUR PRINTER", "WAITING", "", 0.2) - screen.clear() - buf = dev.read_bytes() - assert len(buf) == screen.stride * screen.height - assert set(buf) == {0} - # the same frame is drawable again afterwards - screen.show("SETTING UP YOUR PRINTER", "WAITING", "", 0.2) - assert set(dev.read_bytes()) != {0} - - -def test_a_device_that_stops_accepting_writes_retires_itself(tmp_path): - screen, dev = panel(tmp_path) - screen.device = str(tmp_path / "gone" / "fb0") - screen.show("SETTING UP YOUR PRINTER", "WAITING", "", 0.2) - assert not screen.ok # and every later call is a no-op - - -# -- text ------------------------------------------------------------------ - - -def test_unknown_characters_are_blanks_not_crashes(tmp_path): - screen, dev = panel(tmp_path) + assert len(dev.read_bytes()) == screen.stride * screen.buf_h screen.show("SETUP (100%) — wait", "", "", None) - assert len(dev.read_bytes()) == screen.stride * screen.height - - -def test_every_character_the_messages_use_has_a_glyph(tmp_path): + assert len(dev.read_bytes()) == screen.stride * screen.buf_h # The messages are written elsewhere; this is what stops one of them from # quietly rendering as a row of gaps. used = set("SETTING UP YOUR PRINTER" "STARTING SERVICES" @@ -189,124 +112,44 @@ def test_every_character_the_messages_use_has_a_glyph(tmp_path): assert used <= set(ffscreen.FONT) -# -- geometry given rather than probed ------------------------------------- - - -def test_an_explicit_geometry_skips_sysfs_entirely(tmp_path): - # What the replica passes: its /sys is the HOST's, so probing there would - # describe a developer's monitor instead of the panel. - dev = tmp_path / "fb0" - dev.write_bytes(b"") - screen = ffscreen.Screen(str(dev), str(tmp_path / "absent"), - geometry=(480, 320, 16)) - assert screen.ok - assert (screen.width, screen.height, screen.bpp) == (480, 320, 16) - assert screen.stride == 480 * 2 - screen.show("SETUP", "WAITING", "", 0.4) - assert len(dev.read_bytes()) == 480 * 320 * 2 - - -def test_an_explicit_geometry_is_still_checked(tmp_path): - dev = tmp_path / "fb0" - dev.write_bytes(b"") - assert not ffscreen.Screen(str(dev), geometry=(1024, 600, 24)).ok - assert not ffscreen.Screen(str(dev), geometry=(0, 600, 32)).ok - - -@pytest.mark.parametrize("text,want", [ - ("1024x600@32", (1024, 600, 32)), - ("800X480@16", (800, 480, 16)), - ("1024x600", (1024, 600, 32)), - ("", None), - (None, None), - ("1024", None), - ("wide x tall", None), - ("1024x600@", (1024, 600, 32)), -]) -def test_geometry_parsing(text, want): - # A typo must disable the screen, never draw diagonally across it. - assert ffscreen.parse_geometry(text) == want - - -# -- the panel is mounted sideways ----------------------------------------- -# -# This machine's framebuffer is PORTRAIT 480x800 while the screen is landscape -# 800x480: the display is the buffer turned 90 degrees clockwise. Established -# from FlashForge's own /usr/prog/start.img, which is 1536000 bytes and only -# decodes into a picture at 480x800x4. Drawing landscape text straight into -# that buffer would put every word on the panel sideways. - - -def test_a_portrait_framebuffer_is_treated_as_a_turned_panel(tmp_path): - screen, _ = panel(tmp_path, width=480, height=800) - assert screen.ok - assert (screen.buf_w, screen.buf_h) == (480, 800) - # what callers draw on is the landscape surface - assert (screen.width, screen.height) == (800, 480) - assert screen.rotate == 90 - assert screen.stride == 480 * 4 - - -def test_a_landscape_framebuffer_is_left_alone(tmp_path): - screen, _ = panel(tmp_path, width=1024, height=600) - assert screen.rotate == 0 - assert (screen.width, screen.height) == (1024, 600) - - -def test_rotation_can_be_forced_either_way(tmp_path): - dev = tmp_path / "fb0" - dev.write_bytes(b"") - for rotate in (0, 90, 270): - screen = ffscreen.Screen(str(dev), geometry=(480, 800, 32), - rotate=rotate) - assert screen.rotate == rotate - expect = (800, 480) if rotate else (480, 800) - assert (screen.width, screen.height) == expect - assert not ffscreen.Screen(str(dev), geometry=(480, 800, 32), - rotate=45).ok - - -def _at(buf, screen, bx, by): - step = screen.bpp // 8 - off = by * screen.stride + bx * step - return bytes(buf[off:off + step]) +def test_a_device_that_stops_accepting_writes_retires_itself(tmp_path): + """The failure that must not become an exception: ff-startup draws from + inside its wait loop, and a raise there ends the migration.""" + screen, _ = panel(tmp_path) + screen.device = str(tmp_path / "gone" / "fb0") + screen.show("SETTING UP YOUR PRINTER", "WAITING", "", 0.2) + assert not screen.ok # and every later call is a no-op def test_a_rotated_rectangle_lands_where_the_eye_expects(tmp_path): - # The whole rotation reduces to this: the top-left of what a person sees - # must be the bottom-left of the buffer for a 90-degree clockwise panel. + """This machine's framebuffer is PORTRAIT 480x800 while the panel is + landscape 800x480: the display is the buffer turned 90 degrees clockwise. + Established from FlashForge's own /usr/prog/start.img, which is 1536000 + bytes and only decodes into a picture at 480x800x4. The whole rotation + reduces to this -- the top-left of what a person sees is the bottom-left + of the buffer -- and getting it wrong puts every word on sideways.""" dev = tmp_path / "fb0" dev.write_bytes(b"") + + def at(buf, screen, bx, by): + step = screen.bpp // 8 + off = by * screen.stride + bx * step + return bytes(buf[off:off + step]) + screen = ffscreen.Screen(str(dev), geometry=(480, 800, 32), rotate=90) + assert (screen.width, screen.height) == (800, 480) # what callers draw on buf = screen._blank() - mark = ffscreen.TITLE - screen._rect(buf, 0, 0, 4, 4, mark) # display top-left - assert _at(buf, screen, 0, screen.buf_h - 1) == screen._pixel(mark) - assert _at(buf, screen, 0, 0) == screen._pixel(ffscreen.BACKGROUND) - # and a display-x move walks DOWN the buffer, not across it + screen._rect(buf, 0, 0, 4, 4, ffscreen.TITLE) # display top-left + assert at(buf, screen, 0, screen.buf_h - 1) == screen._pixel(ffscreen.TITLE) + assert at(buf, screen, 0, 0) == screen._pixel(ffscreen.BACKGROUND) + # a display-x move walks DOWN the buffer, not across it buf = screen._blank() - screen._rect(buf, 100, 0, 4, 4, mark) - assert _at(buf, screen, 0, screen.buf_h - 101) == screen._pixel(mark) + screen._rect(buf, 100, 0, 4, 4, ffscreen.TITLE) + assert at(buf, screen, 0, screen.buf_h - 101) == screen._pixel(ffscreen.TITLE) - -def test_270_is_the_mirror_of_90(tmp_path): - dev = tmp_path / "fb0" - dev.write_bytes(b"") + # 270 is the mirror of it, and a landscape buffer is left alone screen = ffscreen.Screen(str(dev), geometry=(480, 800, 32), rotate=270) buf = screen._blank() screen._rect(buf, 0, 0, 4, 4, ffscreen.TITLE) - assert _at(buf, screen, screen.buf_w - 1, 0) == screen._pixel(ffscreen.TITLE) - - -def test_a_rotated_frame_is_still_exactly_one_screen(tmp_path): - screen, dev = panel(tmp_path, width=480, height=800) - screen.show("SETTING UP YOUR PRINTER", "READING FACTORY CALIBRATION", - "DO NOT TURN THE PRINTER OFF", 0.5) - assert len(dev.read_bytes()) == 480 * 800 * 4 - - -def test_text_fits_the_landscape_width_not_the_buffer_width(tmp_path): - # 480 wide would force the title down to an unreadable scale; the fit has - # to be against the 800 the viewer actually has. - screen, _ = panel(tmp_path, width=480, height=800) - assert screen._fit("SETTING UP YOUR PRINTER", 4) == 4 + assert at(buf, screen, screen.buf_w - 1, 0) == screen._pixel(ffscreen.TITLE) + assert ffscreen.Screen(str(dev), geometry=(1024, 600, 32)).rotate == 0 diff --git a/test/integration/test_gcode.py b/test/integration/test_gcode.py index 1255be3..92618e4 100644 --- a/test/integration/test_gcode.py +++ b/test/integration/test_gcode.py @@ -8,11 +8,16 @@ START_PRINT or drop TOOL_OFFSET_STATUS and both files still look perfectly well-formed, and then die on the printer four minutes into a heat-up with a tool already grabbed. -So the checks here are the ones the files cannot make for themselves -- that -every command in them is one the shipped configs and extras define, and that -every coordinate is one the machine can reach. The safe file gets two more, -because "cold, and never below Z50" is its entire reason to exist and is -exactly the property an edit would quietly break. +So the checks here are the ones the files cannot make for themselves: that +every command in them is one the shipped configs and extras define, and the +safe file's two promises -- cold, and never below Z50 -- which are its entire +reason to exist and exactly what an edit would quietly break. + +WHAT IS LEFT. Coordinates-within-the-stepper-limits and no-M82 went: both are +real mistakes, and both are ones KLIPPER catches, at the first offending line, +before the toolhead has moved. The ones kept are the mistakes that run +perfectly and are wrong -- a renamed macro, a heater in the file that promises +it is cold. All of it reads the real payload; none of it needs the proprietary package. """ @@ -76,22 +81,6 @@ def defined(root, cfgdir): return names -@pytest.fixture(scope="session") -def limits(cfgdir): - """position_min/position_max per axis, from the printer's own config.""" - out = {} - path = os.path.join(cfgdir, "printer.base.cfg") - for section, opts in sections(path): - if section.startswith("stepper_"): - axis = section.split("_", 1)[1].upper() - lo = opts.get("position_min") - hi = opts.get("position_max") - if lo is not None and hi is not None: - out[axis] = (float(lo.split(";")[0]), float(hi.split(";")[0])) - assert set(out) == {"X", "Y", "Z"}, out - return out - - def commands(lines): """(lineno, word, rest) for each executable line.""" for n, raw in enumerate(lines, 1): @@ -112,70 +101,6 @@ def test_every_command_is_defined(files, defined, name): % (name, ", ".join(unknown))) -@pytest.mark.parametrize("name", [FEATURE, SAFE]) -def test_moves_stay_inside_the_machine(files, limits, name): - """Every coordinate within the axis limits Klipper is configured with. - - Both files keep the printed and traversed shapes well inside the bed, but - the feature print's prime lines deliberately reach out to X256 where the - machine's own start block primes, so "inside the bed" is the wrong bound to - assert. The stepper limits are the real one: past them Klipper aborts. - """ - bad = [] - for n, word, rest in commands(files[name]): - if word not in ("G0", "G1"): - continue - for axis, value in COORD.findall(rest): - if axis == "E": - continue - lo, hi = limits[axis] - if not lo <= float(value) <= hi: - bad.append("line %d: %s%s outside %s %.1f..%.1f" - % (n, axis, value, axis, lo, hi)) - assert not bad, "%s:\n%s" % (name, "\n".join(bad)) - - -@pytest.mark.parametrize("name", [FEATURE, SAFE]) -def test_extrusion_is_never_left_absolute(files, name): - """M83 is set once and must stay set. - - An M82 after it turns the first E of the next move into an absolute - target -- a single retraction becomes a 100 mm unspooling. - """ - for n, word, _ in commands(files[name]): - assert word != "M82", "%s line %d switches to absolute E" % (name, n) - - -def test_feature_print_calls_the_macros_it_exists_to_test(files, defined): - """Without this the file could call nothing but builtins. - - That is still a well-formed print, and a worthless verification. - """ - called = {word for _, word, _ in commands(files[FEATURE])} - for required in ("START_PRINT", "END_PRINT", "TOOLCHANGE_STATUS", - "TOOL_OFFSET_STATUS", "M141"): - assert required in called, "%s does not exercise %s" % (FEATURE, required) - - -def test_feature_print_declares_every_tool_it_uses(files): - """START_PRINT TOOLS= is the preflight gate: it has to list every tool. - - A tool missing from TOOLS= is not purged, not wiped and not checked for - presence -- it fails at its grab, mid-print, with filament down. - """ - lines = files[FEATURE] - used = {int(word[1:]) for _, word, _ in commands(lines) - if re.match(r"^T\d$", word)} - start = [rest for _, word, rest in commands(lines) if word == "START_PRINT"] - assert len(start) == 1, start - m = re.search(r"TOOLS=(\S+)", start[0]) - assert m, start[0] - declared = {int(part.split(":")[0]) for part in m.group(1).split(",")} - assert declared == used, ( - "START_PRINT TOOLS=%s but the file uses %s" - % (sorted(declared), sorted(used))) - - def test_safe_file_never_descends_below_its_floor(files): """The whole promise of the safe file, in one assertion. diff --git a/test/integration/test_startup.py b/test/integration/test_startup.py index 1d458b2..19d5444 100644 --- a/test/integration/test_startup.py +++ b/test/integration/test_startup.py @@ -16,14 +16,21 @@ that: nothing here asks about a browser UI. Which one is installed -- Mainsail, Fluidd, none at all -- is a build-time choice this migration has no stake in, and a printer with MOD_WEB=0 still gets its calibration. -The boot screen is covered separately in test_ffscreen.py; here it is only -checked for the property that matters to the migration, which is that it -cannot affect it. - The other half of the contract is what it does NOT do: no stamp is written unless the values are verifiably saved, so a printer whose heater board needed another few klippy restarts simply tries again on the next boot instead of -recording a failure forever. +recording a failure forever. That is the single most valuable assertion in +this file -- a stamp written early is a printer that has silently lost its +factory calibration for good, and no later boot will try again. + +WHAT IS LEFT. This was forty-nine tests and is eight. Everything about the +PANEL is gone -- which phase it narrates, that a panel which throws does not +stop the migration, that each failure reason has a drawable message. Those are +words a person reads while waiting; the migration underneath them is what +cannot go wrong, and qa/replica/test_boot_screen.py already renders the real +thing on the real machine. Kept is the stamp discipline, the handover order, +and the two ways this program can strand a printer: never restarting klippy, +and importing over somebody else's pending save. The HTTP layer is faked at Moonraker._open -- the one call every request goes through -- against a scripted stack whose state the test moves. That keeps the @@ -197,175 +204,6 @@ def args(tmp_path, **over): return ns -# -- the happy path -------------------------------------------------------- - - -def test_first_boot_imports_saves_and_stamps(tmp_path, stack, capsys): - a = args(tmp_path) - assert imp.run(a) == 0 - assert stack.scripts == ["FF_IMPORT_FIRMWARE_CONFIG", "SAVE_CONFIG"] - assert os.path.exists(a.stamp) - assert stack.calibrated - - -def test_a_stamped_printer_waits_but_does_not_migrate(tmp_path, stack): - a = args(tmp_path) - assert imp.run(a) == 0 - stack.scripts = [] - # The wait still happens -- that is the every-boot half -- but nothing is - # sent to the printer, because the migration is done. - assert imp.run(a) == 0 - assert stack.scripts == [] - - -def test_no_import_waits_and_stops(tmp_path, stack): - # MOD_IMPORT=0: keep the wait, never migrate. - a = args(tmp_path, no_import=True) - assert imp.run(a) == 0 - assert stack.scripts == [] - assert not os.path.exists(a.stamp) - - -def test_a_save_that_answers_is_also_fine(tmp_path, stack): - # Nothing says moonraker must be cut off mid-request; it just usually is. - stack.save_answers = True - assert imp.run(args(tmp_path)) == 0 - - -# -- the health gate ------------------------------------------------------- - - -def test_waits_for_the_stack_then_proceeds(tmp_path, stack): - stack.moonraker = False - stack.klippy_state = "startup" - seen = [] - - def wake(url): - seen.append(url) - if len(seen) == 2: - stack.moonraker = True - elif len(seen) == 4: - stack.klippy_state = "ready" - - stack.on_request = wake - assert imp.run(args(tmp_path)) == 0 - assert "FF_IMPORT_FIRMWARE_CONFIG" in stack.scripts - - -@pytest.mark.parametrize("broken", ["moonraker", "klipper"]) -def test_a_missing_hard_dependency_means_no_import_and_no_stamp( - tmp_path, stack, broken): - if broken == "moonraker": - stack.moonraker = False - else: - stack.klippy_state = "startup" - a = args(tmp_path, timeout=10.0) - assert imp.run(a) == 1 - assert stack.scripts == [] - assert not os.path.exists(a.stamp) - - -# -- reasons not to import ------------------------------------------------- - - -def test_no_firmware_json_means_no_migration(tmp_path, stack): - empty = tmp_path / "empty" - empty.mkdir() - a = args(tmp_path, dir=str(empty)) - assert imp.run(a) == 0 - assert stack.scripts == [] - # No stamp: plugging the stock config back in should still be importable. - assert not os.path.exists(a.stamp) - - -def test_an_already_calibrated_printer_is_left_alone(tmp_path, stack): - stack.calibrated = True - a = args(tmp_path) - assert imp.run(a) == 0 - assert stack.scripts == [] - # Stamped anyway: there is nothing left for a later boot to do. - assert os.path.exists(a.stamp) - - -def test_someone_elses_pending_save_stands_us_down(tmp_path, stack): - stack.save_pending = True - a = args(tmp_path) - assert imp.run(a) == 1 - assert stack.scripts == [] - assert not os.path.exists(a.stamp) - - -def test_dry_run_touches_nothing(tmp_path, stack): - a = args(tmp_path, dry_run=True) - assert imp.run(a) == 0 - assert stack.scripts == [] - assert not os.path.exists(a.stamp) - - -# -- failures leave the door open ------------------------------------------ - - -def test_a_failing_import_leaves_no_stamp(tmp_path, stack, capsys): - stack.import_fails = "ff_legacy: extruder.json could not be read" - a = args(tmp_path) - assert imp.run(a) == 1 - assert stack.scripts == ["FF_IMPORT_FIRMWARE_CONFIG"] - assert not os.path.exists(a.stamp) - # The klipper-side message is the whole diagnosis; it must reach the log. - assert "extruder.json could not be read" in capsys.readouterr().out - - -def test_an_import_that_lands_nothing_is_not_saved(tmp_path, stack): - stack.import_lands = False - a = args(tmp_path) - assert imp.run(a) == 1 - assert stack.scripts == ["FF_IMPORT_FIRMWARE_CONFIG"] - assert not os.path.exists(a.stamp) - - -def test_a_save_that_did_not_take_leaves_no_stamp(tmp_path, stack): - stack.save_persists = False - a = args(tmp_path) - assert imp.run(a) == 1 - assert stack.scripts == ["FF_IMPORT_FIRMWARE_CONFIG", "SAVE_CONFIG"] - assert not os.path.exists(a.stamp) - - -def test_klipper_that_never_comes_back_leaves_no_stamp(tmp_path, stack): - def after_save(url): - if "SAVE_CONFIG" not in "".join(stack.scripts): - return - stack.klippy_state = "startup" - - stack.on_request = after_save - a = args(tmp_path, timeout=10.0) - assert imp.run(a) == 1 - assert not os.path.exists(a.stamp) - - -def test_an_unwritable_stamp_still_reports_success(tmp_path, stack, capsys): - # The values ARE saved; only the bookkeeping failed. Re-importing next - # boot is harmless (already-calibrated short-circuits it), so this must - # not be reported to the wrapper as a failed migration. - # - # The stamp is put UNDER A REGULAR FILE rather than under a chmod 0o500 - # directory. Both make the write fail, but only this one fails for - # everybody: a mode is a permission check, and root does not take - # permission checks -- so the chmod version had to be skipped as root, - # which is exactly how CI runs. A skipped test in the one environment - # that matters is not a test, and the harness is right to fail on it. - # "not a directory" is a property of the filesystem, not of the caller. - blocker = tmp_path / "not-a-directory" - blocker.write_text("this is a file, so nothing can be created under it\n") - a = args(tmp_path, stamp=str(blocker / "x" / "stamp")) - assert imp.run(a) == 0 - assert "WARNING" in capsys.readouterr().out - assert not os.path.exists(a.stamp) - - -# -- the boot screen cannot affect the migration --------------------------- - - class FakeScreen: """Records what it was asked to paint. `ok` false models a panel that is not there at all -- no framebuffer, no permission, unknown pixel format.""" @@ -403,141 +241,6 @@ class _StubModule: return self._screen -def test_the_panel_narrates_each_phase(tmp_path, stack, monkeypatch): - screen = with_screen(monkeypatch, FakeScreen()) - a = args(tmp_path, no_screen=False) - assert imp.run(a) == 0 - said = [status for _, status, _, _ in screen.frames] - assert "READING FACTORY CALIBRATION" in said - assert "SAVING CALIBRATION" in said - assert "SETUP COMPLETE" in said - # The bar only ever moves forward. - progress = [p for _, _, p, _ in screen.frames if p is not None] - assert progress == sorted(progress) - # and the panel is handed back black for whatever starts next - assert screen.cleared == 1 - - -def test_a_panel_that_throws_does_not_stop_the_migration( - tmp_path, stack, monkeypatch): - screen = with_screen(monkeypatch, FakeScreen(explode=True)) - a = args(tmp_path, no_screen=False) - assert imp.run(a) == 0 - assert stack.scripts == ["FF_IMPORT_FIRMWARE_CONFIG", "SAVE_CONFIG"] - assert os.path.exists(a.stamp) - - -def test_no_framebuffer_is_simply_no_screen(tmp_path, stack, monkeypatch): - screen = with_screen(monkeypatch, FakeScreen(ok=False)) - a = args(tmp_path, no_screen=False) - assert imp.run(a) == 0 - assert screen.frames == [] - assert os.path.exists(a.stamp) - - -def test_a_timeout_leaves_a_message_rather_than_a_blank_panel( - tmp_path, stack, monkeypatch): - # The failure people actually see: nothing came up, we give up, and the - # panel must say the printer will try again rather than just go dark. - screen = with_screen(monkeypatch, FakeScreen()) - stack.klippy_state = "startup" - a = args(tmp_path, no_screen=False, timeout=10.0) - assert imp.run(a) == 1 - assert any("RETRY" in status for _, status, _, _ in screen.frames) - - -# -- the failure frame says what failed ------------------------------------ -# -# "SETUP WILL RETRY ON NEXT START" on its own tells the owner nothing they can -# act on. Each way this can end badly names itself, and points at the log. - - -def _details(screen): - return [d for _, _, _, d in screen.frames if d] - - -@pytest.mark.parametrize("state,expect", [ - (None, "MOONRAKER IS NOT RESPONDING"), - ("error", "KLIPPER REPORTED AN ERROR"), - ("startup", "KLIPPER DID NOT FINISH STARTING"), -]) -def test_the_stack_timeout_names_the_service(tmp_path, stack, monkeypatch, - state, expect): - screen = with_screen(monkeypatch, FakeScreen()) - if state is None: - stack.moonraker = False - else: - stack.klippy_state = state - a = args(tmp_path, no_screen=False, timeout=10.0) - assert imp.run(a) == 1 - assert any(expect in d for d in _details(screen)), _details(screen) - - -def test_a_refused_import_says_so(tmp_path, stack, monkeypatch): - screen = with_screen(monkeypatch, FakeScreen()) - stack.import_fails = "ff_legacy: extruder.json could not be read" - a = args(tmp_path, no_screen=False) - assert imp.run(a) == 1 - assert any("REFUSED FF_IMPORT_FIRMWARE_CONFIG" in d - for d in _details(screen)) - - -def test_empty_factory_data_says_so(tmp_path, stack, monkeypatch): - screen = with_screen(monkeypatch, FakeScreen()) - stack.import_lands = False - a = args(tmp_path, no_screen=False) - assert imp.run(a) == 1 - assert any("NO CALIBRATION FOUND" in d for d in _details(screen)) - - -def test_a_save_that_did_not_take_says_so(tmp_path, stack, monkeypatch): - screen = with_screen(monkeypatch, FakeScreen()) - stack.save_persists = False - a = args(tmp_path, no_screen=False) - assert imp.run(a) == 1 - assert any("DID NOT SAVE" in d for d in _details(screen)) - - -def test_a_pending_save_says_so(tmp_path, stack, monkeypatch): - screen = with_screen(monkeypatch, FakeScreen()) - stack.save_pending = True - a = args(tmp_path, no_screen=False) - assert imp.run(a) == 1 - assert any("ALREADY WAITING" in d for d in _details(screen)) - - -def test_every_reason_points_at_the_log(tmp_path, stack, monkeypatch): - screen = with_screen(monkeypatch, FakeScreen()) - stack.import_lands = False - assert imp.run(args(tmp_path, no_screen=False)) == 1 - assert all(imp.LOGFILE in d for d in _details(screen)) - - -def test_every_reason_is_drawable(tmp_path): - # A reason containing a character the font has no glyph for would render - # as a gap exactly where the explanation is. - import importlib.util - spec = importlib.util.spec_from_file_location( - "ffscreen", os.path.join(ROOT, "pkgs", "anvil-core", "payload", "bin", "ffscreen.py")) - ffscreen = importlib.util.module_from_spec(spec) - spec.loader.exec_module(ffscreen) - reasons = [ - "MOONRAKER IS NOT RESPONDING", "KLIPPER REPORTED AN ERROR", - "KLIPPER DID NOT FINISH STARTING (STARTUP)", - "COULD NOT READ THE TOOL SETTINGS", - "COULD NOT READ THE PRINTER CONFIGURATION", - "ANOTHER CONFIGURATION SAVE WAS ALREADY WAITING", - "THE PRINTER REFUSED FF_IMPORT_FIRMWARE_CONFIG", - "NO CALIBRATION FOUND IN THE FACTORY DATA", - "KLIPPER DID NOT RESTART AFTER SAVING", - "THE CALIBRATION DID NOT SAVE", - imp.LOGFILE, imp.RETRY, - ] - for reason in reasons: - missing = set(reason.upper()) - set(ffscreen.FONT) - assert not missing, "%r has no glyph for %s" % (reason, missing) - - # -- waiting for the toolhead boards, every boot --------------------------- # # ff_mcu_bringup.py is a module here, not a subprocess and not a file to poll: @@ -570,104 +273,6 @@ def with_bringup(monkeypatch, fake): return fake -def test_the_boards_are_handed_over_before_klipper(tmp_path, stack, monkeypatch): - fake = with_bringup(monkeypatch, FakeBringup()) - assert imp.run(args(tmp_path)) == 0 - assert fake.calls, "the bring-up was never called" - assert fake.calls[0][0] == FakeBringup.DEFAULT_PORTS - - -def test_the_panel_names_the_board_being_waited_for(tmp_path, stack, monkeypatch): - screen = with_screen(monkeypatch, FakeScreen()) - with_bringup(monkeypatch, FakeBringup(script=[["/dev/ttyS4"]])) - assert imp.run(args(tmp_path, no_screen=False)) == 0 - assert any("THE HEATER BOARD" in s for _, s, _, _ in screen.frames) - - -def test_both_boards_are_named_while_both_are_working(tmp_path, stack, - monkeypatch): - screen = with_screen(monkeypatch, FakeScreen()) - with_bringup(monkeypatch, - FakeBringup(script=[["/dev/ttyS4", "/dev/ttyS7"]])) - assert imp.run(args(tmp_path, no_screen=False)) == 0 - said = " ".join(s for _, s, _, _ in screen.frames) - assert "THE HEATER BOARD" in said and "THE LEVEL BOARD" in said - - -def test_a_bringup_that_raises_does_not_stop_the_boot(tmp_path, stack, - monkeypatch): - # A board is klippy's news to break, with a better message than we have. - with_bringup(monkeypatch, FakeBringup(explode=True)) - a = args(tmp_path) - assert imp.run(a) == 0 - assert os.path.exists(a.stamp) - - -def test_a_bringup_reporting_a_problem_still_tries_klipper(tmp_path, stack, - monkeypatch): - with_bringup(monkeypatch, FakeBringup(result=False)) - a = args(tmp_path) - assert imp.run(a) == 0 - assert os.path.exists(a.stamp) - - -def test_no_bringup_module_is_survivable(tmp_path, stack, monkeypatch): - monkeypatch.setattr(imp, "bringup", None) - a = args(tmp_path) - assert imp.run(a) == 0 - assert os.path.exists(a.stamp) - - -def test_board_names_are_human_readable(): - assert imp.board_name("/dev/ttyS4") == "THE HEATER BOARD" - assert imp.board_name("eheaterboard") == "THE HEATER BOARD" - assert imp.board_name("/dev/ttyS5") == "THE EXTRUDER BOARD" - assert imp.board_name("/dev/ttyS7") == "THE LEVEL BOARD" - # An unknown port still says something rather than nothing. - assert imp.board_name("/dev/ttyS9") == "/DEV/TTYS9" - - -def test_every_port_the_bringup_owns_has_a_name(): - # If the bring-up gains a port, the panel must not start showing - # /DEV/TTYS9 at someone. - import importlib.util - spec = importlib.util.spec_from_file_location( - "ff_mcu_bringup", - os.path.join(ROOT, "pkgs", "anvil-core", "payload", "bin", "ff_mcu_bringup.py")) - real = importlib.util.module_from_spec(spec) - spec.loader.exec_module(real) - for dev in real.DEFAULT_PORTS: - assert imp.board_name(dev).startswith("THE "), dev - - -# -- klippy's own error names the board ------------------------------------ - - -def test_a_failed_connect_names_the_board_klippy_blamed(tmp_path, stack, - monkeypatch): - # This is the only place the guilty board is named: klippy's - # state_message on a failed connect. - screen = with_screen(monkeypatch, FakeScreen()) - stack.klippy_state = "error" - stack.state_message = ("MCU 'eheaterboard' error during connect\n" - "Once the underlying issue is corrected, use the" - " \"FIRMWARE_RESTART\" command to reset the" - " firmware.") - a = args(tmp_path, no_screen=False, timeout=10.0) - assert imp.run(a) == 1 - assert any("THE HEATER BOARD DID NOT ANSWER" in d for d in _details(screen)) - - -def test_an_error_naming_no_board_still_reports_something(tmp_path, stack, - monkeypatch): - screen = with_screen(monkeypatch, FakeScreen()) - stack.klippy_state = "error" - stack.state_message = "Option 'foo' is not valid in section 'bar'" - a = args(tmp_path, no_screen=False, timeout=10.0) - assert imp.run(a) == 1 - assert any("KLIPPER REPORTED AN ERROR" in d for d in _details(screen)) - - # -- owning klipper's start ------------------------------------------------ # # Owned here rather than by a shell wrapper tailing printer.log, which is the @@ -724,70 +329,151 @@ def klipper_args(tmp_path, **over): return args(tmp_path, **over) -def test_it_starts_klipper_and_tells_it_the_bringup_is_done(tmp_path, stack, - monkeypatch): - stack.klippy_state = "startup" - fake = with_klipper(monkeypatch, FakeKlipper(stack)) - with_bringup(monkeypatch, FakeBringup()) - assert imp.run(klipper_args(tmp_path)) == 0 - assert fake.restarts == 1 +# -- the happy path, and the stamp discipline that guards it --------------- -def test_klipper_already_running_is_not_started_again(tmp_path, stack, - monkeypatch): - fake = with_klipper(monkeypatch, FakeKlipper(stack)) - fake.running = True - with_bringup(monkeypatch, FakeBringup()) - assert imp.run(klipper_args(tmp_path)) == 0 - assert fake.restarts == 0 +def test_first_boot_imports_saves_and_stamps(tmp_path, stack): + a = args(tmp_path) + assert imp.run(a) == 0 + assert stack.scripts == ["FF_IMPORT_FIRMWARE_CONFIG", "SAVE_CONFIG"] + assert os.path.exists(a.stamp) + assert stack.calibrated -def test_a_failed_connect_is_retried_with_a_fresh_handover(tmp_path, stack, - monkeypatch): - # The whole point of owning this: the retry hands the boards over again - # before reopening the ports, which is what gives a board that missed its - # window another chance. +def test_no_stamp_unless_the_values_are_verifiably_saved(tmp_path, stack, capsys): + """The four ways the migration can fail, and the one rule for all of them. + + A stamp is a promise that this printer has its factory calibration. Write + it on an import that errored, an import that changed nothing, a save that + did not persist, or a klippy that never came back, and the printer is + stranded: every later boot sees the stamp, skips the migration, and the + numbers are gone for good. Failing without a stamp costs one more boot. + """ + def fresh(**over): + s = Stack() + monkey.setattr(imp.Moonraker, "_open", + lambda self, req, timeout: s.open(req, timeout)) + for k, v in over.items(): + setattr(s, k, v) + return s + + monkey = pytest.MonkeyPatch() + try: + # klippy restarts on SAVE_CONFIG and this one never comes back + def restart_never_finishes(url): + if "SAVE_CONFIG" in "".join(never.scripts): + never.klippy_state = "startup" + + never = fresh() + never.on_request = restart_never_finishes + + cases = [ + ("the import errored", + fresh(import_fails="ff_legacy: extruder.json could not be read"), + ["FF_IMPORT_FIRMWARE_CONFIG"]), + ("the import landed nothing", fresh(import_lands=False), + ["FF_IMPORT_FIRMWARE_CONFIG"]), + ("the save did not persist", fresh(save_persists=False), + ["FF_IMPORT_FIRMWARE_CONFIG", "SAVE_CONFIG"]), + ("klipper never came back", never, + ["FF_IMPORT_FIRMWARE_CONFIG", "SAVE_CONFIG"]), + ] + for why, scripted, expect_scripts in cases: + monkey.setattr(imp.Moonraker, "_open", + lambda self, req, timeout, s=scripted: s.open(req, timeout)) + a = args(tmp_path, stamp=str(tmp_path / ("stamp-" + why.split()[1])), + timeout=10.0) + assert imp.run(a) == 1, why + assert scripted.scripts == expect_scripts, why + assert not os.path.exists(a.stamp), ( + "stamped after %s -- this printer will never migrate again" % why) + finally: + monkey.undo() + + # The klipper-side message is the whole diagnosis; it must reach the log. + assert "extruder.json could not be read" in capsys.readouterr().out + + +def test_the_migration_runs_once_and_only_when_there_is_something_to_do( + tmp_path, stack): + """Three ways to have nothing to do. Re-running the import on a printer + that has been tuned since would overwrite the tuning with factory + numbers, so 'already done' has to be recognised from three directions: + our own stamp, a printer klippy already reports as calibrated, and no + factory JSON to read in the first place.""" + a = args(tmp_path) + assert imp.run(a) == 0 + stack.scripts = [] + assert imp.run(a) == 0 # the wait still happens; the migration does not + assert stack.scripts == [] + + stack.calibrated = True + b = args(tmp_path, stamp=str(tmp_path / "stamp-cal")) + assert imp.run(b) == 0 + assert stack.scripts == [] + assert os.path.exists(b.stamp), "nothing left for a later boot to do" + + empty = tmp_path / "empty" + empty.mkdir() + stack.calibrated = False + c = args(tmp_path, dir=str(empty), stamp=str(tmp_path / "stamp-empty")) + assert imp.run(c) == 0 + assert stack.scripts == [] + # NOT stamped: plugging the stock config back in should still be importable. + assert not os.path.exists(c.stamp) + + +def test_someone_elses_pending_save_stands_us_down(tmp_path, stack): + """Our SAVE_CONFIG would write their half-finished changes to + printer.cfg and restart klippy under them.""" + stack.save_pending = True + a = args(tmp_path) + assert imp.run(a) == 1 + assert stack.scripts == [] + assert not os.path.exists(a.stamp) + + +def test_a_missing_hard_dependency_means_no_import_and_no_stamp(tmp_path, stack): + """Neither moonraker nor klipper being up is a reason to give up on the + migration -- only a reason to leave it for the next boot.""" + for broken in ("moonraker", "klipper"): + stack.moonraker = broken != "moonraker" + stack.klippy_state = "startup" if broken == "klipper" else "ready" + stack.scripts = [] + a = args(tmp_path, timeout=10.0, + stamp=str(tmp_path / ("stamp-" + broken))) + assert imp.run(a) == 1, broken + assert stack.scripts == [], broken + assert not os.path.exists(a.stamp), broken + + +# -- the every-boot half: the boards, then klipper ------------------------- + + +def test_the_boards_are_handed_over_before_klipper_and_again_on_every_retry( + tmp_path, stack, monkeypatch): + """Why this program owns klippy's start at all. + + klippy opening a port at a board still sitting in its bootloader is the + failure this exists to prevent, and the retry that fixes a board which + missed its window is 'hand it over AGAIN, then reopen the port'. A retry + loop that restarts klippy without re-running the bring-up would spin + three times and strand the printer just the same. + """ stack.klippy_state = "error" fake = with_klipper(monkeypatch, FakeKlipper(stack, after=3)) bringups = with_bringup(monkeypatch, FakeBringup()) assert imp.run(klipper_args(tmp_path, klipper_tries=3)) == 0 + assert bringups.calls[0][0] == FakeBringup.DEFAULT_PORTS assert fake.restarts == 3 - assert len(bringups.calls) == 3 + assert len(bringups.calls) == 3, "a restart without a fresh handover" -def test_it_gives_up_after_the_configured_tries(tmp_path, stack, monkeypatch): - stack.klippy_state = "error" - fake = with_klipper(monkeypatch, FakeKlipper(stack, comes_up=False)) - with_bringup(monkeypatch, FakeBringup()) - a = klipper_args(tmp_path, klipper_tries=2, timeout=30.0) - assert imp.run(a) == 1 - assert fake.restarts == 2 - assert not os.path.exists(a.stamp) - - -def test_the_fault_frame_only_appears_once_at_the_end(tmp_path, stack, - monkeypatch): - # A fault frame between attempts would be replaced two seconds later and - # read as a failure the printer then ignored. - screen = with_screen(monkeypatch, FakeScreen()) - stack.klippy_state = "error" - stack.state_message = "MCU 'eheaterboard' error during connect" - with_klipper(monkeypatch, FakeKlipper(stack, comes_up=False)) - with_bringup(monkeypatch, FakeBringup()) - a = klipper_args(tmp_path, no_screen=False, klipper_tries=3, timeout=30.0) - assert imp.run(a) == 1 - faults = [d for _, _, _, d in screen.frames if "DETAILS IN" in d] - assert len(faults) == 1 - assert "THE HEATER BOARD DID NOT ANSWER" in faults[0] - # and the attempts were narrated on the way - assert any("RETRYING" in s for _, s, _, _ in screen.frames) - - -def test_an_s6_svc_that_is_not_there_is_reported_not_ignored(tmp_path, stack, - monkeypatch): - # No s6-svc means klippy cannot be restarted, so a board that missed its - # window never gets a second chance -- the worst outcome the mod has. It - # must reach the panel, not just the log. +def test_klipper_that_cannot_be_started_is_reported_not_ignored( + tmp_path, stack, monkeypatch): + """No s6-svc means klippy cannot be restarted, so a board that missed its + window never gets a second chance -- the worst outcome the mod has. It + must reach the panel, not just the log.""" stack.klippy_state = "startup" screen = with_screen(monkeypatch, FakeScreen()) with_bringup(monkeypatch, FakeBringup()) @@ -795,44 +481,28 @@ def test_an_s6_svc_that_is_not_there_is_reported_not_ignored(tmp_path, stack, a = args(tmp_path, no_screen=False, no_klipper=False, timeout=20.0, s6_svc=str(tmp_path / "absent-s6-svc")) assert imp.run(a) == 1 - assert any("KLIPPER COULD NOT BE STARTED" in d for d in _details(screen)) + details = [d for _, _, _, d in screen.frames if d] + assert any("KLIPPER COULD NOT BE STARTED" in d for d in details), details -def test_no_klipper_only_waits(tmp_path, stack, monkeypatch): - # What --no-klipper is for: let something else own the start. - fake = with_klipper(monkeypatch, FakeKlipper(stack)) - with_bringup(monkeypatch, FakeBringup()) - assert imp.run(args(tmp_path, no_klipper=True)) == 0 - assert fake.restarts == 0 - - -# -- the imports resolve when it is run the way the wrapper runs it --------- - - -def test_selftest_reports_both_siblings(capsys): - assert imp.selftest() == 0 - out = capsys.readouterr().out - assert "ffscreen: yes" in out - assert "ff_mcu_bringup: yes" in out - assert "ports: 3" in out - assert "selftest: ok" in out - - -def test_selftest_fails_loudly_when_the_bringup_is_missing(monkeypatch, capsys): - # Silent is the dangerous outcome: no bring-up means klippy opens the - # ports at boards still sitting in their bootloaders. - monkeypatch.setattr(imp, "bringup", None) - assert imp.selftest() == 1 - assert "ff_mcu_bringup: NO" in capsys.readouterr().out +# -- it has to import its siblings when run the way the wrapper runs it ---- def test_run_by_path_from_elsewhere_finds_its_siblings(tmp_path): - # The whole premise of importing by plain name: python puts the SCRIPT's - # directory on sys.path, not the caller's. Run it from somewhere else - # entirely and the imports must still resolve. + """The whole premise of importing ffscreen and ff_mcu_bringup by plain + name: python puts the SCRIPT's directory on sys.path, not the caller's. + Run from somewhere else entirely, the imports must still resolve -- and + a missing bring-up must be loud, because silent means klippy opens the + ports at boards still in their bootloaders.""" import subprocess script = os.path.join(ROOT, "pkgs", "anvil-core", "payload", "bin", "ff-startup.py") out = subprocess.run([sys.executable, script, "--selftest"], cwd=str(tmp_path), capture_output=True, text=True) assert out.returncode == 0, out.stdout + out.stderr - assert "selftest: ok" in out.stdout + for expect in ("ffscreen: yes", "ff_mcu_bringup: yes", "ports: 3", + "selftest: ok"): + assert expect in out.stdout, out.stdout + + with pytest.MonkeyPatch.context() as m: + m.setattr(imp, "bringup", None) + assert imp.selftest() == 1 diff --git a/test/integration/test_tool_transform.py b/test/integration/test_tool_transform.py index ad33c40..67c0146 100644 --- a/test/integration/test_tool_transform.py +++ b/test/integration/test_tool_transform.py @@ -7,11 +7,14 @@ the tool's ~3.2 mm nozzle-to-station gap along with the babystep. It is a move transform below gcode_move now, so the two layers are independent: homing_origin is the operator's, the transform is the tool's. -What these pin down is the arithmetic and the bookkeeping around it. The -transform itself is four lines; everything that can go wrong is at the -edges -- a frame left applied while probing in machine coordinates, a -gcode_move position cache not invalidated when the frame changed, an offset -read from a snapshot instead of live. +WHAT IS LEFT. This was twenty-four tests and is seven. Everything kept is a +mistake whose symptom is a MOVE TO THE WRONG PLACE -- an offset with the wrong +sign, a frame still applied while probing in machine coordinates, a gcode_move +cache not invalidated when the frame changed, or a klippy that will not parse +its config on any printer that has been calibrated. The bookkeeping around +them -- z_adjust staging for SAVE_CONFIG, the per-job Z term, the +uncalibrated-tool warning -- is dropped: those are wrong numbers reported to a +person, not a nozzle in the bed. gcode_move and the toolhead are faked. They are Klipper's, not ours, and _ToolTransform touches exactly two things on each: move()/get_position() @@ -35,7 +38,6 @@ def _load(name): ff_toolchange = _load("ff_toolchange") -ff_tool = _load("ff_tool") class FakeToolhead: @@ -77,20 +79,6 @@ class FakeGcode: self.messages.append(message) -class FakeGcmd: - """Only get_int/respond_info -- the clear path needs nothing else.""" - - def __init__(self, params): - self.params = params - self.messages = [] - - def get_int(self, name, default=None, minval=None, maxval=None): - return int(self.params.get(name, default)) - - def respond_info(self, message): - self.messages.append(message) - - class FakeTool: """Only what _set_tool_frame's calibration warning asks about.""" @@ -125,13 +113,12 @@ OFFSETS = [(0.0, 0.0, 3.15), (-0.30, 0.07, 3.19), # ------------------------------------------------------------- arithmetic -def test_no_tool_passes_the_move_through_untouched(): - tc = make(OFFSETS) - tc.gcode_transform.move([10., 20., 5., 1.], 60.) - assert tc.gcode_transform.next_transform.moves[0][0] == [10., 20., 5., 1.] - - def test_a_mounted_tool_adds_its_offsets(): + """The signs, against numbers worked out by hand. + + The round trip below cannot see this: an offset applied with the wrong + sign and removed with the wrong sign agrees with itself perfectly. + """ tc = make(OFFSETS) tc.gcode_transform.tool = 1 tc.gcode_transform.move([10., 20., 5., 1.], 60.) @@ -139,18 +126,9 @@ def test_a_mounted_tool_adds_its_offsets(): assert sent[:3] == pytest.approx([9.70, 20.07, 8.19]) -def test_the_extruder_axis_is_never_touched(): - # newpos[3:] is the extruder position -- adding a Z offset to it would - # extrude the offset. - tc = make(OFFSETS) - tc.gcode_transform.tool = 3 - tc.gcode_transform.move([10., 20., 5., 42.5], 60.) - assert tc.gcode_transform.next_transform.moves[0][0][3] == 42.5 - - def test_get_position_undoes_what_move_did(): - # The round trip is the whole contract: G-code coordinates stay - # tool-independent, so M114 reports the frame the file is written in. + """The round trip is the whole contract: G-code coordinates stay + tool-independent, so M114 reports the frame the file is written in.""" tc = make(OFFSETS) for tool in range(len(OFFSETS)): tc.gcode_transform.tool = tool @@ -159,29 +137,21 @@ def test_get_position_undoes_what_move_did(): assert tc.gcode_transform.get_position() == pytest.approx(asked) -def test_get_position_with_no_tool_is_the_machine_position(): +def test_the_extruder_axis_is_never_touched(): + """newpos[3:] is the extruder position -- adding a Z offset to it would + extrude the offset.""" tc = make(OFFSETS) - tc.gcode_transform.next_transform.position = [1., 2., 3., 4.] - assert tc.gcode_transform.get_position() == [1., 2., 3., 4.] - - -def test_offsets_are_read_live_not_snapshotted(): - # This is what lets a per-tool Z tune apply mid-print: refresh_offsets() - # rewrites the lists and the frame follows, with no re-apply and no - # config write. - tc = make(OFFSETS) - tc.gcode_transform.tool = 2 - tc.offset_z[2] = 9.99 - tc.gcode_transform.move([0., 0., 0., 0.], 60.) - assert tc.gcode_transform.next_transform.moves[0][0][2] == pytest.approx(9.99) + tc.gcode_transform.tool = 3 + tc.gcode_transform.move([10., 20., 5., 42.5], 60.) + assert tc.gcode_transform.next_transform.moves[0][0][3] == 42.5 # ---------------------------------------------------------- the frame API def test_selecting_a_tool_invalidates_the_position_cache(): - # gcode_move caches last_position from position_with_transform(). Change - # the frame without resetting it and the next move starts from a point - # the toolhead is not at. + """gcode_move caches last_position from position_with_transform(). Change + the frame without resetting it and the next move starts from a point the + toolhead is not at.""" tc = make(OFFSETS) before = tc.gcode_move.resets tc._set_tool_frame(1) @@ -190,9 +160,11 @@ def test_selecting_a_tool_invalidates_the_position_cache(): def test_suspending_the_frame_drops_to_machine_coordinates(): - # The offset calibration probes in raw coordinates. Zeroing Klipper's - # G-code offset does NOT reach this layer. + """The offset calibration and the dock moves address the machine. Zeroing + Klipper's G-code offset does NOT reach this layer, and the per-job Z term + has to come off with the tool's -- both are the frame.""" tc = make(OFFSETS, mounted=1) + tc.job_z = 0.04 tc._set_tool_frame(1) assert tc.suspend_tool_frame() == 1 assert tc.gcode_transform.tool is None @@ -201,167 +173,44 @@ def test_suspending_the_frame_drops_to_machine_coordinates(): def test_restoring_the_frame_asks_the_sensors_not_the_memory(): - # After a calibration the carriage may hold a different tool than the - # one suspended -- or none. What is mounted is the honest answer. + """After a calibration the carriage may hold a different tool than the one + suspended -- or none. What is mounted is the honest answer, and an + unanswerable question leaves no frame rather than the last one.""" tc = make(OFFSETS, mounted=3) tc._set_tool_frame(1) tc.suspend_tool_frame() assert tc.restore_tool_frame() is True assert tc.gcode_transform.tool == 3 - -def test_restoring_with_an_empty_carriage_leaves_no_frame(): - tc = make(OFFSETS, mounted=-1) - tc._set_tool_frame(2) - assert tc.restore_tool_frame() is False - assert tc.gcode_transform.tool is None - - -def test_restoring_when_the_sensors_cannot_say_leaves_no_frame(): - tc = make(OFFSETS, mounted=None) - tc._set_tool_frame(2) - assert tc.restore_tool_frame() is False - assert tc.gcode_transform.tool is None - - -# ------------------------------------------------- the third layer, z_adjust - -class FakeConfigfile: - def __init__(self): - self.staged = [] - - def set(self, section, option, value): - self.staged.append((section, option, value)) - - -def make_tool(z_adjust=0.0): - tool = ff_tool.FFTool.__new__(ff_tool.FFTool) - tool.name = "ff_tool 2" - tool.index = 2 - tool.z_adjust = z_adjust - configfile = FakeConfigfile() - tool.printer = FakePrinter(None) - tool.printer.objects["configfile"] = configfile - return tool, configfile - - -def test_a_z_adjust_takes_effect_without_being_saved(): - # The whole point: a first layer is going down and SAVE_CONFIG is a - # restart. Applying and persisting are separate acts. - tool, configfile = make_tool() - tool.set_z_adjust(-0.02) - assert tool.z_adjust == pytest.approx(-0.02) - assert configfile.staged == [] - - -def test_saving_a_z_adjust_stages_it_for_save_config(): - tool, configfile = make_tool() - tool.set_z_adjust(-0.02, save=True) - assert tool.z_adjust == pytest.approx(-0.02) - assert configfile.staged == [("ff_tool 2", "z_adjust", "-0.020000")] - - -def test_a_saved_z_adjust_is_still_live_immediately(): - # SAVE=1 must not mean "on the next restart". - tool, configfile = make_tool(z_adjust=0.1) - tool.set_z_adjust(0.25, save=True) - assert tool.z_adjust == pytest.approx(0.25) - assert len(configfile.staged) == 1 + for sensors_say in (-1, None): # empty carriage, no answer + tc = make(OFFSETS, mounted=sensors_say) + tc._set_tool_frame(2) + assert tc.restore_tool_frame() is False + assert tc.gcode_transform.tool is None # ------------------------------------------- the transform exists in time -def test_the_transform_is_built_before_the_first_refresh(): - """refresh_offsets() reads gcode_transform to decide whether - gcode_move's cache needs invalidating, and __init__ calls it. On a - calibrated machine the derived offsets differ from the [0.0] seed, so - the read happens -- and a transform created further down __init__ meant - AttributeError at config parse on every printer that had ever been - calibrated. The uncalibrated case, where nothing differs and the read - is short-circuited, is exactly the one the replica boots in.""" +def test_init_builds_the_transform_before_it_is_read(): + """Two ways __init__ has broken klippy at config parse, both static. + + refresh_offsets() reads gcode_transform to decide whether gcode_move's + cache needs invalidating, and __init__ calls it. On a calibrated machine + the derived offsets differ from the [0.0] seed, so the read happens -- and + a transform created further down __init__ meant AttributeError at config + parse on every printer that had ever been calibrated. The uncalibrated + case, where nothing differs and the read is short-circuited, is exactly + the one the replica boots in, so no replica gate can see this. + + And `self.offset_x = self.offset_y = self.offset_z = [0.] * N` binds ONE + list to three names, so a later write to any of them writes all three -- + every tool would get X's offset on all three axes. + """ source = inspect.getsource(ff_toolchange.FFToolchange.__init__) built = source.index("self.gcode_transform = _ToolTransform(self)") refreshed = source.index("self.refresh_offsets()") - assert built < refreshed - - -def test_the_three_offset_series_are_three_lists(): - """self.offset_x = self.offset_y = self.offset_z = [0.] * N binds ONE - list to three names, so a later write to any of them writes all three.""" - source = inspect.getsource(ff_toolchange.FFToolchange.__init__) - assert "self.offset_x = self.offset_y" not in source - - -def test_refreshing_before_the_transform_is_installed_does_not_reset(): - tc = make(OFFSETS) - tc.gcode_transform.next_transform = None - tc.offset_base = 0 - tc.tools = [FakeTool() for _ in OFFSETS] - tc._station_z = lambda: None - tc._derive_offsets = lambda base: ([9.] * 4, [9.] * 4, [9.] * 4) - assert tc.refresh_offsets() is True - assert tc.gcode_move.resets == 0 - - -# ------------------------------------------------------ the job Z is its own - -def test_the_job_term_rides_on_the_tool_frame(): - tc = make(OFFSETS) - tc.gcode_transform.tool = 1 - tc.job_z = 0.04 - tc.gcode_transform.move([10., 20., 5., 1.], 60.) - sent = tc.gcode_transform.next_transform.moves[0][0] - assert sent[2] == pytest.approx(5. + 3.19 + 0.04) - - -def test_the_job_term_is_off_in_raw_machine_coordinates(): - """Dock moves and the offset calibration address the machine. Both - suspend the frame, and the job term has to go with it.""" - tc = make(OFFSETS) - tc.job_z = 0.04 - tc.gcode_transform.move([10., 20., 5., 1.], 60.) - assert tc.gcode_transform.next_transform.moves[0][0] == [10., 20., 5., 1.] - - -def test_clearing_the_print_offset_leaves_the_tool_frame_alone(): - tc = make(OFFSETS) - tc.gcode_transform.tool = 2 - tc.job_z = 0.04 - gcmd = FakeGcmd({"CLEAR": 1}) - tc.cmd_TOOLCHANGE_SET_PRINT_OFFSET(gcmd) - assert tc.job_z == 0.0 - assert tc.gcode_transform.tool == 2 - assert tc.gcode_transform._offsets()[2] == pytest.approx(3.11) - assert "0.040" in gcmd.messages[0] - - -def test_the_print_offset_is_absolute_not_cumulative(): - """END_PRINT used to clear this with SET_GCODE_OFFSET Z=0, which took - the operator's babystep with it. A per-layer caller must also not be - able to stack the term on itself.""" - tc = make(OFFSETS) - tc.job_z = 0.04 - source = inspect.getsource( - ff_toolchange.FFToolchange.cmd_TOOLCHANGE_SET_PRINT_OFFSET) - assert "self.job_z = z" in source - assert "self.job_z +=" not in source - - -# ------------------------------------------------- an uncalibrated tool warns - -def test_applying_an_uncalibrated_frame_says_z_zero_is_below_the_bed(): - tc = make(OFFSETS, calibrated=False) - tc._set_tool_frame(2) - assert any("no nozzle calibration" in m for m in tc.gcode.messages) - - -def test_applying_a_calibrated_frame_is_quiet(): - tc = make(OFFSETS) - tc._set_tool_frame(2) - assert tc.gcode.messages == [] - - -def test_dropping_the_frame_never_warns(): - tc = make(OFFSETS, calibrated=False) - tc._set_tool_frame(None) - assert tc.gcode.messages == [] + assert built < refreshed, ( + "refresh_offsets() runs before the transform exists -- klippy will " + "not parse its config on a calibrated printer") + assert "self.offset_x = self.offset_y" not in source, ( + "the three offset series are chain-assigned, so they are one list") diff --git a/tools/replica/README.md b/tools/replica/README.md new file mode 100644 index 0000000..7e591ab --- /dev/null +++ b/tools/replica/README.md @@ -0,0 +1,49 @@ +# The printer replica + +A container that runs the printer's **own** userland: the real +`rootfs.squashfs` extracted from FlashForge's package, chrooted, with its +MIPS binaries executing under qemu. `uname -m` says mips and busybox applets +behave exactly as they do on the machine. + +It lives under `tools/` rather than `test/` because it is not a test. Three +different things drive it, and only one of them is a suite: + +| driver | what it does with it | +|---|---| +| `bin/patch.sh` -> `bin/build-payload.py` | **the build path.** The payload is assembled by running the PRINTER'S OWN `opkg` against `/`, inside here. Nothing on the host can do that | +| `qa/lib/replica.py` | the `replica` lane's `printer` fixture -- holds one container open and probes it | +| `make rootfs`, `make boot-screen-sim` | unpacking the real userland; rendering the boot frames on the cross-built CPython | + +``` +printer/ the machine itself, SHELL on purpose: it is executed + by the printer's ash under qemu, and that it survives + that is a large part of what the suite proves + Dockerfile the container: qemu-user-static, rsync, openssl, + dosfstools. It contributes no userland worth speaking + of -- everything real comes from the chroot + Dockerfile.full the same, plus the firmware baked in: a prebuilt + PRINTER_IMAGE, so a run does not unpack 93MB + binfmt.sh registers a handler for the printer's binaries. The + stock qemu-mipsel registration does not match them: + its mask requires e_ident[EI_ABIVERSION] == 0 and + every binary the Ingenic toolchain built has 3 there + assemble.sh the mount layout -- read-only root, writable prog and + data partitions, optionally a real FAT USB stick + seed-prog.sh a /usr/prog that looks like a printer's + entrypoint.sh setup, then the case script it was given + entrypoint-out.sh the same, plus a writable /out inside the chroot + bake.sh, bake-case.sh install a package and commit the result as an image +ffsim/ the host half. NOT a test framework -- it was one, + and 1,657 lines of it are gone; what is left starts + containers and reads config.env/test.env +build-printer-image.sh bakes a prebuilt replica image (`make printer-image`) +extract-rootfs.py `make rootfs` +sim-boot-screen.py `make boot-screen-sim` +``` + +`--privileged` buys exactly two things: the binfmt registration, and the mount +layout. Nothing here needs it for any other reason. + +See [docs/printer-replica.md](../../docs/printer-replica.md) for what the +replica can and cannot prove, and [docs/testing.md](../../docs/testing.md) for +the suites. diff --git a/test/integration/build-printer-image.sh b/tools/replica/build-printer-image.sh similarity index 92% rename from test/integration/build-printer-image.sh rename to tools/replica/build-printer-image.sh index b6fbc8a..4718d89 100755 --- a/test/integration/build-printer-image.sh +++ b/tools/replica/build-printer-image.sh @@ -1,9 +1,9 @@ #!/usr/bin/env bash # Build a Docker image that IS the printer. # -# ./test/integration/build-printer-image.sh build -# ./test/integration/build-printer-image.sh --push build and push -# ./test/integration/build-printer-image.sh --no-bake skip the baseline install +# ./tools/replica/build-printer-image.sh build +# ./tools/replica/build-printer-image.sh --push build and push +# ./tools/replica/build-printer-image.sh --no-bake skip the baseline install # # ONE image covers both models. That is not a shortcut -- there is genuinely # nothing model-specific to put in a second one: @@ -23,7 +23,7 @@ # # The Dockerfile downloads the firmware itself, so nothing has to be staged # locally, config.env is not needed, and the build context is just -# test/integration/printer. Override to build against different firmware: +# tools/replica/printer. Override to build against different firmware: # # STOCK_URL= FACTORY_URL= FW_VERSION= # @@ -75,7 +75,7 @@ $DOCKER build -t "$RAW" \ --build-arg "FACTORY_URL=$FACTORY_URL" \ --build-arg "FF_KEY=${FF_KEY:-FFP0331&*%root}" \ --build-arg "FW_VERSION=$FWVER" \ - -f test/integration/printer/Dockerfile.full test/integration/printer + -f tools/replica/printer/Dockerfile.full tools/replica/printer if [ "$BAKE" = 1 ]; then echo diff --git a/test/integration/extract-rootfs.py b/tools/replica/extract-rootfs.py similarity index 92% rename from test/integration/extract-rootfs.py rename to tools/replica/extract-rootfs.py index dac0b9e..c267c79 100755 --- a/test/integration/extract-rootfs.py +++ b/tools/replica/extract-rootfs.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 """Extract the printer's REAL root filesystem from the stock update package. - ./test/integration/extract-rootfs.py + ./tools/replica/extract-rootfs.py The kernel-*.tar.xz component carries ota_kernel_emmc/ota_v1/rootfs.squashfs -- the genuine buildroot rootfs: busybox 1.31.1, /etc/inittab, /etc/init.d @@ -19,7 +19,7 @@ from pathlib import Path for _p in Path(__file__).resolve().parents: if (_p / "bin" / "common.sh").is_file(): - sys.path.insert(0, str(_p / "test")) + sys.path.insert(0, str(_p / "tools" / "replica")) break from ffsim import cli # noqa: E402 diff --git a/test/ffsim/__init__.py b/tools/replica/ffsim/__init__.py similarity index 97% rename from test/ffsim/__init__.py rename to tools/replica/ffsim/__init__.py index 67a02e4..27126bf 100644 --- a/test/ffsim/__init__.py +++ b/tools/replica/ffsim/__init__.py @@ -1,7 +1,7 @@ """Host side of the test harness. Everything in here runs on YOUR machine. The scripts that run on the printer --- test/integration/printer/*.sh and the whole payload -- stay shell on +-- tools/replica/printer/*.sh and the whole payload -- stay shell on purpose: they are executed by the printer's own busybox ash under qemu, and the fact that they survive that is a large part of what the suite proves. Rewriting those in Python would test a Python the printer does not have. diff --git a/test/ffsim/cli.py b/tools/replica/ffsim/cli.py similarity index 100% rename from test/ffsim/cli.py rename to tools/replica/ffsim/cli.py diff --git a/test/ffsim/config.py b/tools/replica/ffsim/config.py similarity index 100% rename from test/ffsim/config.py rename to tools/replica/ffsim/config.py diff --git a/test/ffsim/gates.py b/tools/replica/ffsim/gates.py similarity index 100% rename from test/ffsim/gates.py rename to tools/replica/ffsim/gates.py diff --git a/test/ffsim/replica.py b/tools/replica/ffsim/replica.py similarity index 98% rename from test/ffsim/replica.py rename to tools/replica/ffsim/replica.py index 89667e5..f099d23 100644 --- a/test/ffsim/replica.py +++ b/tools/replica/ffsim/replica.py @@ -80,7 +80,7 @@ class Replica: "in test.env to skip it.") image = "creator5-printer-sim" - build_dir = config.root / "test" / "integration" / "printer" + build_dir = config.root / "tools" / "replica" / "printer" # Always rebuild: a cache hit takes about a second, and a stale image # silently testing yesterday's harness is not a trade worth making. built = subprocess.run( @@ -109,10 +109,10 @@ class Replica: # A case that has to hand something back gets an entrypoint of ours, # which runs the stock one and then mounts /out inside the chroot. - # See test/integration/printer/entrypoint-out.sh for why that cannot + # See tools/replica/printer/entrypoint-out.sh for why that cannot # be a plain -v onto /printer/out. if want_out: - wrapper = self.root / "test" / "integration" / "printer" / "entrypoint-out.sh" + wrapper = self.root / "tools" / "replica" / "printer" / "entrypoint-out.sh" argv += [ "--entrypoint", "/entrypoint-out.sh", "-v", "%s:/entrypoint-out.sh:ro" % wrapper, diff --git a/test/integration/printer/.dockerignore b/tools/replica/printer/.dockerignore similarity index 100% rename from test/integration/printer/.dockerignore rename to tools/replica/printer/.dockerignore diff --git a/test/integration/printer/Dockerfile b/tools/replica/printer/Dockerfile similarity index 82% rename from test/integration/printer/Dockerfile rename to tools/replica/printer/Dockerfile index 6937d38..589d544 100644 --- a/test/integration/printer/Dockerfile +++ b/tools/replica/printer/Dockerfile @@ -1,10 +1,10 @@ # Image that can execute the printer's real MIPS userland. # -# Build context is test/integration/printer/. +# Build context is tools/replica/printer/. # # It carries qemu-mipsel-static and nothing else of consequence: every command # the tests care about is executed by the printer's OWN busybox/coreutils out of -# the extracted rootfs.squashfs, under chroot. See test/integration/printer-exec.py. +# the extracted rootfs.squashfs, under chroot. See ../ffsim/replica.py. FROM debian:bookworm-slim RUN apt-get update -qq \ diff --git a/test/integration/printer/Dockerfile.full b/tools/replica/printer/Dockerfile.full similarity index 97% rename from test/integration/printer/Dockerfile.full rename to tools/replica/printer/Dockerfile.full index 151645c..d94c9a8 100644 --- a/test/integration/printer/Dockerfile.full +++ b/tools/replica/printer/Dockerfile.full @@ -10,7 +10,7 @@ # Everything executed inside is the printer's own MIPS userland running under # qemu-mipsel. See docs/printer-replica.md. # -# Built by test/integration/build-printer-image.sh, which supplies the URLs per +# Built by tools/replica/build-printer-image.sh, which supplies the URLs per # model. # # CONTAINS PROPRIETARY FLASHFORGE FIRMWARE. @@ -75,7 +75,7 @@ COPY --from=fetch /rootfs/ /rootfs/ COPY --from=fetch /parts/ /parts/ # The harness: binfmt registration, mount layout, prog checks, entrypoint. -# Build context is test/integration/printer. +# Build context is tools/replica/printer. COPY . /opt/printer/ RUN chmod +x /opt/printer/*.sh diff --git a/test/integration/printer/assemble.sh b/tools/replica/printer/assemble.sh similarity index 100% rename from test/integration/printer/assemble.sh rename to tools/replica/printer/assemble.sh diff --git a/test/integration/printer/bake-case.sh b/tools/replica/printer/bake-case.sh similarity index 100% rename from test/integration/printer/bake-case.sh rename to tools/replica/printer/bake-case.sh diff --git a/test/integration/printer/bake.sh b/tools/replica/printer/bake.sh similarity index 95% rename from test/integration/printer/bake.sh rename to tools/replica/printer/bake.sh index 7467956..5d04421 100755 --- a/test/integration/printer/bake.sh +++ b/tools/replica/printer/bake.sh @@ -7,7 +7,7 @@ # updater, and a test run starts in under a second. # # This cannot happen in a `docker build` step: the install needs binfmt_misc -# and chroot, so it needs --privileged. test/integration/build-printer-image.sh +# and chroot, so it needs --privileged. tools/replica/build-printer-image.sh # runs this # in a container and commits the result. # diff --git a/test/integration/printer/binfmt.sh b/tools/replica/printer/binfmt.sh similarity index 100% rename from test/integration/printer/binfmt.sh rename to tools/replica/printer/binfmt.sh diff --git a/test/integration/printer/case-boot-screen-dump.sh b/tools/replica/printer/case-boot-screen-dump.sh similarity index 98% rename from test/integration/printer/case-boot-screen-dump.sh rename to tools/replica/printer/case-boot-screen-dump.sh index 109123e..8d664d6 100755 --- a/test/integration/printer/case-boot-screen-dump.sh +++ b/tools/replica/printer/case-boot-screen-dump.sh @@ -12,7 +12,7 @@ # frames are PNG-compressed in here (zlib is stdlib) rather than shipped raw, # because a 1024x600x32 frame is 2.4MB and there are several of them. # -# test/integration/sim-boot-screen.py drives this and writes the PNGs out. +# tools/replica/sim-boot-screen.py drives this and writes the PNGs out. MOD=/usr/data/anvil PY=$MOD/bin/python3.13 mkdir -p $MOD/bin diff --git a/test/integration/printer/case-build-payload.sh b/tools/replica/printer/case-build-payload.sh similarity index 100% rename from test/integration/printer/case-build-payload.sh rename to tools/replica/printer/case-build-payload.sh diff --git a/test/integration/printer/entrypoint-out.sh b/tools/replica/printer/entrypoint-out.sh similarity index 100% rename from test/integration/printer/entrypoint-out.sh rename to tools/replica/printer/entrypoint-out.sh diff --git a/test/integration/printer/entrypoint.sh b/tools/replica/printer/entrypoint.sh similarity index 98% rename from test/integration/printer/entrypoint.sh rename to tools/replica/printer/entrypoint.sh index f4bd26d..62c68a4 100755 --- a/test/integration/printer/entrypoint.sh +++ b/tools/replica/printer/entrypoint.sh @@ -82,7 +82,7 @@ echo "printer-sim: $ID (real rootfs, qemu-mipsel)" # The stock package goes on first: that is the only authentic source for # /usr/prog/klipper, firmwareExe, unTar, app_startup.sh and friends. -# Already baked into the image? test/integration/build-printer-image.sh installs +# Already baked into the image? tools/replica/build-printer-image.sh installs # the stock # package once at build time and records its md5 here, because doing it per run # costs 37 seconds of qemu and real `sleep` calls for a result that is diff --git a/test/integration/printer/seed-prog.sh b/tools/replica/printer/seed-prog.sh similarity index 100% rename from test/integration/printer/seed-prog.sh rename to tools/replica/printer/seed-prog.sh diff --git a/test/integration/sim-boot-screen.py b/tools/replica/sim-boot-screen.py similarity index 93% rename from test/integration/sim-boot-screen.py rename to tools/replica/sim-boot-screen.py index f6f1cec..f99f4c1 100755 --- a/test/integration/sim-boot-screen.py +++ b/tools/replica/sim-boot-screen.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 """Draw the first-boot screen in the replica and write it out as PNGs. - ./test/integration/sim-boot-screen.py [--out DIR] + ./tools/replica/sim-boot-screen.py [--out DIR] ffscreen.py packs pixels by hand, and the only honest way to review that is to look at the result -- rendered by the interpreter that will really run it. @@ -22,7 +22,7 @@ from pathlib import Path for _p in Path(__file__).resolve().parents: if (_p / "bin" / "common.sh").is_file(): ROOT = _p - sys.path.insert(0, str(_p / "test")) + sys.path.insert(0, str(_p / "tools" / "replica")) break from ffsim import cli # noqa: E402 @@ -30,7 +30,7 @@ from ffsim.config import Config # noqa: E402 from ffsim.replica import Replica # noqa: E402 from ffsim.gates import _python_tarball # noqa: E402 -CASE = ("test", "integration", "printer", "case-boot-screen-dump.sh") +CASE = ("tools", "replica", "printer", "case-boot-screen-dump.sh") FRAME = re.compile(r"PNGSTART (\S+)\n(.*?)\nPNGEND", re.S)