Skip to content

Commit fa7c55a

Browse files
committed
test(dylib): screenshot regression for ringbuf capacity + canvas semantics
The existing test_dylib_confirm_flow covers the caller-driven polling contract — Initialize / Wipe / LoadDevice / GetAddress — but never asks the firmware for a layout. Two changes that just landed in the firmware emulator runtime PR (BitHighlander/keepkey-firmware#217) need functional coverage that confirm-flow doesn't provide: 1. RINGBUF_CAPACITY in lib/emulator/ringbuf.h was bumped from 32 to 128. DebugLinkState's 2048-byte `layout` plus the rest of the message serializes to ~44 HID reports through the output ring; the previous capacity left effective room for 31 reports, so screenshot capture truncated mid-layout (msg_debug_write ignores emulatorSocketWrite's 0-on-full return). 2. fsm_msgDebugLinkGetState in lib/firmware/fsm_msg_debug.h now does a single display_refresh() instead of force_animation_start() + animate(). The old form overwrote static layouts with stale animation frames or no-ops depending on queue state, so screenshots captured something different from what the user was seeing. Both fixes are functionally invisible to the existing test suite. Without these tests, regressing either change ships green. This commit adds: - tests/test_dylib_screenshot.py — four tests: * test_layout_round_trip_fits_through_ring (RINGBUF_CAPACITY) * test_layout_repeated_reads_no_truncation (RINGBUF_CAPACITY) * test_layout_stable_across_idle_reads (canvas semantics) * test_layout_features_dont_corrupt_capture (iface separation) Constructs a fresh KeepKeyDebuglinkClient against the dylib singleton WITHOUT going through common.KeepKeyTest.setUp — that fixture wipes the device on every test and exercises the confirm-flow path that test_dylib_confirm_flow is itself a pending regression for. Reading a layout doesn't require any of that; we just init and ask DebugLink for the home-screen capture. - tests/config.py — explicit-transport precedence fix: Previously HID/WebUSB were always autodetected first. With a real KeepKey plugged in, KK_TRANSPORT=dylib was silently overridden — the dylib regression suite would either route to hardware or crash on hid.pyx. Now the explicit env var (KK_TRANSPORT=dylib) skips hardware enumeration entirely, the dylib path runs as requested, and the default (no env var set) falls back to the existing UDP behavior. Verified locally: cmake -DKK_EMULATOR=1 -DKK_BUILD_DYLIB=1 -DKK_DEBUG_LINK=ON \ -DCMAKE_POLICY_VERSION_MINIMUM=3.5 -B build-emu . cmake --build build-emu --target kkemulator_dylib KK_TRANSPORT=dylib KK_DYLIB=build-emu/lib/libkkemu.dylib \ PYTHONPATH=keepkeylib:. python -m pytest tests/test_dylib_screenshot.py ======================== 4 passed in 0.36s ======================== Out of scope: SignTx + other multi-step flows that go through confirm_helper. They share the same hang as test_dylib_confirm_flow's test_load_device_with_auto_confirm — copying the pattern would just produce a second red regression for the same underlying firmware bug, not new coverage. Once the confirm-flow regression goes green, signtx expansion is a follow-up.
1 parent ebc0db7 commit fa7c55a

2 files changed

Lines changed: 175 additions & 11 deletions

File tree

tests/config.py

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,28 @@
2929
from keepkeylib.transport_socket import SocketTransportClient
3030
from keepkeylib.transport_udp import UDPTransport
3131

32-
try:
33-
from keepkeylib.transport_hid import HidTransport
34-
hid_devices = HidTransport.enumerate()
35-
except Exception:
36-
print("Error loading HID. HID devices not enumerated.")
37-
hid_devices = []
32+
# Skip HID/WebUSB autodetect when an explicit transport is requested.
33+
# Otherwise a connected real KeepKey wins over `KK_TRANSPORT=dylib` and the
34+
# dylib regression tests silently route to hardware instead.
35+
_explicit_transport = os.getenv("KK_TRANSPORT")
3836

39-
try:
40-
from keepkeylib.transport_webusb import WebUsbTransport
41-
webusb_devices = WebUsbTransport.enumerate()
42-
except Exception:
43-
print("Error loading WebUSB. WebUSB devices not enumerated.")
37+
if _explicit_transport:
38+
hid_devices = []
4439
webusb_devices = []
40+
else:
41+
try:
42+
from keepkeylib.transport_hid import HidTransport
43+
hid_devices = HidTransport.enumerate()
44+
except Exception:
45+
print("Error loading HID. HID devices not enumerated.")
46+
hid_devices = []
47+
48+
try:
49+
from keepkeylib.transport_webusb import WebUsbTransport
50+
webusb_devices = WebUsbTransport.enumerate()
51+
except Exception:
52+
print("Error loading WebUSB. WebUSB devices not enumerated.")
53+
webusb_devices = []
4554

4655
# Only count a hid device if it has more than just the U2F interface exposed
4756
onlyU2F = len(hid_devices) > 0 and \

tests/test_dylib_screenshot.py

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
"""Regression tests for libkkemu's screenshot / DebugLinkGetState path.
2+
3+
Two firmware-side changes need functional coverage that the existing dylib
4+
confirm-flow test doesn't provide:
5+
6+
1. ``RINGBUF_CAPACITY`` in ``lib/emulator/ringbuf.h``. A 2048-byte
7+
``DebugLinkState.layout`` field plus the rest of the message serializes
8+
to ~44 HID reports through the output ring; the previous capacity left
9+
effective room for 31 reports, so screenshot capture truncated silently
10+
(``msg_debug_write`` ignored ``emulatorSocketWrite``'s 0-on-full
11+
return). The host saw a short payload, not an error.
12+
13+
2. ``fsm_msgDebugLinkGetState`` in ``lib/firmware/fsm_msg_debug.h``: now
14+
does a single ``display_refresh()`` instead of
15+
``force_animation_start() + animate()``. The old form overwrote static
16+
layouts (``layout_warning``, address displays, etc.) with stale
17+
animation frames or no-ops depending on queue state, so screenshots
18+
captured something different from what the user was seeing on screen.
19+
20+
Both fixes are functionally invisible to the existing
21+
``test_dylib_confirm_flow`` suite — that test never asks for a layout. So
22+
without these tests, regressing either change ships green.
23+
24+
Skipped unless ``KK_TRANSPORT=dylib``. Set ``KK_DYLIB=/path/to/libkkemu.dylib``
25+
to run.
26+
"""
27+
28+
import os
29+
import unittest
30+
31+
32+
@unittest.skipUnless(
33+
os.environ.get("KK_TRANSPORT") == "dylib",
34+
"dylib screenshot regression — set KK_TRANSPORT=dylib KK_DYLIB=...",
35+
)
36+
class TestDylibScreenshot(unittest.TestCase):
37+
"""Constructs a fresh KeepKeyDebuglinkClient against the dylib singleton
38+
WITHOUT going through ``common.KeepKeyTest.setUp`` — the canonical
39+
fixture wipes the device on every test, and ``wipe_device`` exercises
40+
the confirm-flow path that ``test_dylib_confirm_flow`` is itself a
41+
pending regression for. Reading a layout doesn't require any of that;
42+
we just init and ask DebugLink for the home-screen capture.
43+
"""
44+
45+
def setUp(self):
46+
# Late imports — `config` and `common` construct transports on
47+
# import and would fail / hang under non-dylib runs even though
48+
# this class is skip-decorated.
49+
import config # noqa: WPS433
50+
from keepkeylib.client import KeepKeyDebuglinkClient # noqa: WPS433
51+
52+
transport = config.TRANSPORT(*config.TRANSPORT_ARGS, **config.TRANSPORT_KWARGS)
53+
debug_transport = config.DEBUG_TRANSPORT(
54+
*config.DEBUG_TRANSPORT_ARGS, **config.DEBUG_TRANSPORT_KWARGS
55+
)
56+
self.client = KeepKeyDebuglinkClient(transport)
57+
self.client.set_debuglink(debug_transport)
58+
# No wipe_device — dylib boot already drew the home screen and
59+
# that's what we want to capture. Going through wipe would also
60+
# exercise confirm_helper, which is intentionally out of scope here.
61+
62+
def tearDown(self):
63+
try:
64+
self.client.close()
65+
except Exception:
66+
pass
67+
68+
# ── Ring capacity coverage ──────────────────────────────────────────
69+
70+
def test_layout_round_trip_fits_through_ring(self):
71+
"""The smoking-gun test for ``RINGBUF_CAPACITY``.
72+
73+
``messages.options`` declares ``DebugLinkState.layout max_size:2048``.
74+
If the output ring is too small, the response is truncated mid-
75+
layout-field and either fails to decode or returns a short value.
76+
Either way the canonical contract — 2048 bytes — is broken.
77+
"""
78+
layout = self.client.debug.read_layout()
79+
80+
# nanopb encodes the layout field as bytes; python-keepkey returns
81+
# whatever bytes the firmware put in. The contract is exactly 2048.
82+
self.assertEqual(
83+
len(layout), 2048,
84+
"DebugLinkState.layout returned %d bytes; firmware contract is 2048. "
85+
"Truncation here points at an undersized libkkemu output ring." % len(layout),
86+
)
87+
# Sanity: the home screen has *something* drawn on it; a fully-zero
88+
# layout would mean we read a frame before the firmware drew home.
89+
self.assertGreater(
90+
sum(layout), 0,
91+
"Layout came back all zeros — host raced firmware boot? "
92+
"DylibState.__init__ pumps 8 polls before returning; if that "
93+
"stops being enough to settle the home screen, this test will "
94+
"catch it.",
95+
)
96+
97+
def test_layout_repeated_reads_no_truncation(self):
98+
"""Ten back-to-back ``read_layout`` calls must each return 2048 bytes.
99+
100+
A subtle ring-capacity bug could pass a single read (writer fills,
101+
reader drains, writer re-fills cleanly) but fail under repeated
102+
reads if writer/reader fall out of phase. Catches half-step
103+
truncation that the single-shot test above misses.
104+
"""
105+
for i in range(10):
106+
layout = self.client.debug.read_layout()
107+
self.assertEqual(
108+
len(layout), 2048,
109+
"Read #%d returned %d bytes" % (i, len(layout)),
110+
)
111+
112+
# ── Canvas semantics coverage ───────────────────────────────────────
113+
114+
def test_layout_stable_across_idle_reads(self):
115+
"""When the firmware is idle (sitting on the home screen) the
116+
captured layout must be byte-identical between reads.
117+
118+
With the OLD ``fsm_msgDebugLinkGetState`` code, the
119+
``force_animation_start() + animate()`` calls before the canvas
120+
capture would either:
121+
(a) re-run a queued animation → the bytes would change between
122+
reads as the animation advanced, OR
123+
(b) overwrite a static canvas with a no-op redraw → bytes match
124+
this read but the next layout-changing call sees stale state.
125+
126+
With the new ``display_refresh()`` form, the canvas is whatever
127+
the firmware last drew — stable across reads of an idle UI.
128+
"""
129+
first = self.client.debug.read_layout()
130+
for i in range(5):
131+
again = self.client.debug.read_layout()
132+
self.assertEqual(
133+
first, again,
134+
"Idle layout byte-changed between reads (iter %d). "
135+
"fsm_msgDebugLinkGetState may be running animations again." % i,
136+
)
137+
138+
def test_layout_features_dont_corrupt_capture(self):
139+
"""An interleaved Initialize call (which the canonical
140+
``KeepKeyTest`` setUp ALSO does as part of ``KeepKeyClient``
141+
construction) must not desynchronize the next ``read_layout``.
142+
143+
Catches a class of dylib-output-ring bugs where a non-debug
144+
response leaves bytes in the main ring that bleed into the next
145+
DebugLink read. Both rings are independent, but a serializer bug
146+
that writes to the wrong iface would surface as a misframed
147+
screenshot.
148+
"""
149+
self.client.init_device() # round-trips Features on iface 0
150+
layout = self.client.debug.read_layout()
151+
self.assertEqual(len(layout), 2048)
152+
153+
154+
if __name__ == "__main__":
155+
unittest.main()

0 commit comments

Comments
 (0)