diff --git a/tests/browser/workspace-activity.spec.mjs b/tests/browser/workspace-activity.spec.mjs index cbd848c..a3a1616 100644 --- a/tests/browser/workspace-activity.spec.mjs +++ b/tests/browser/workspace-activity.spec.mjs @@ -724,6 +724,42 @@ test('a held legacy job shows no Cancel in the list or its view', async ({ page, await expect(heading(page).locator('.heading-actions > button')).toHaveText(['Download report']); }); +// Since #289 an item dropped its Cancel job only when it was already in the page, so the first render of jobs the +// store already knew (Activity opened from another page) kept a Cancel job on a job that had ended, and moved a +// running job's progress after its actions. The read Activity starts on opening is held, so that first render is +// what the page shows (the read's own render would otherwise repair it a moment later). +test('jobs that ended before Activity opened show no Cancel job, a running one keeps its progress first', async ({ page, request }) => { + await page.setViewportSize({ width: 1600, height: 1100 }); + const finished = await startJob(request, { title: 'Finished scan' }); + await finish(request, finished.id); + const cancelled = await startJob(request, { title: 'Cancelled scan' }); + await finish(request, cancelled.id, 'cancelled'); + await startJob(request, { title: 'Running scan' }); + // The page's first list read has all three jobs once the running one shows in the pill. + await page.goto(`${workspace}#/images`); + await expect(pill(page)).toBeVisible(); + let hold; + const held = new Promise(resolve => { hold = resolve; }); + // Only the first read is held; any later one goes through. + await page.route(url => url.pathname === '/prompt_manager/jobs', route => { + if (!hold) return route.continue(); + hold(route); + hold = null; + }); + await activityButton(page).click(); + const route = await held; + await expect(items(page)).toHaveCount(3); + const item = title => items(page).filter({ has: page.getByRole('link', { name: new RegExp(`${title}$`) }) }); + for (const ended of ['Finished scan', 'Cancelled scan']) { + await expect(item(ended).locator('progress, .row-actions')).toHaveCount(0); + await expect(item(ended).getByRole('button', { name: 'Cancel job', exact: true })).toHaveCount(0); + } + await expect(item('Running scan').getByRole('button', { name: 'Cancel job', exact: true })).toHaveCount(1); + expect(await item('Running scan').evaluate(node => [...node.children].map(child => child.localName + (child.className ? `.${child.className}` : '')))) + .toEqual(['div.panel-heading', 'p', 'progress', 'div.row-actions']); + await route.continue(); +}); + test('busy start names the running job', async ({ page, request }) => { await page.setViewportSize({ width: 1600, height: 1100 }); // The page's timers are Playwright's, so the toast's lifetime below is run out rather than waited for. diff --git a/tests/test_database_backup.py b/tests/test_database_backup.py index deb1346..9368697 100644 --- a/tests/test_database_backup.py +++ b/tests/test_database_backup.py @@ -24,6 +24,9 @@ from database.operations import PromptDatabase from tests.open_handles import assert_closed from utils.hashing import generate_prompt_hash +# A bound on thread hand-offs that only a hung test reaches (never a pacing timer). +THREAD_WAIT_SECONDS = 120 + def _count_prompts(path): """Row count read through a brand-new connection (closed before return).""" @@ -350,7 +353,13 @@ class TestRestoreFromFile(BackupTestCase): def worker(): self.db.search_prompts(text="old") # opens this thread's connection ready.set() - proceed.wait(5) + # Wait for the restore itself, not for a timer: a stalled Windows + # runner once took longer than 5 s to restore, and the worker then + # read the old database before the restore had run. + if not proceed.wait(THREAD_WAIT_SECONDS): + results["error"] = "the restore did not finish" + self.db.close() + return try: results["texts"] = [p["text"] for p in self.db.search_prompts()] except Exception as exc: # noqa: BLE001 - reported through results @@ -360,12 +369,15 @@ class TestRestoreFromFile(BackupTestCase): thread = threading.Thread(target=worker) thread.start() - ready.wait(5) + self.assertTrue(ready.wait(THREAD_WAIT_SECONDS)) - self.db.model.restore_from_file(src) - proceed.set() - thread.join(5) + try: + self.db.model.restore_from_file(src) + finally: + proceed.set() + thread.join(THREAD_WAIT_SECONDS) + self.assertFalse(thread.is_alive()) self.assertNotIn("error", results) self.assertEqual(results["texts"], ["restored"]) self.assertEqual([p["text"] for p in self.db.search_prompts()], ["restored"]) diff --git a/web/js/workspace/activity.mjs b/web/js/workspace/activity.mjs index eb7bc14..f4deede 100644 --- a/web/js/workspace/activity.mjs +++ b/web/js/workspace/activity.mjs @@ -148,18 +148,20 @@ function itemFor(job, scope) { chip.textContent = state; detail.textContent = jobDetail(next); const running = next.state === 'running'; + // parentNode, not isConnected: on the first render the item is not in the page yet, so a job that had already + // ended when Activity opened kept its Cancel job (and a running one's progress moved after its actions). if (running) { progress.value = next.progress; progress.textContent = `${next.progress}%`; progress.setAttribute('aria-label', `${next.title} progress`); - if (!progress.isConnected) item.insertBefore(progress, actions.isConnected ? actions : null); + if (!progress.parentNode) item.insertBefore(progress, actions.parentNode ? actions : null); } else { progress.remove(); } // Truth rule: a job that cannot be cancelled (a held legacy-page job) shows no Cancel job. if (running && next.cancellable) { - if (!actions.isConnected) item.append(actions); - } else if (actions.isConnected) { + if (!actions.parentNode) item.append(actions); + } else if (actions.parentNode) { // A focused Cancel job that goes away hands its focus to the item's title link, never to the page body. if (actions.contains(document.activeElement)) link.focus(); actions.remove();