Skip to content

ci(integration): bound every test, so a deadlock fails by name instead of by timeout - #477

Merged
BitHighlander merged 1 commit into
release/7.14.2from
fix/integration-per-test-timeout
Aug 17, 2026
Merged

ci(integration): bound every test, so a deadlock fails by name instead of by timeout#477
BitHighlander merged 1 commit into
release/7.14.2from
fix/integration-per-test-timeout

Conversation

@BitHighlander

Copy link
Copy Markdown
Owner

Step 2 of #466 — harness robustness. Does not attempt the expectation updates, which belong with the deps/python-keepkey repin.

Before / after, same emulator, same tests, same firmware

before 30-minute job kill, no JUnit XML, Phase 2 never runs, every file after the stall unmeasured
after 22 failed, 362 passed, 47 skipped in 133.59s, 66KB of JUnit XML

What it does

Bounds each test rather than the job. --timeout=60 --timeout-method=signal.

signal rather than thread is the load-bearing choice: thread kills the process, so one deadlock still costs the rest of the run, whereas signal raises inside the blocked test — which then fails by name — and the suite continues. Measured on the known-deadlocking THORChain file:

1 failed, 5 passed in 20.29s

The stack dump points straight at the blocked read:

File "../keepkeylib/transport_udp.py", line 60, in _raw_read
    data = self.socket.recv(64)
+++++++++++++++++++++++++++++++++++ Timeout ++++++++++++++++++++++++++++++++++++

60s is roughly 30x the slowest healthy file in this suite (multisig, ~2s), so a genuinely slow test is not at risk.

The interesting part: the deadlock was hiding 22 failures, not 2

Every one is an intentional change in this release meeting a stale expectation. None indicates a firmware defect.

count cause
14 AdvancedMode policy gate — TON, TRON and ETH contract data now require it (the documented BREAKING change). These tests never enable it.
5 chain_id refusal#445 working as designed. Fixed by keepkey/python-keepkey#215.
2 the deadlocks — THORChain and EOS extra disclosure screens.
1 a CallException still to be triaged individually.

By file: TestMsgTonSignTx 9, TestMsgEthereumSigntx 5, TestMsgTronSignTx 3, and one each in TestMsgEosSignTx, TestMsgEthereum0xtxERC20, TestMsgThorChainSignTx, TestMsgEthereumSignTypedDataHash, TestMsgE712Verify.

That inventory did not exist before this change — the job died before producing it.

Scope

This does not fix the expectation mismatch. It makes it legible in two minutes with a test name attached instead of an opaque half-hour timeout, and it is worth keeping afterwards regardless: an integration test should never be able to consume the whole CI budget because one side is waiting for a button acknowledgement the other forgot to send.

It will not turn the branch green. 22 tests still fail, correctly. Combined with the aggregate gate from #474, release/7.14.2 will now report red for a legible reason, which is the point.

Verified

pytest-timeout 2.1.0 installs cleanly in the pinned Alpine 3.8 / Python 3.6.9 image. Both phases of python-keepkey-tests.sh carry the flags; sh -n clean.

…d of by timeout

python-integration-tests has never completed on this branch. Two tests deadlock
-- this release added confirmation screens the pinned suite does not
acknowledge, so the firmware waits for a ButtonAck that never arrives while the
test waits for a response that never comes -- and with no per-test bound that is
a 30-minute JOB kill producing no JUnit XML. Phase 2, the documented gate, never
runs, and every file after the stall is unmeasured. Absence of a result is
indistinguishable from a pass.

Bound each test instead of the job. method=signal, not thread: thread kills the
process so one deadlock still costs the rest of the run, while signal raises
inside the blocked test, which fails BY NAME, and the suite continues.

The effect is not subtle. Same emulator, same tests, same firmware:

  before   30-minute job kill, no JUnit XML, 2 files known to stall,
           everything after them unmeasured
  after    22 failed, 362 passed, 47 skipped in 133.59s, 66KB of JUnit XML

The deadlock was hiding 22 failures, not 2. Every one is a consequence of an
intentional change in this release meeting a stale expectation, and none
indicates a firmware defect:

  14  AdvancedMode policy gate -- TON, TRON and ETH contract data now require
      it (the documented BREAKING change); these tests never enable it
   5  chain_id refusal -- #445 working as designed; fixed by python-keepkey #215
   2  the deadlocks themselves -- THORChain and EOS extra disclosure screens
   1  a CallException still to be triaged individually

60s per test is roughly 30x the slowest healthy file in this suite (multisig,
~2s), so a genuinely slow test is not at risk.

This does not fix the underlying expectation mismatch, which belongs with the
deps/python-keepkey repin. It makes that mismatch legible in two minutes with a
test name attached, rather than as an opaque half-hour timeout -- and it is
worth keeping afterwards regardless: an integration test should never be able to
consume the whole CI budget because one side is waiting for a button
acknowledgement the other forgot to send.

Refs #466
@BitHighlander
BitHighlander merged commit 1a6d889 into release/7.14.2 Aug 17, 2026
8 of 9 checks passed
@BitHighlander
BitHighlander deleted the fix/integration-per-test-timeout branch August 17, 2026 23:16
BitHighlander added a commit that referenced this pull request Aug 17, 2026
…riant

The integration suite is red, the quickest route to green is to stop showing a
screen, and the commit that does it looks like a test fix. Writing the rule down
is the cheapest defence against that, because the pressure arrives disguised as
housekeeping and the result would look like progress.

Tests adapt to newly required disclosure. Firmware disclosure is never removed to
satisfy a stale test. If a test fails because the device now shows more, the test
is wrong. The extra THORChain screens disclose affiliate-fee bytes that were
previously signed invisibly.

Also replaces the known-red section, which was wrong in two ways. It said the
integration stall was pre-existing at 4406197 -- a controlled run (same test
files, only firmware varying) showed 1af2ffe passes and head hangs, so this
release introduced it. And it could not enumerate the failures at all, because
the job was killed before producing JUnit XML.

With #477 the suite completes in 134s and the list is finite: 22 failed, 362
passed, 47 skipped, every failure a stale pinned expectation and none a firmware
defect. Recorded as a table with an owner per row, since that is now the concrete
work list for the repin.

Adds the two measurement traps that cost real time here: do not run the suite
sequentially against one emulator, and treat the PINNED submodule commit as the
test source of truth rather than the working tree -- reading master's copy of
test__sign_transformERC20 produced a confident and wrong "this breaks nothing".

Refs #466 #474 #477
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant