Skip to content

Commit 5dc8738

Browse files
committed
fix: 3 review findings — junit collision, SPL opcode, xref format
1. parse_junit: key by module::method to disambiguate collisions 2. SPL Token: opcode is u8 not u32 (bytes([3]) not struct.pack('<I', 3)) 3. PDF xref: use 'n' (in-use) not 'g' (free) for object entries
1 parent e80da9b commit 5dc8738

2 files changed

Lines changed: 31 additions & 18 deletions

File tree

scripts/generate-test-report.py

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ def write(self, path):
128128
f.write(b'xref\n')
129129
f.write(f'0 {len(objs)+1}\n'.encode())
130130
f.write(b'0000000000 65535 f \n')
131-
for o in offs: f.write(f'{o:010d} 00000 g \n'.encode())
131+
for o in offs: f.write(f'{o:010d} 00000 n \n'.encode())
132132
f.write(f'trailer\n<< /Size {len(objs)+1} /Root 1 0 R >>\nstartxref\n{xr}\n%%EOF\n'.encode())
133133

134134
GREEN = (0.13, 0.55, 0.13)
@@ -173,6 +173,10 @@ def gap(self, h=4):
173173
def finish(self):
174174
self._flush()
175175

176+
def _lookup(results, mod, meth):
177+
"""Look up test result by module::method (precise), then bare method (fallback)."""
178+
return results.get(f'{mod}::{meth}') or results.get(meth) or ''
179+
176180
def ver_t(s): return tuple(int(x) for x in s.replace('v','').split('.')[:3])
177181
def ver_ge(a, b): return ver_t(a) >= ver_t(b)
178182
def _w(text, n=95):
@@ -235,8 +239,8 @@ def detect_fw():
235239
except: return None
236240

237241
def parse_junit(path):
238-
"""Parse junit XML for pass/fail. Returns dict keyed by both 'classname.method' and 'method'.
239-
When names collide, pass wins over fail (avoids false negatives from unrelated test classes)."""
242+
"""Parse junit XML for pass/fail. Returns dict keyed by 'module::method' (precise)
243+
and 'method' (fallback). Module is extracted from classname: tests.test_msg_foo.TestBar → test_msg_foo."""
240244
if not path or not os.path.exists(path): return {}
241245
import xml.etree.ElementTree as ET
242246
results = {}
@@ -247,10 +251,19 @@ def parse_junit(path):
247251
elif tc.find('error') is not None: status = 'error'
248252
elif tc.find('skipped') is not None: status = 'skip'
249253
else: status = 'pass'
250-
# Key by classname.method (precise) and method-only (fallback)
254+
# Extract module from classname: tests.test_msg_foo.TestBar → test_msg_foo
255+
mod = ''
251256
if cls:
257+
parts = cls.split('.')
258+
for p in parts:
259+
if p.startswith('test_msg_') or p.startswith('test_sign_') or p.startswith('test_verify_'):
260+
mod = p
261+
break
252262
results[f'{cls}.{name}'] = status
253-
# For method-only key, pass wins over fail (avoid collision false negatives)
263+
# Key by module::method (disambiguates collisions like test_sign_btc_eth_swap)
264+
if mod:
265+
results[f'{mod}::{name}'] = status
266+
# Bare method fallback — only set if no collision
254267
if name not in results or status == 'pass':
255268
results[name] = status
256269
return results
@@ -954,12 +967,12 @@ def render(output_path, fw_version, results, screenshot_dir=None):
954967
specs = [s for s in active if not s[5]]
955968
# Sections with results first, pending sections at bottom.
956969
# Within each group: existing chains first (proven), then new features.
957-
has_results = [s for s in active if s[5] and any(results.get(t[2]) for t in s[5])]
958-
no_results = [s for s in active if s[5] and not any(results.get(t[2]) for t in s[5])]
970+
has_results = [s for s in active if s[5] and any(_lookup(results, t[1], t[2]) for t in s[5])]
971+
no_results = [s for s in active if s[5] and not any(_lookup(results, t[1], t[2]) for t in s[5])]
959972
test_sections = has_results + no_results
960973
total = sum(len(s[5]) for s in test_sections)
961-
passed = sum(1 for s in test_sections for t in s[5] if results.get(t[2]) == 'pass')
962-
failed = sum(1 for s in test_sections for t in s[5] if results.get(t[2]) in ('fail','error'))
974+
passed = sum(1 for s in test_sections for t in s[5] if _lookup(results, t[1], t[2]) == 'pass')
975+
failed = sum(1 for s in test_sections for t in s[5] if _lookup(results, t[1], t[2]) in ('fail','error'))
963976
skipped = total - passed - failed
964977

965978
# Title
@@ -975,15 +988,15 @@ def render(output_path, fw_version, results, screenshot_dir=None):
975988
pb.text(12, 'Sections', bold=True)
976989
_shown_tested = _shown_pending = False
977990
for letter, title, mf, _, _, tests in test_sections:
978-
has_any = any(results.get(t[2]) for t in tests)
991+
has_any = any(_lookup(results, t[1], t[2]) for t in tests)
979992
is_new = ver_t(mf) > (7, 10, 0)
980993
if has_any and not _shown_tested:
981994
_shown_tested = True
982995
elif not has_any and not _shown_pending:
983996
pb.text(9, f' --- Pending (no firmware support yet) ---', bold=True, color=GRAY)
984997
_shown_pending = True
985998
tag = ' [NEW]' if is_new else ''
986-
p = sum(1 for t in tests if results.get(t[2]) == 'pass')
999+
p = sum(1 for t in tests if _lookup(results, t[1], t[2]) == 'pass')
9871000
if p == len(tests) and len(tests) > 0:
9881001
pb.text(8, f' {letter} {title}{tag} -- {p}/{len(tests)} passed', color=GREEN)
9891002
elif p > 0:
@@ -1003,8 +1016,8 @@ def render(output_path, fw_version, results, screenshot_dir=None):
10031016
for line in user_flow: pb.text(7, line)
10041017
if not tests: continue
10051018
pb.gap(3)
1006-
p = sum(1 for t in tests if results.get(t[2]) == 'pass')
1007-
f_count = sum(1 for t in tests if results.get(t[2]) in ('fail','error'))
1019+
p = sum(1 for t in tests if _lookup(results, t[1], t[2]) == 'pass')
1020+
f_count = sum(1 for t in tests if _lookup(results, t[1], t[2]) in ('fail','error'))
10081021
if p == len(tests):
10091022
pb.text(9, f'Tests: {p}/{len(tests)} -- ALL PASSED', bold=True, color=GREEN)
10101023
elif f_count > 0:
@@ -1014,7 +1027,7 @@ def render(output_path, fw_version, results, screenshot_dir=None):
10141027
pb.gap(2)
10151028
for tid, mod, meth, title, ctx, scr in tests:
10161029
pb.need(50)
1017-
r = results.get(meth, '')
1030+
r = _lookup(results, mod, meth)
10181031
pb.check(9, f'{tid} {meth}', r)
10191032
pb.text(7, f'{title} ({mod}.py)')
10201033
for cline in _w(ctx, 95): pb.text(7, cline)

tests/test_msg_solana_signtx.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,8 @@ def test_solana_sign_token_transfer(self):
243243
from_pubkey = self._get_from_pubkey()
244244
to_account = b'\x33' * 32 # destination token account
245245
owner = from_pubkey # token owner = signer
246-
# SPL Token Transfer instruction: type=3 (LE u32) + amount (LE u64)
247-
instr_data = struct.pack('<I', 3) + struct.pack('<Q', 50000000) # 50M tokens
246+
# SPL Token Transfer instruction: opcode=3 (u8) + amount (LE u64)
247+
instr_data = bytes([3]) + struct.pack('<Q', 50000000) # 50M tokens
248248
raw_tx = self._build_tx(from_pubkey, [to_account], self.TOKEN_PROGRAM, instr_data)
249249
resp = self.client.call(messages.SolanaSignTx(
250250
address_n=parse_path("m/44'/501'/0'/0'"), raw_tx=raw_tx))
@@ -256,8 +256,8 @@ def test_solana_sign_token_approve(self):
256256
self.setup_mnemonic_allallall()
257257
from_pubkey = self._get_from_pubkey()
258258
delegate = b'\x44' * 32
259-
# SPL Token Approve: type=4 (LE u32) + amount (LE u64)
260-
instr_data = struct.pack('<I', 4) + struct.pack('<Q', 100000000)
259+
# SPL Token Approve: opcode=4 (u8) + amount (LE u64)
260+
instr_data = bytes([4]) + struct.pack('<Q', 100000000)
261261
raw_tx = self._build_tx(from_pubkey, [delegate], self.TOKEN_PROGRAM, instr_data)
262262
resp = self.client.call(messages.SolanaSignTx(
263263
address_n=parse_path("m/44'/501'/0'/0'"), raw_tx=raw_tx))

0 commit comments

Comments
 (0)