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.
This commit is contained in:
+15
-2
@@ -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 += '<div class="flex items-center gap-1.5 px-2 py-1.5">'
|
||||
+ '<span class="w-5 text-right text-xs text-gray-400">' + (i + 1) + '</span>'
|
||||
+ roleBadge(e.role)
|
||||
+ '<span class="flex-1 truncate ' + (script ? '' : 'text-red-500') + '">' + esc(name) + '</span>'
|
||||
+ '<select onchange="testSuiteRoleChanged(' + i + ', this.value)" class="' + sel + '">' + testRoleOptions(e.role) + '</select>'
|
||||
+ '<select onchange="testSuiteFailChanged(' + i + ', this.value)" class="' + sel + '" title="' + esc(t('on failure')) + '">' + testFailOptions(e.on_fail) + '</select>'
|
||||
+ '<button onclick="testSuiteMoveScript(' + i + ',-1)" class="px-1.5 text-xs text-gray-500 hover:text-gray-800 disabled:opacity-30" ' + (i === 0 ? 'disabled' : '') + '>↑</button>'
|
||||
+ '<button onclick="testSuiteMoveScript(' + i + ',1)" class="px-1.5 text-xs text-gray-500 hover:text-gray-800 disabled:opacity-30" ' + (i === entries.length - 1 ? 'disabled' : '') + '>↓</button>'
|
||||
+ '<button onclick="testSuiteMoveScript(' + i + ',-1)" class="px-1.5 text-xs text-gray-500 hover:text-gray-800 disabled:opacity-30" ' + (!movable || i === 0 ? 'disabled' : '') + '>↑</button>'
|
||||
+ '<button onclick="testSuiteMoveScript(' + i + ',1)" class="px-1.5 text-xs text-gray-500 hover:text-gray-800 disabled:opacity-30" ' + (!movable || i === entries.length - 1 ? 'disabled' : '') + '>↓</button>'
|
||||
+ (script ? '<button data-needs="card" onclick="testOpenScript(\'' + e.script_id + '\', true)" class="px-2 py-0.5 text-xs rounded bg-emerald-600 text-white hover:bg-emerald-700 disabled:opacity-40 disabled:cursor-not-allowed">' + esc(t('Run')) + '</button>' : '')
|
||||
+ (script ? '<button onclick="testOpenScript(\'' + e.script_id + '\')" class="' + btn + '">' + esc(t('Edit')) + '</button>' : '')
|
||||
+ '<button onclick="testSuitePickTarget(' + i + ',\'move\')" class="' + btn + '">' + esc(t('Move')) + '</button>'
|
||||
@@ -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();
|
||||
|
||||
+1
-1
@@ -1,4 +1,4 @@
|
||||
const CACHE = 'simple-v397';
|
||||
const CACHE = 'simple-v398';
|
||||
const URLS = [
|
||||
'index.html',
|
||||
'help.html',
|
||||
|
||||
@@ -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\(\)/);
|
||||
});
|
||||
|
||||
@@ -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:])
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user