Skip to content

Commit 44d82ef

Browse files
committed
ci(bitcoin-only): actually run the product-boundary suite
tests/test_msg_bitcoin_only_variant.py calls requires_bitcoinOnly() in setUp(), and ci.yml built and started only the regular emulator. All eleven product-boundary tests therefore ran as skips in the one required integration job, so the advertised bitcoin-only coverage was never executed by any check. The module docstring also claimed "NOTHING HERE SKIPS", which the unconditional gate had already made false. Add an integration-btc job that builds the emulator with -DKK_BITCOIN_ONLY=ON via the Dockerfile's existing coinsupport build arg, asserts features.firmware_variant is EmulatorBTC before pytest runs, and fails when any test in the module skips -- pytest exits 0 on a fully skipped module, so a green run proves nothing unless the skip count is zero. Rewrite the docstring to describe the real scope and name the job the file now depends on.
1 parent 70f3055 commit 44d82ef

2 files changed

Lines changed: 230 additions & 8 deletions

File tree

.github/workflows/ci.yml

Lines changed: 215 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
# └─ lint Python syntax + deterministic protocol contract tests
88
#
99
# Stage 2: TEST (gated by Stage 1)
10-
# └─ integration full pytest suite against emulator
10+
# ├─ integration full pytest suite against the regular emulator
11+
# └─ integration-btc bitcoin-only product boundary against a
12+
# -DKK_BITCOIN_ONLY=ON emulator
1113

1214
name: CI
1315

@@ -282,3 +284,215 @@ jobs:
282284
run: |
283285
STATUS=$(cat keepkey-firmware/deps/python-keepkey/tests/status 2>/dev/null || echo "1")
284286
[ "$STATUS" = "0" ] || exit 1
287+
288+
# ═══════════════════════════════════════════════════════════
289+
# STAGE 2b: TEST — the OTHER shipping product
290+
# ═══════════════════════════════════════════════════════════
291+
292+
integration-btc:
293+
needs: [lint]
294+
runs-on: ubuntu-latest
295+
timeout-minutes: 15
296+
297+
# KK_BITCOIN_ONLY=ON is a second shipping product, not a build flavour:
298+
# coins.def keeps only Bitcoin and Testnet, messagemap.def drops every
299+
# altcoin handler, ZCASH_PRIVACY is forced OFF, and transaction.c takes a
300+
# BITCOIN_ONLY arm on the OP_RETURN path.
301+
#
302+
# tests/test_msg_bitcoin_only_variant.py asserts all of that, and its
303+
# setUp() calls requires_bitcoinOnly() -- so against the regular emulator
304+
# the `integration` job runs it as ELEVEN SKIPS. Skips are green. Without
305+
# this job the advertised bitcoin-only coverage is never executed by any
306+
# required check, which is the exact condition that file was written to
307+
# end. The step below therefore fails closed on a skip, not just on a
308+
# failure.
309+
310+
steps:
311+
- uses: actions/checkout@v4
312+
with:
313+
submodules: recursive
314+
path: python-keepkey
315+
316+
- name: Checkout firmware
317+
uses: actions/checkout@v4
318+
with:
319+
repository: BitHighlander/keepkey-firmware
320+
ref: alpha
321+
path: keepkey-firmware
322+
323+
# Same non-recursive init as the regular job: trezor-firmware's
324+
# micropython vendor tree pulls lib/lwip from git.savannah.gnu.org,
325+
# which cannot serve the shallow clone actions/checkout asks for.
326+
- name: Init the submodules the emulator build needs
327+
working-directory: keepkey-firmware
328+
run: |
329+
git submodule update --init --depth 1 deps/crypto/trezor-firmware
330+
git submodule update --init --depth 1 deps/device-protocol
331+
git submodule update --init --depth 1 deps/googletest
332+
git submodule update --init --depth 1 deps/qrenc/QR-Code-generator
333+
git submodule update --init --depth 1 deps/sca-hardening/SecAESSTM32
334+
335+
- name: Overlay this python-keepkey onto the firmware tree
336+
run: |
337+
rm -rf keepkey-firmware/deps/python-keepkey
338+
cp -a python-keepkey keepkey-firmware/deps/python-keepkey
339+
340+
# scripts/emulator/Dockerfile forwards ARG coinsupport into the cmake
341+
# invocation, so this is the same emulator build with the product flag
342+
# the shipping bitcoin-only image is built with.
343+
- name: Build the bitcoin-only emulator
344+
timeout-minutes: 20
345+
working-directory: keepkey-firmware
346+
run: |
347+
docker build -t kkemu-btc-ci \
348+
--build-arg coinsupport=-DKK_BITCOIN_ONLY=ON \
349+
-f scripts/emulator/Dockerfile .
350+
351+
- name: Start the emulator
352+
run: |
353+
docker run -d --name kkemu-btc \
354+
-p 11044:11044/udp -p 11045:11045/udp -p 5000:5000 kkemu-btc-ci
355+
sleep 3
356+
docker logs kkemu-btc | head -5
357+
358+
- uses: actions/setup-python@v5
359+
with:
360+
python-version: '3.11'
361+
362+
- name: Install dependencies
363+
working-directory: python-keepkey
364+
run: |
365+
pip install --upgrade pip
366+
pip install "protobuf>=3.20,<4"
367+
pip install -e .
368+
pip install pytest semver rlp requests eth-keys pycryptodome
369+
370+
- name: Wait for emulator
371+
run: |
372+
echo "Waiting for emulator bridge on port 5000..."
373+
for i in $(seq 1 30); do
374+
if curl -sf -X POST http://localhost:5000/exchange/main \
375+
-H 'Content-Type: application/json' \
376+
-d '{"data":""}' > /dev/null 2>&1; then
377+
echo "Emulator ready after ${i}s"
378+
break
379+
fi
380+
sleep 1
381+
done
382+
383+
# A bitcoin-only emulator that reports "Emulator" instead of
384+
# "EmulatorBTC" makes requires_bitcoinOnly() skip the whole file, and a
385+
# regular emulator built by a broken --build-arg does the same. Assert
386+
# the variant BEFORE pytest so that failure is named, not silent.
387+
- name: Assert the emulator really is the bitcoin-only product
388+
timeout-minutes: 2
389+
env:
390+
KK_TRANSPORT_MAIN: "127.0.0.1:11044"
391+
KK_TRANSPORT_DEBUG: "127.0.0.1:11045"
392+
KK_MIN_FW: "7.15.0"
393+
KK_UDP_TIMEOUT: "20"
394+
working-directory: keepkey-firmware/deps/python-keepkey/tests
395+
run: |
396+
python - <<'PY'
397+
import os, sys
398+
sys.path.insert(0, '..')
399+
import config
400+
from keepkeylib.client import KeepKeyDebuglinkClient
401+
c = KeepKeyDebuglinkClient(config.TRANSPORT(*config.TRANSPORT_ARGS,
402+
**config.TRANSPORT_KWARGS))
403+
c.set_debuglink(config.DEBUG_TRANSPORT(*config.DEBUG_TRANSPORT_ARGS,
404+
**config.DEBUG_TRANSPORT_KWARGS))
405+
c.init_device()
406+
f = c.features
407+
got = (f.major_version, f.minor_version, f.patch_version)
408+
floor = tuple(int(x) for x in os.environ['KK_MIN_FW'].split('.'))
409+
print('emulator firmware %d.%d.%d, variant %r' %
410+
(got + (f.firmware_variant,)))
411+
if got < floor:
412+
sys.exit('FATAL: the emulator image predates the tests that run '
413+
'against it.')
414+
if f.firmware_variant not in ('KeepKeyBTC', 'EmulatorBTC'):
415+
sys.exit('FATAL: firmware_variant is %r, so requires_bitcoinOnly() '
416+
'would skip every test in this job. The -DKK_BITCOIN_ONLY=ON '
417+
'build arg did not take effect.' % (f.firmware_variant,))
418+
PY
419+
420+
- name: Run the bitcoin-only product-boundary tests
421+
timeout-minutes: 8
422+
env:
423+
KK_TRANSPORT_MAIN: "127.0.0.1:11044"
424+
KK_TRANSPORT_DEBUG: "127.0.0.1:11045"
425+
PYTHONPATH: "${{ github.workspace }}/keepkey-firmware/deps/python-keepkey"
426+
KK_UDP_TIMEOUT: "45"
427+
run: |
428+
cd keepkey-firmware/deps/python-keepkey/tests
429+
pytest -v --junitxml=junit-btc.xml test_msg_bitcoin_only_variant.py \
430+
2>&1 | tee pytest-btc-output.txt
431+
echo "${PIPESTATUS[0]}" > status-btc
432+
433+
# The whole reason this job exists. `pytest` exits 0 on a fully skipped
434+
# module, so a green run proves nothing unless the skip count is zero.
435+
- name: Fail if the product-boundary tests skipped
436+
if: always()
437+
run: |
438+
XML="keepkey-firmware/deps/python-keepkey/tests/junit-btc.xml"
439+
if [ ! -f "$XML" ]; then
440+
echo "::error::no junit-btc.xml -- the suite crashed before completion"
441+
exit 1
442+
fi
443+
python3 - "$XML" <<'PY'
444+
import sys, xml.etree.ElementTree as ET
445+
tree = ET.parse(sys.argv[1])
446+
cases = list(tree.iter('testcase'))
447+
skipped = [c for c in cases if c.find('skipped') is not None]
448+
print('bitcoin-only boundary: %d tests, %d skipped' %
449+
(len(cases), len(skipped)))
450+
if not cases:
451+
sys.exit('FATAL: collected zero tests.')
452+
for c in skipped:
453+
print('::error::SKIPPED %s: %s' %
454+
(c.get('name'), c.find('skipped').get('message', '')))
455+
if skipped:
456+
sys.exit('FATAL: %d of %d bitcoin-only tests skipped. A skip here '
457+
'means the variant went unaudited, which is the failure '
458+
'this job exists to catch.' % (len(skipped), len(cases)))
459+
PY
460+
461+
- name: Bitcoin-only summary
462+
if: always()
463+
run: |
464+
XML="keepkey-firmware/deps/python-keepkey/tests/junit-btc.xml"
465+
echo "## 🔑 KeepKey python-keepkey — Bitcoin-only product boundary" >> "$GITHUB_STEP_SUMMARY"
466+
echo "" >> "$GITHUB_STEP_SUMMARY"
467+
if [ ! -f "$XML" ]; then
468+
echo "❌ **No test results found** — suite may have crashed." >> "$GITHUB_STEP_SUMMARY"
469+
else
470+
TOTAL=$(grep -oP 'tests="\K[0-9]+' "$XML" | head -1)
471+
FAILED=$(grep -oP 'failures="\K[0-9]+' "$XML" | head -1)
472+
ERRORS=$(grep -oP 'errors="\K[0-9]+' "$XML" | head -1)
473+
SKIPPED=$(grep -oP 'skipped="\K[0-9]+' "$XML" | head -1)
474+
TOTAL=${TOTAL:-0}; FAILED=${FAILED:-0}; ERRORS=${ERRORS:-0}; SKIPPED=${SKIPPED:-0}
475+
PASSED=$((TOTAL - FAILED - ERRORS - SKIPPED))
476+
echo "| Metric | Count |" >> "$GITHUB_STEP_SUMMARY"
477+
echo "|--------|-------|" >> "$GITHUB_STEP_SUMMARY"
478+
echo "| Total | $TOTAL |" >> "$GITHUB_STEP_SUMMARY"
479+
echo "| ✅ Passed | $PASSED |" >> "$GITHUB_STEP_SUMMARY"
480+
echo "| ⏭️ Skipped (must be 0) | $SKIPPED |" >> "$GITHUB_STEP_SUMMARY"
481+
echo "| ❌ Failed | $FAILED |" >> "$GITHUB_STEP_SUMMARY"
482+
echo "| 💥 Errors | $ERRORS |" >> "$GITHUB_STEP_SUMMARY"
483+
fi
484+
485+
- name: Annotate test results
486+
uses: mikepenz/action-junit-report@v4
487+
if: always()
488+
with:
489+
report_paths: keepkey-firmware/deps/python-keepkey/tests/junit-btc.xml
490+
annotate_only: true
491+
require_tests: true
492+
fail_on_failure: true
493+
494+
- name: Fail on test failure
495+
if: always()
496+
run: |
497+
STATUS=$(cat keepkey-firmware/deps/python-keepkey/tests/status-btc 2>/dev/null || echo "1")
498+
[ "$STATUS" = "0" ] || exit 1

tests/test_msg_bitcoin_only_variant.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,21 @@
77
None of that had a test, and CI only ever ran the multi-chain emulator -- so
88
the whole variant was unaudited.
99
10-
NOTHING HERE SKIPS. Each test asserts the behaviour that is correct for the
11-
variant it is talking to, so it is evidence on both builds: on the bitcoin-only
12-
image it proves the strip happened, and on the regular image it proves the
13-
strip did NOT happen (a guard that leaked into the multi-chain product would
14-
fail here just as loudly). `requires_fullFeature()` is deliberately not used --
15-
see test_firmware_variant_names_the_bitcoin_only_product for why it cannot
16-
work.
10+
SCOPE: THIS FILE RUNS ON THE BITCOIN-ONLY IMAGE ONLY. `setUp()` calls
11+
`requires_bitcoinOnly()`, so every test here SKIPS on the regular multi-chain
12+
build. That is deliberate and not symmetric coverage: several tests assert
13+
screen sequences that legitimately differ on the multi-chain build -- the
14+
OP_RETURN one decodes a THORChain memo there and draws more screens -- so
15+
running them against a full-feature device is a category error, not a finding.
16+
The regular image is covered by the rest of the suite, which asserts the
17+
altcoin handlers these tests assert are absent.
18+
19+
Because of that gate, this file is only evidence when a bitcoin-only emulator
20+
is actually under test. `.github/workflows/ci.yml` runs the `integration-btc`
21+
job for exactly that reason: it builds the emulator with
22+
`-DKK_BITCOIN_ONLY=ON` and runs this module against it. If that job is ever
23+
dropped, these eleven tests go silently green-by-skip and the variant is
24+
unaudited again -- which is the state this file was written to end.
1725
1826
The variant is identified by GetCoinTable, not by features.firmware_variant:
1927
the coin table comes from coins.def, which is a different mechanism from the

0 commit comments

Comments
 (0)