mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-18 03:54:17 +02:00
fix(tests): make formatTimeOnly assertions locale-agnostic (#1213)
formatTimeOnly calls date.toLocaleTimeString([], …) which respects the user's locale by design — the en_DK.UTF-8 locale uses "." as the time separator, so the function correctly returns "02.30 pm" / "14.30" for a Danish-English user. The two assertions hard-coded ":" as the separator, which made the tests fail under any locale that doesn't use ":". The implementation was right, the tests were wrong. Switch the regex to use \D+ (any non-digit, one or more) for the separator. This tests the actual contract — "hours and minutes, separated somehow" — without coupling to a separator that varies by locale (en_DK uses ".", some en_* locales use a narrow no-break space at U+202F, most others use ":"). Verified passing under en_DK.UTF-8, en_US.UTF-8, and de_DE.UTF-8. Audited every other toLocaleTimeString / toLocaleString call site in the test suite — no other places hard-code separator characters.
This commit is contained in:
@@ -20,6 +20,8 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
- **Filament Track Switch (FTS) support — print modal filament dropdown is no longer empty when an X2D / H2D has the FTS accessory installed** ([#1162](https://github.com/maziggy/bambuddy/issues/1162), reported by @mkavalecz) — When the FTS accessory is installed the printer's MQTT changes one nibble of the per-AMS `info` bitmask: bits 8-11 flip from a fixed extruder ID (0x0 / 0x1) to `0xE` ("uninitialized"), because the AMS is no longer wired to a single nozzle — the FTS dynamically routes any slot to either extruder. Bambuddy's MQTT parser already skipped 0xE entries when building `ams_extruder_map` (matching BambuStudio's reading for boot-time transient state), so with the FTS installed the map ended up empty and the print modal's filament dropdown — which filters by `extruderId === nozzle_id` to prevent cross-nozzle assignment ("position of left hotend is abnormal" failures) — filtered out *every* loaded slot. Net effect: empty Filament Mapping dropdown on every dual-nozzle print with the FTS, even when the AMS was fully loaded with the right material. Detection comes from a new MQTT field — `print.device.fila_switch` — which is non-null only when the accessory is installed; it carries the routing topology as two arrays: `in[track] = currently fed slot (-1 = empty)` and `out[track] = extruder this track terminates at`. The fix surfaces this through a new `FilaSwitchState` dataclass on `PrinterState` (`installed`, `in_slots`, `out_extruders`, `stat`, `info`) and the equivalent `FilaSwitchResponse` Pydantic schema on the `GET /printers/{id}/status` route. Frontend (`useFilamentMapping.ts` + `FilamentMapping.tsx`) skips the per-extruder filter when `printerStatus.fila_switch?.installed === true` so any compatible AMS slot can satisfy any nozzle's filament requirement, since the FTS handles the routing. Slots currently fed into a track also get a routing badge in the dropdown — `[L]` or `[R]` — so the user can tell at a glance which slot the FTS is currently routing where (idle slots get no badge: they can be routed to either extruder on demand). The hard "no cross-nozzle assignment" filter on real dual-nozzle printers without the FTS stays untouched (still trips the same way it always has — `fila_switch == null` keeps the existing behaviour). 4 backend tests in `test_bambu_mqtt.py::TestFilamentTrackSwitchDetection` (default-not-installed, detect-from-MQTT-using-the-reporter's-bundle, no-fila_switch-field-stays-not-installed, missing-in-out-arrays-don't-crash) and 2 frontend tests in `useFilamentMapping.test.ts` (FTS-active drops the nozzle filter; explicit `fila_switch: null` keeps the filter applied). Upstream fila_switch payloads with anything other than the documented shape are tolerated — `installed` flips on the *presence* of the field, the routing arrays default to empty lists if missing, and the dropdown skips the badge for slots not currently in `in_slots`.
|
||||
|
||||
### Fixed
|
||||
- **`formatTimeOnly` tests failed under non-`:`-separator locales** ([#1213](https://github.com/maziggy/bambuddy/issues/1213), reported by @maugsburger) — Running the frontend test suite under `LC_ALL=en_DK.UTF-8` (or any locale whose `toLocaleTimeString` uses a separator other than `:`) failed two tests in `frontend/src/__tests__/utils/date.test.ts`: `formats time with 12h format` (expected `02.30 pm` to match `/2:30|02:30/`) and `formats time with 24h format` (expected `14.30` to contain `14:30`). The implementation is correct — `formatTimeOnly` calls `date.toLocaleTimeString([], …)` which by design respects the user's locale, so a Danish-English user genuinely should see `02.30 pm` in the UI. The tests just hard-coded the `:` separator. **Fix:** test assertions now use `\D+` (any non-digit, one or more) for the separator: `expect(result).toMatch(/\b0?2\D+30\b/)` and `expect(result).toMatch(/\b14\D+30\b/)`. Tests the actual contract — "the function returns hours and minutes, separated somehow" — without coupling to a specific separator that varies by locale (en_DK uses `.`, some en_* locales use a narrow no-break space at U+202F, most others use `:`). Verified passing under `en_DK.UTF-8`, `en_US.UTF-8`, and `de_DE.UTF-8`. Audited every other `toLocaleTimeString`/`toLocaleString` call site in the test suite — no other places hard-code separator characters; `formatETA`, `formatDateInput` etc. assert via `toBeTruthy()` or check translated content.
|
||||
|
||||
- **SpoolBuddy kiosk screen-blank timeout setting was ignored after the first save** (reported by maziggy) — Picking a new "Screen Blank Timeout" in SpoolBuddy Settings → Display didn't change the actual blanking behaviour: whatever value was active when the kiosk last booted continued to fire — a user who started with the 10 m preset and then switched to 1 m, 5 m, or "Off" still saw the screen blank at 10 m forever. Cause: blanking is driven by `swayidle`, started once by `spoolbuddy/install/spoolbuddy-idle.sh` at labwc autostart with the timeout passed as a command-line argument (`swayidle -w timeout $T 'wlopm --off' resume 'wlopm --on'`). The script fetched `blank_timeout` from the backend exactly once at startup and `swayidle` has no runtime control surface for changing its timeout. The Python daemon's `display.set_blank_timeout()` updated an in-memory variable on the daemon side that was only used for daemon-side idle bookkeeping (`tick()` log-line) and never reached `swayidle`, so UI changes were silently discarded until the next kiosk restart. Documented as such in the daemon's docstring (`display_control.py:5`: *"swayidle is the sole authority on screen blanking"*) — the architecture predicted the bug, the UX never matched. **Fix:** the wake FIFO at `/tmp/spoolbuddy-wake` now carries a second message in addition to `wake`: `reload-timeout N`. The daemon writes it whenever `set_blank_timeout()` is called with a value that differs from the current one (the very first call is suppressed because the watchdog already fetched the same value at its own startup — signalling there would just thrash `swayidle` on every cold start). The watchdog script's FIFO loop is restructured around `start_swayidle` / `stop_swayidle` helpers and a single `case` statement that dispatches on the message: `wake` → `wlopm --on` + arm a re-blank at the *current* timeout; `reload-timeout N` → kill the running `swayidle`, set `TIMEOUT=$N`, restart `swayidle`, and `wlopm --on` so the user sees the change took effect even if the screen was already blanked. The script de-dupes too — a `reload-timeout N` whose `N` matches the current value is a no-op, so the daemon's local de-dupe and the script's de-dupe both guard against thrash. Going from any positive timeout to `0` ("Off") correctly stops `swayidle` and never restarts it, going from `0` to a positive value starts a fresh `swayidle` — both work without a kiosk restart. The script's main loop opens the FIFO read+write (`exec 3<>"$WAKE_FIFO"`) so the bash `read` never sees EOF when the daemon momentarily disconnects between writes (without that, the loop would exit the first time the daemon closed its write end). A `cleanup` trap on `TERM/INT/HUP` stops `swayidle`, removes the FIFO, and exits cleanly. 7 new tests in `spoolbuddy/tests/test_display_control.py::TestDisplayControlFifoMessages` pin the daemon side of the protocol against a real FIFO in `tmp_path`: `wake()` writes the literal `wake\n` line; first `set_blank_timeout` is suppressed (script already has the right value); subsequent change emits `reload-timeout N\n`; identical-value calls don't signal; transitioning to `0` emits `reload-timeout 0\n` (covers "user picks Off after enabling"); negative inputs are clamped to `0` in the signal payload; missing-FIFO writes are silent no-ops (kiosk-not-running case). Also handles the SpoolBuddy `0` schema default — the first `set_blank_timeout(0)` call from a fresh daemon doesn't signal (init suppressed) so no spurious thrash on a never-configured device.
|
||||
|
||||
- **Archive 3MFs (and library file bytes) silently deleted from disk on every print completion** ([#1212](https://github.com/bambuman/bambuddy/issues/1212), reported by @abbasegbeyemi; matches private "file disappeared overnight" reports) — Reprint and View G-code on a freshly-completed archive returned 404 with no log line explaining why; the DB row was intact, the archive grid kept showing the entry, but `archive.file_path` pointed at a path that no longer existed on disk. Same shape independently reported by a daily-build user whose `.gcode.3mf` "disappeared by itself overnight" between Saturday's print and Monday morning's reprint attempt. Root cause was a regression introduced by [#1166](https://github.com/maziggy/bambuddy/issues/1166)'s cover-cache pre-population: the dispatch sites in `background_dispatch.py:692`, `background_dispatch.py:896`, and `print_scheduler.py:1897` started caching the **live archive copy** (and library file bytes for the Direct-Print flow) in the shared 3MF download cache so the `/cover` endpoint could skip a redundant FTP transfer to the printer mid-print. The cache itself was originally designed for transient downloads under `archive_dir/temp/` and `clear_3mf_cache(printer_id, delete_files=True)` — called from `on_print_complete` to keep that temp dir from accumulating — happily `unlink()`'d every cached path. Pre-#1166 every cached path was a temp file, so deletion was correct. Post-#1166 the cleanup was destroying user data: every print → archive 3mf cached → on print complete `clear_3mf_cache` walks the cache → `path.unlink()` on the actual archive copy. The `Path.exists()` guard inside `_maybe_unlink` masked the failure: the file existed at unlink time, so no exception, no warning, just silent destruction. The DB row remained, so the UI listing didn't change — only when the user tried to *act* on the archive (reprint / view-gcode / re-export) did the missing file surface as a 404. Affected every daily build since [`889c8bd8`](https://github.com/bambuman/bambuddy/commit/889c8bd8) (Apr 29). **Fix:** `clear_3mf_cache._maybe_unlink` in `backend/app/services/bambu_ftp.py` now refuses to `unlink()` any path outside `archive_dir/temp` — the cache dict is still cleared either way (so re-cache logic continues to work and the cover endpoint still hits a fresh path on the next print), only the on-disk delete is gated. Persistent locations — `archive/<printer_id>/...`, `archive/unassigned/...` (VP-archived prints with `printer_id=None`), `library_files/...`, and any `is_external` library mount — survive intact. The dispatch sites that cache those paths are unchanged: it's correct for `/cover` to read straight from the live archive copy and avoid the redundant 36 MB FTP transfer; the only bug was the cleanup branch treating all cached paths as transient. Regression test `test_clear_does_not_delete_persistent_files` in `test_bambu_ftp.py` pins the contract end-to-end: an archive 3mf at `archive/1/.../...gcode.3mf`, a library 3mf at `library_files/...`, and a temp 3mf at `archive/temp/...` are all cached for the same printer; after `clear_3mf_cache(1)` runs, all three cache entries are dropped from the dict (so the cache state is consistent), but only the temp file is unlinked from disk — the archive and library files still exist. Two existing cache tests (`test_clear_by_printer_scoped`, `test_clear_without_deleting_files`) updated to put their fixtures under `archive_dir/temp` since that's now the only path the cleanup will touch. **Damage:** users on daily builds since Apr 29 with a `print → wait for completion → reprint or view-3mf later` workflow have been silently losing archive copies. Recovery for individual users: re-import the source 3mf from your slicer / NAS, or re-archive from the printer's FTP if the file is still there. Going forward the bytes are safe.
|
||||
|
||||
@@ -296,14 +296,15 @@ describe('formatTimeOnly', () => {
|
||||
it('formats time with 12h format', () => {
|
||||
const date = new Date(2025, 5, 15, 14, 30);
|
||||
const result = formatTimeOnly(date, '12h');
|
||||
expect(result).toMatch(/2:30|02:30/);
|
||||
// Locale-agnostic: separator is "." in en_DK, " " (NBSP) in some, ":" elsewhere.
|
||||
expect(result).toMatch(/\b0?2\D+30\b/);
|
||||
expect(result.toUpperCase()).toContain('PM');
|
||||
});
|
||||
|
||||
it('formats time with 24h format', () => {
|
||||
const date = new Date(2025, 5, 15, 14, 30);
|
||||
const result = formatTimeOnly(date, '24h');
|
||||
expect(result).toContain('14:30');
|
||||
expect(result).toMatch(/\b14\D+30\b/);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user