Repository navigation
Widen CI coverage - #72
Merged
Merged
Conversation
The pure logic that decides what a server gets written and which Java runs it had no tests of its own, only the end-to-end checks against a built image. `npm test` now runs node:test over test/unit, with no new dependency and no install step, and CI runs it alongside the image build: - server.properties: parsing (escapes, spacing, CRLF), a new file's defaults and first-start settings per version, merging into a modpack's file line for line, in-place updates, and the Properties page's defaults agreeing with a new server's; - Minecraft version ids: the id pattern, isAtLeast across releases, snapshots and pre-releases, build ordering and world type spellings; - the Java major each version needs, including snapshots and NeoForge's pseudo 1.26 ids; - NeoForge's build to Minecraft version mapping, through listVersions and getBuilds with Maven's list stubbed, and the pre-1.2.1 relabel; - path containment (traversal, look-alike siblings, links out) and the zip magic-number check. The glob is spelt out because a bare `node --test` would also pick up the test/ci scripts, which need a running panel.
The image only copies src/, views/, public/ and the package files, so none of these ever reached it, but they were still sent to the builder on every build. Ignoring them keeps the context to what the image uses, and a later broad COPY can't pull the tests or the README screenshots in by accident.
Every check that creates a server depends on Mojang, PaperMC, the Forge and NeoForge Mavens, Fabric or Purpur, with no retries, so a brief outage anywhere failed the run and looked exactly like a Craftbox regression. lib.mjs now has: - upstreamFetch(), for the tests' own calls out: network errors, timeouts, 429 and 5xx are retried with backoff; - withUpstream(), for anything that has Craftbox call out: a failure that reads like the network's (Craftbox's own "HTTP 503", "fetch failed" or "timed out", or a Forge installer's Java socket exceptions) is retried once after a pause. If it persists, the runner probes the upstreams involved itself. Only when the runner can't reach one either is it reported as an outage; otherwise it stays a Craftbox failure, noted as such; - provisionServer(), which creates a server and waits it out through withUpstream, deleting a failed attempt's server before the next. The API smoke and server start tests use them. An outage still fails the job, so nothing merges on a check that never ran, but the log, the annotation and a section of its own in the step summary all say "Upstream unavailable", naming the service. Annotation titles are now escaped as workflow commands require: a check name with a comma, such as "creates, writes, reads...", used to cut the title short, and multi-line errors keep their line breaks.
No page had been rendered in CI beyond three form checks, and nothing covered how the panel handles a browser's session. The new web test runs on a fresh instance, over plain HTTP: - before setup, pages go to the wizard and the API answers 503; each of the setup form's validation messages is shown, a form without its CSRF token is refused, and setup can't be run twice; - every page renders with its title and in full, and every script, stylesheet and image they load is served with the right type; the security headers are sent and the inline script carries the CSP nonce; unknown servers, bad paths and a vanilla server's plugins page get the error pages, and signed-out visitors are sent to sign in; - a wrong password is turned away with its message; signing in changes the session id and returns to the page that asked; the cookie is HttpOnly, SameSite=Strict and refreshed to an hour on each request; - the API takes a session for reads, but a change without the page's token, with a wrong one or with another session's is a 403, as is a signed-in form posted without it; signing out needs the token; - a session aged past its expiry in the container's database is signed out, and a form posted from it goes to sign in rather than a 403; - /status lists only servers whose status page is public, while any server's page and JSON are reachable by its link as documented, with only the documented fields, and a provisioning server shows as stopped; the client-mods zip leaves out server-only mods and carries an exact Content-Length; - last, since it locks the address out for 15 minutes, the sixth sign-in inside the window is refused with 429, even with the right password. It runs as the first entry of a new matrix job, with the container's name passed in for the checks that reach inside it. lib.mjs gains a browser-like session (bootstrapPanel now builds on it), loginPanel(), test jars for each loader, a zip listing, the container helpers and a WebSocket client (openSocket, upgradeStatus) for the checks that follow.
The panel's live pages all run on the WebSocket, and none of it was tested. The new WebSocket test follows docs/API.md's protocol section, with a vanilla server started for real: - the upgrade is refused (401) with no cookie, a forged one or an API key, and opens for a session and publicly on /ws/status; - ping gets a pong, and bad JSON, an unknown type, a subscribe without a server and a command without a line each get their error; - creating or regrouping a server tells every signed-in socket, echoing the request's X-Client-Id as the origin, and no public one; - subscribing sends a snapshot, without history or crash details on the public socket; a file change is announced with its directory; - a start streams starting then running, the console through "Done" and the started event, while the public socket gets the state and event but no console output; - a `command` message (field `line`) runs and its reply streams back; the public socket's commands are refused and never run; - a stop streams stopping then stopped; a backup is reported as an `operation`; clearing the event log is announced; and nothing more arrives after unsubscribing.
Checking the HTML says nothing about whether the public/js scripts that drive every page actually run. The new browser test loads the panel in headless Chrome, driven over the DevTools protocol with Node's own WebSocket (chrome.mjs), so it needs no Puppeteer or Playwright, only the Chrome the runners already have. Every page must load without an uncaught exception, a console error, a CSP violation, a failed request, an error response or an error toast. Then the main flows go through the real UI: - signing in through the login form; - creating a server on the create page, with the newest version it fills in, and landing on its console; - starting it with the Start button, sending `list` from the console input and reading the reply, then stopping it; - saving a file in the editor, Properties (plainly, and behind a restore-point backup, which completes over the WebSocket), and Settings from its Advanced Options. Typing into a field a user couldn't reach, such as one in a collapsed section, is an error rather than silently doing nothing. The page list now lives in lib.mjs, shared with the web test.
Nothing checked what the panel does to a server over time: restarts, crashes, upgrades or restoring a backup. The new operations test covers that on vanilla servers started for real, in two sections that run as separate jobs. Lifecycle: - a version upgrade from the previous release to the latest, over the WebSocket, refusing a bad version and downgrades (through Settings too); - restart, with the `restarting` flag on its stopped broadcast, restart behind a backup, and kill, which is a clean stop; - a running server refusing a backup without stopFirst, a jar upgrade, delete, rename and another start; - crash detection with the JVM made to exit 143 inside the container, its crash details kept off the public status JSON, and auto-restart; - a restore-point Settings save on a running server, which restarts it; - auto-start across a panel (container) restart, only when it's on. Backups: a backup and its download, a restore over later changes (files added since are gone), a restore-point Properties save, backing up and restoring a running server with startAfter, and the schedule and retention settings, which keep out-of-range values out and prune to the retention count. A JVM killed by a signal, as the kernel's OOM killer does, is recorded as a clean stop with no crash and no auto-restart. That is reported separately rather than fixed here, and its check runs as a known issue: the runner's new `knownIssue` option makes it a warning while it fails and a failure once it passes, so the marker gets removed with the fix.
Nothing moved a file in or out of the panel in CI beyond one small .mrpack. The new transfers test covers: - uploads in one request (replacing a file, refusing a folder's name and a missing destination) and in chunks, sent last to first, with a repeated completion answered from the first, a wrong hash, a short chunk, an incomplete upload and a cancel each refused; - icons: a new server's default, an upload of any size resized to a 64x64 PNG that Minecraft can read, an undecodable image and a non-PNG refused, a chunked upload, delete and reset; - plugins on Paper and mods on Fabric: uploads (a non-jar refuses the request, a jar that isn't a zip is refused on its own), chunked uploads, listing, delete and delete-all, and client-only mods disabled on disk and re-enabled by uploading them again; - downloads of a plugin, all plugins, a file and the whole server, each with an exact Content-Length and reported over the WebSocket, with a missing file reported as failed; - a .cbx export with its backups and events, imported back both in one request and in chunks, keeping files, settings, group, backups and events under a new id with a port warning, and non-exports refused; - duplicating with and without the world, templates, and a server made from a .mrpack sent in chunks. lib.mjs gains a DGUP client, a PNG writer and reader, and raw request bodies for chunks.
Updating a container is the one path every user takes, and nothing tested that data written by the last release survives the new one. The upgrade test starts the published image on a fresh volume and fills it through that version's own API: a server with its settings, MOTD, properties, a file, a group and colour, a backup schedule, a backup and a template. Only what that version offers is seeded and checked, since groups arrived in 1.1.0. It then plants the records the startup migrations exist for (a server stuck provisioning, a failed provision, a NeoForge template labelled "1.26.1" as before 1.2.1) and kills the container with the server running. The image under test must come up on the same volume with the admin, the API key and the open session still valid, everything seeded intact, the running server reset to stopped, the stuck provision marked crashed, the failed one purged with its directory, the template relabelled to 26.1.2, and the server able to start. PRs upgrade from `latest`; the weekly run adds older releases. The test runs its own containers, and keeps both for their logs when it fails.
The WebSocket, transfers, operations and browser tests each insisted on a fresh panel to run the setup wizard on. They now sign in to one that's already set up (openPanel() in lib.mjs), so several can run in turn against one panel, as they will against Craftbox run outside Docker. The lifecycle section's restore-point Settings save needs nothing from the container, so it now runs everywhere instead of being skipped with the checks that do, and the skipped checks carry their real names. Waiting for a WebSocket message left its timeout running after the message arrived, so a script could sit idle for up to three minutes before exiting. The timeout is now cleared.
… standalone The per-PR checks stay on each type's newest version inside Docker, so nothing exercised the older Java runtimes the image carries, Modrinth itself, upgrades from before the last release, the arm64 image or Craftbox run outside Docker. ci-extended.yml does, weekly, by hand, and on a PR that changes it: - server-start.mjs now takes `type@version` (and vanilla@snapshot, and custom@<version> pointed at Mojang's own jar), and with CRAFTBOX_EXPECT_JAVA checks which of the image's JREs the server runs on. The matrix covers vanilla 1.12.2 and 1.16.5 on Java 8, 1.18.2 and 1.20.4 on 17, 1.21.1 on 21, the newest snapshot, a custom jar, Paper 1.20.4, Fabric 1.16.5, Forge 1.12.2/1.16.5/1.20.1 and NeoForge 1.21.1. Its `list` check also reads 1.12's "There are 0/20 players online". - modrinth.mjs covers search and lookups, a mod installed with its required dependency and loaded on start, a plugin, and a server made from a published modpack and started. - The upgrade test runs from 1.0.0, 1.1.0 and 1.2.0. - The arm64 image is built natively on an arm64 runner and runs the API smoke test and three server starts. - Standalone jobs on Ubuntu and Windows run npm ci, the unit tests, and the API, start, WebSocket, transfers and operations tests against node src/server.js with the runner's Java. Scripts run in turn there share one sign-in through CRAFTBOX_SESSION_FILE, since the login rate limit allows five in 15 minutes, and the lifecycle section stops its server so the next one's port is free. Forge 1.12.2 can't be provisioned today: Forge's Maven publishes no .sha256 for its installers, which Craftbox requires. That is reported separately, and its entry runs as a known issue until it's fixed.
Publishing a release pushed the image straight to Docker Hub, tested only by whatever CI had last run on master, and never on arm64 or with the weekly checks. The workflow now checks the release tag first, so a mislabelled release still fails in seconds, then runs both CI workflows against the release's own commit (ci.yml and ci-extended.yml, called as reusable workflows), and pushes only when every job has passed. The push builds from the cache those runs filled, amd64 from CI and arm64 from the extended run's native build, so wherever the cache holds them the layers published are the ones that were tested. A release now takes as long as the extended run before it publishes. If a check fails, an upstream outage included, re-running the failed jobs publishes once they pass.
The upgrade test cleared the way for itself with `docker rm -f` on the names craftbox and craftbox-old. craftbox is also the container_name in docker-compose.yml, so run on a machine with the compose setup, the test would have deleted that container. It now runs its containers as craftbox-upgrade-from and craftbox-upgrade-to, labels them and its volume craftbox-ci=upgrade, removes only containers carrying that label, and stops with an explanation if the name is taken by anything else. It also stops after a failed start instead of failing every step after it.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Widens CI from an API smoke test and a start of each server type to the rest of the panel: its pages and their scripts, sessions and CSRF, the WebSocket, lifecycle and crashes, backups, uploads and transfers, upgrades, older Minecraft versions, Modrinth, arm64 and installs outside Docker. The Docker publish now waits for all of it. No dependencies were added: the unit tests use
node:test, and Chrome is driven over the DevTools protocol with Node's own WebSocket.What runs where
Every PR and push to master (
ci.yml), about 8–10 minutes wall time:npm test: server.properties parsing, merging and updates; version ids and ordering; world types; Java per version; NeoForge build → Minecraft version (Maven stubbed); path containment; zip checkscommand(line), state/event/operation/content/dashboard broadcasts, the public socket's reduced viewWeekly, by hand, and on a PR that changes it (
ci-extended.yml), about 20–25 minutes:custom, Paper 1.20.4, Fabric 1.16.5, Forge 1.12.2/1.16.5/1.20.1, NeoForge 1.21.1, each checked for the Java runtime it actually runs onubuntu-24.04-arm, API smoke test and three server startsnpm ci, the unit tests, then the API, start, WebSocket, transfers and operations tests againstnode src/server.jswith the runner's JavaReleases (
docker-image.yml): the tag check runs first, then both workflows above against the release's commit, and the image is only pushed when every job passes. The push builds from the cache those runs filled. After a failure, re-running the failed jobs publishes.Upstream outages
Checks that depend on Mojang, PaperMC, Forge, NeoForge, Fabric, Purpur or Modrinth retry through a short outage. If a failure persists, the runner probes the service itself, and only calls it an outage when it can't reach it either. An outage still fails the job, but the log, the annotation and a separate section of the step summary say "Upstream unavailable" and name the service.
Known issues
Bugs these checks found are reported separately rather than fixed here. Their checks run as known issues: a warning while the bug is there, and a failure once it passes, so the marker comes off with the fix.
.sha256for its installers, only.sha1and.md5(Start forge@1.12.2).Running them locally
Each script's header says how, with
CRAFTBOX_URLpointing at a panel (andCRAFTBOX_CONTAINERnaming its container for the checks that reach inside it), for example: