From 22a0a58f27e9fa73f16d068c7d00e291f2a057fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D0=BD=D1=82=D0=BE=D0=BD=20=D0=A2=D1=80=D0=BE=D1=88?= =?UTF-8?q?=D0=B8=D0=BD?= Date: Tue, 6 Oct 2026 00:41:11 +0300 Subject: [PATCH] v3.22.0 (review fixes): the suite members share the SCP80 counter; drain/PWA nits The v3.22.0 review found that a suite's SCP80 counter state was rebuilt from the preset snapshot for every member: a second SCP80 script sent the counter the first had consumed (the store persists the advance, the in-memory preset copy does not move) and the card answered cntr_low. Reproduced with a two-member suite (both packets carried CNTR=0000000010). - runner: `_test_run_script` takes a `shared` counter state (created once per suite, passed to every member; a single script passes None) and copies the no-keyset counter back into it, so a suite behaves like one script for counter bookkeeping. Pinned by a two-member test asserting the second packet's counter is the first's + 1 and the final counters. - `_test_drain_pending`: the unknown-length FETCH uses 0xff - `'%02x' % 0x100` emitted an odd-length APDU (a malformed fetch) when a 9100 SW (length 0) reached the drain. - PWA: the suite save adopts the stored suite (the server normalises the setup-first/teardown-last order); the role rows' up/down buttons are disabled (only members move); deleting a suite cancels its pending name debounce (no late 404); `refreshDynamicI18n` redraws the test views, so a language switch updates the suites list/report immediately. - tests: the cross-member counter test (Python) and the save-adoption / move-guard / i18n wiring tests (Node). Tests: 835 frontend + 795 Python; sw cache simple-v398. --- frontend/index.html | 17 ++++++++++++-- frontend/sw.js | 2 +- frontend/tests/testsuites.test.js | 37 +++++++++++++++++++++++++++++- pysim_simple_server/server.py | 30 +++++++++++++++++++----- tests/test_testscript.py | 38 +++++++++++++++++++++++++++---- 5 files changed, 110 insertions(+), 14 deletions(-) diff --git a/frontend/index.html b/frontend/index.html index c909a80..ec1d02c 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -7319,6 +7319,8 @@ async function testSuiteDelete(i) { const s = testSuites()[i]; if (!s) return; if (!confirm(t('Delete this test suite?'))) return; + // a pending debounced name write must not 404 after the delete + if (_testSuiteSaveTimer) { clearTimeout(_testSuiteSaveTimer); _testSuiteSaveTimer = null; } try { const resp = await pysimFetch('/api/test/suites/delete', {id: s.id}); if (resp.error) throw new Error(resp.error); @@ -7381,6 +7383,13 @@ function testSuiteSave(immediate) { id: s.id, suite: {name: s.name || '', require_adm: !!s.require_adm, scripts: s.scripts || []}}); if (resp.error) throw new Error(resp.error); + // Adopt the stored suite: the server normalises the order (setup + // first, teardown last) and may adjust the roles. + if (resp.suite) { + const i = testSuites().findIndex(x => x.id === s.id); + if (i >= 0) _testSuites[i] = resp.suite; + if (_testView === 'suite') testRenderSuiteScripts(); + } } catch (e) { ioStatus('test-io-status', t('Could not save the test suite') + ': ' + e.message); } @@ -7420,14 +7429,17 @@ function testRenderSuiteScripts() { entries.forEach((e, i) => { const script = testScriptById(e.script_id); const name = script ? (script.name || script.id) : ('(' + t('unknown script') + ': ' + String(e.script_id || '').slice(0, 8) + ')'); + // The roles define the order (setup first, teardown last): only the + // member rows can be moved. + const movable = e.role === 'member'; html += '
' + '' + (i + 1) + '' + roleBadge(e.role) + '' + esc(name) + '' + '' + '' - + '' - + '' + + '' + + '' + (script ? '' : '') + (script ? '' : '') + '' @@ -22505,6 +22517,7 @@ function refreshDynamicI18n() { pysimProactiveLogRender(); } if (isViewVisible('phone-sub-tr')) pysimPliRender(); + if (isViewVisible('phone-sub-test')) testRender(); if (isViewVisible('phone-sub-esim')) { esimRenderChip(); esimRenderProfiles(); diff --git a/frontend/sw.js b/frontend/sw.js index d176a9a..35019e1 100644 --- a/frontend/sw.js +++ b/frontend/sw.js @@ -1,4 +1,4 @@ -const CACHE = 'simple-v397'; +const CACHE = 'simple-v398'; const URLS = [ 'index.html', 'help.html', diff --git a/frontend/tests/testsuites.test.js b/frontend/tests/testsuites.test.js index f44105c..d4912ef 100644 --- a/frontend/tests/testsuites.test.js +++ b/frontend/tests/testsuites.test.js @@ -26,9 +26,13 @@ function extractFunc(src, name, asyncFn) { let code = ''; for (const fn of ['testStatusBadge', 'testRunStepHtml', 'testReportMemberHtml', - 'testReportMarkdown', 'testSuiteProgress', 'testRoleOptions', 'testFailOptions']) { + 'testReportMarkdown', 'testSuiteProgress', 'testRoleOptions', 'testFailOptions', + 'testSuiteSave']) { code += extractFunc(html, fn) + '\n'; } +code += 'var _testSuites = []; var _testScripts = []; var _testSuiteIdx = -1;' + + ' var _testView = "suite"; var _testSuiteSaveTimer = null;' + + ' var _testSuiteTarget = null; var _testScriptId = null; var _testScriptDraft = null;\n'; eval(code); globalThis.t = s => s; globalThis.esc = s => String(s); @@ -149,3 +153,34 @@ test('a single script run sends its require_adm flag and keeps its id', () => { assert.match(src, /body\.script_id = cur\.id/); assert.match(src, /testRenderRun\(resp\)/); }); + +test('the suite save adopts the stored suite (the server normalises the order)', async () => { + const calls = []; + const suite = {id: 'a'.repeat(32), name: 'edited', require_adm: false, scripts: []}; + _testSuites = [suite]; + _testSuiteIdx = 0; + globalThis.testCurrentSuite = () => suite; + globalThis.testSuites = () => _testSuites; + globalThis.testRenderSuiteScripts = () => {}; + globalThis.ioStatus = () => {}; + globalThis.pysimFetch = async (path, body) => { + calls.push([path, body]); + return {ok: true, suite: Object.assign({}, suite, {name: 'stored'})}; + }; + testSuiteSave(true); + await new Promise(r => setTimeout(r, 10)); + assert.strictEqual(calls[0][0], '/api/test/suites/update'); + assert.strictEqual(calls[0][1].suite.scripts.length, 0); + assert.strictEqual(_testSuites[0].name, 'stored'); +}); + +test('role rows cannot be moved and the i18n refresh redraws the test views', () => { + const rows = extractFunc(html, 'testRenderSuiteScripts'); + assert.match(rows, /const movable = e\.role === 'member'/); + assert.match(rows, /!movable \|\| i === 0/); + assert.match(rows, /!movable \|\| i === entries\.length - 1/); + const del = extractFunc(html, 'testSuiteDelete', true); + assert.match(del, /_testSuiteSaveTimer/); + const i18n = extractFunc(html, 'refreshDynamicI18n'); + assert.match(i18n, /isViewVisible\('phone-sub-test'\)\) testRender\(\)/); +}); diff --git a/pysim_simple_server/server.py b/pysim_simple_server/server.py index 79c1fdc..fe68bea 100644 --- a/pysim_simple_server/server.py +++ b/pysim_simple_server/server.py @@ -6873,8 +6873,10 @@ def _test_drain_pending(scc, fetch_len, result=0x00, max_commands=3): answered = [] try: while len(answered) < max_commands: + # fetch_len 0 (a 9100 SW) means "length unknown": fetch the full + # 255 bytes - `%02x` of 0x100 would emit an odd-length APDU fdata, sw = scc._tp.send_apdu('%s120000%02x' % (scc.cat_cla, - fetch_len or 0x100)) + fetch_len or 0xff)) if not fdata: break raw = bytes.fromhex(fdata) @@ -6904,16 +6906,24 @@ def _test_drain_pending(scc, fetch_len, result=0x00, max_commands=3): for a in answered)} -def _test_run_script(server, script, preset): +def _test_run_script(server, script, preset, shared=None): """One script's steps - the single-script run or one suite member. The live steps land in `_TEST_RUN['steps']` (the snapshot the PWA renders), the semantic lines in `_TEST_RUN['log']` (the report's excerpt). Returns ``(ctx, stopped)``: the per-script context (preset, per-keyset counters) - and whether the stop request cut the script short.""" + and whether the stop request cut the script short. + + `shared` carries the counter state across a suite's members (the SCP80 + counter is per keyset and per card session: a second member must start + from what the first consumed - the server persisted it, but the in-memory + preset copy is the suite-start snapshot). The dict is created once per + suite and reused; a single script passes None.""" + shared = shared if shared is not None else {} scc = server.scc ctx = {'preset': dict(preset or {}), - 'counter': str((preset or {}).get('counter') or (preset or {}).get('cntr') or '').upper(), - 'counters': {}, + 'counter': (shared.get('counter') + or str((preset or {}).get('counter') or (preset or {}).get('cntr') or '').upper()), + 'counters': shared.setdefault('counters', {}), 'uses_scp80': any(s['type'] == 'action' and s['kind'] == 'scp80' for s in script['steps'])} if ctx['uses_scp80']: @@ -6984,6 +6994,9 @@ def _test_run_script(server, script, preset): with _TEST_LOCK: _TEST_RUN['scp80_counter'] = ctx['counter'] _TEST_RUN['scp80_counters'] = dict(ctx.get('counters') or {}) + # The no-keyset counter is a plain value in the ctx: carry it into the + # shared state so the next suite member starts where this one ended. + shared['counter'] = ctx.get('counter') or '' return ctx, stopped @@ -7180,6 +7193,11 @@ def _test_suite_execute(server, suite, members, preset, preset_id): teardown_i = next((i for i, (e, _s) in enumerate(members) if e['role'] == 'teardown'), None) member_idx = [i for i, (e, _s) in enumerate(members) if e['role'] == 'member'] + # The counter state is per card session: one shared dict for the whole + # suite, so a second SCP80 script starts from what the first consumed + # (the store persists each packet's counter; the preset dict here is the + # suite-start snapshot). + shared = {} def run_member(i): entry, script = members[i] @@ -7192,7 +7210,7 @@ def _test_suite_execute(server, suite, members, preset, preset_id): _TEST_RUN['total'] = len(script['steps']) _TEST_RUN['log_prefix'] = _test_member_prefix(i, member) log_from = len(_TEST_RUN['log']) - ctx, member_stopped = _test_run_script(server, script, preset) + ctx, member_stopped = _test_run_script(server, script, preset, shared=shared) with _TEST_LOCK: steps = json.loads(json.dumps(_TEST_RUN['steps'])) log = list(_TEST_RUN['log'][log_from:]) diff --git a/tests/test_testscript.py b/tests/test_testscript.py index 95ddc81..e0f12cf 100644 --- a/tests/test_testscript.py +++ b/tests/test_testscript.py @@ -1615,8 +1615,8 @@ class TestSuiteRunner(SuiteRunnerTestCase): real = S._test_run_script calls = [] - def wrapped(srv, script, preset): - ctx, stopped = real(srv, script, preset) + def wrapped(srv, script, preset, shared=None): + ctx, stopped = real(srv, script, preset, shared=shared) calls.append(script['name']) if srv is server and len(calls) == 1: S._TEST_RUN['stop'] = True # the operator stops here @@ -1638,8 +1638,8 @@ class TestSuiteRunner(SuiteRunnerTestCase): server = FakeServer(scc) real = S._test_run_script - def wrapped(srv, script, preset): - ctx, stopped = real(srv, script, preset) + def wrapped(srv, script, preset, shared=None): + ctx, stopped = real(srv, script, preset, shared=shared) if srv is server and len(scc.sent) <= 1: srv.card_session += 1 # the card reset mid-suite return ctx, stopped @@ -1668,6 +1668,36 @@ class TestSuiteRunner(SuiteRunnerTestCase): self.assertEqual(run['suite']['members'][1]['note'], 'no card in the reader') + def test_scp80_members_share_the_counter_state(self): + # The counter is per keyset and per card session: the second SCP80 + # script must send what the first consumed + 1, not the suite-start + # snapshot again (the store persists each packet's counter, but the + # in-memory preset copy does not move). + built = [] + server = FakeServer(FakeScc()) + step = {'type': 'action', 'kind': 'scp80', + 'params': {'apdu': '80E2900000', 'tar': 'B00000'}} + script = T.normalise_script({'name': 'scp80', 'steps': [step]}, + S._test_command_type) + preset = {'id': 'p1', + 'keysets': [{'kic': '15', 'kid': '15', 'kicKey': '00' * 16, + 'kidKey': '00' * 16, 'cntr': '0000000010'}], + 'tars': [{'role': 'isd', 'tar': 'B00000', 'msl': '16'}]} + members = [({'script_id': 'a' * 32, 'role': 'member', 'on_fail': 'stop'}, script), + ({'script_id': 'b' * 32, 'role': 'member', 'on_fail': 'stop'}, script)] + with mock.patch.object(S, '_build_secured_packet', + side_effect=lambda *a, **k: ( + built.append(a[5]) or ('00' * 20, {}))), \ + mock.patch.object(S, '_send_secured_packet', return_value={ + 'success': True, 'sw': '9000', 'response_data': '', + 'bytes': 10, 'segments': 1}), \ + mock.patch.object(S, '_decode_por', return_value=None), \ + mock.patch.object(S, '_preset_counter_persist', return_value=None): + run = self.suite_run(server, members, preset=preset) + self.assertEqual(built, ['0000000010', '0000000011']) + self.assertEqual([m['status'] for m in run['suite']['members']], ['ok', 'ok']) + self.assertEqual(run['scp80_counters'], {1: '0000000012'}) + class TestAdmGate(unittest.TestCase): """The run prerequisite: a script/suite flagged `require_adm` runs only