diff --git a/.github/workflows/portability.yml b/.github/workflows/portability.yml index 43782e03..0a8767e3 100644 --- a/.github/workflows/portability.yml +++ b/.github/workflows/portability.yml @@ -21,15 +21,16 @@ env: PYTHONFAULTHANDLER: "1" # Time limit rule as in test.yml: ceil(1.5 x the slowest of three recent green runs on main) + 3 -# minutes, never under 5. Measured 2026-10-26 in test.yml on runs 40257217681 (3be0ada), -# 40240706568 (fb9134f), 40228167328 (f9835aa), before these legs moved here in #420: -# slowest py3.10 on windows-latest: 12m43s / 15m54s / 14m53s -> 27 min -# (macos 6m14s-8m30s, windows 11m42s-15m54s) +# minutes, never under 5, timed as whole-job wall time. One matrix job, so the slowest leg of each +# run counts (tests/vendor/ci-workflow.test.mjs recomputes it, #493). +# Measured 2026-10-29 on main runs 40935337099 (dd247e3), 40938407252 (dd247e3), 40941240624 (dd247e3): +# slowest leg: 7m24s (py3.12 windows) / 7m31s (py3.13 windows) / 7m32s (py3.14 windows) -> 15 min +# (windows 4m56s-7m32s, macos 4m39s-6m14s) jobs: test: name: py${{ matrix.python-version }} on ${{ matrix.os }} runs-on: ${{ matrix.os }} - timeout-minutes: 27 + timeout-minutes: 15 strategy: fail-fast: false matrix: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 85194d1a..50a7cdd2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,8 +28,8 @@ env: # py matrix, slowest py3.10 on windows-latest: 12m43s / 15m54s / 14m53s -> 27 min # (ubuntu 5m27s-6m27s, macos 6m14s-8m30s, windows 11m42s-15m54s) # 3.10 and 3.11 left the matrix and 3.14 joined it in #356; re-measure after three green main runs. -# Windows and macOS moved to portability.yml (#420), which keeps this 27-minute limit; this -# Linux-only matrix keeps it too until it is re-measured. +# Windows and macOS moved to portability.yml (#420), which has its own measured limit (#493); +# this Linux-only matrix keeps 27 minutes until it is re-measured. # real ComfyUI compatibility: 13m32s / 8m55s / 11m46s -> 24 min # coverage gate (85%): 7m26s / 7m22s / 7m54s -> 15 min # perf budget 30s / 29s / 27s, black and flake8 39s / 37s / 39s, diff --git a/TESTING.md b/TESTING.md index 81ebfcb2..eaa1a77a 100644 --- a/TESTING.md +++ b/TESTING.md @@ -267,11 +267,13 @@ the process exits non-zero. use `\\localhost\`) or a default TLS context (it reads the Windows certificate store) costs seconds there; the slowest list shows it. -Every job in `test.yml`, `code-quality.yml` and `viewerjs-check.yml` has -`timeout-minutes` (`tests/vendor/ci-workflow.test.mjs` checks it). The rule is -ceil(1.5 x the slowest of three recent green runs on `main`) + 3 minutes, never -under 5; the three run ids, their times and the resulting limits are written -in a comment above `jobs:`. The 3 minutes leave room for one watchdog firing, so +Every job in `test.yml`, `code-quality.yml`, `viewerjs-check.yml` and +`portability.yml` has `timeout-minutes` (`tests/vendor/ci-workflow.test.mjs` +checks it). The rule is ceil(1.5 x the slowest of three recent green runs on +`main`) + 3 minutes, never under 5, timed as whole-job wall time; the three run +ids, their times and the resulting limits are written in a comment above +`jobs:`. For `portability.yml` the tooling test recomputes the limit from that +comment (#493); its one matrix job takes the slowest Windows or macOS leg. The 3 minutes leave room for one watchdog firing, so the dump prints before GitHub cancels the job. A job that hits its limit, or a watchdog dump, is a failure like any other: read the dump, find the blocking call, fix it. Never raise a limit or rerun to get green; a limit changes only diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index f40e0afc..b6cb2630 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -1376,6 +1376,61 @@ async function expectStar(button, favorite) { await expect(button).toHaveAttribute('aria-pressed', String(favorite)); } +// #494: the Favorites count is re-read 300 ms after a star changes. A page that has begun to leave starts no count read +// (WebKit fails it with "Fetch API cannot load … due to access control checks.", reported as a page error; queue run +// 40934368716). A leave that is called off counts again on the next input. +for (const leave of ['beforeunload', 'pagehide']) { + test(`a star changed just before ${leave} starts no Favorites count read while the page leaves`, async ({ page }) => { + await page.clock.install(); + const countReads = []; + page.on('request', item => { + const url = new URL(item.url()); + if (url.pathname === '/prompt_manager/images/output' && url.searchParams.get('favorites') === '1' && url.searchParams.get('limit') === '1') countReads.push(url.search); + }); + await page.goto(workspace); + const card = page.locator('article[data-image-key="0:shared/same.png"]'); + await expectStar(star(card, 'same.png'), false); + await expect.poll(() => countReads.length).toBe(1); + await star(card, 'same.png').click(); + await expectStar(star(card, 'same.png'), true); + await expect(toast(page)).toHaveText('Added to favorites.'); + await page.evaluate(type => window.dispatchEvent(new Event(type)), leave); + await page.clock.runFor(1000); + // A read made after the clock ran is listed, so the listener saw every count read. + await page.evaluate(() => fetch('/prompt_manager/images/output?mode=workspace&favorites=1&limit=1&probe=1')); + await expect.poll(() => countReads.length).toBe(2); + expect(countReads.at(-1)).toContain('probe=1'); + if (leave === 'beforeunload') { + // The leave was called off (the page is still here): the next input counts again, once, and the badge shows it. + await page.keyboard.press('Shift'); + await expect.poll(() => countReads.length).toBe(3); + expect(countReads.at(-1)).not.toContain('probe=1'); + await expect(page.locator('nav [data-route="favorites"] .nav-count')).toHaveText('1'); + await page.clock.runFor(1000); + expect(countReads).toHaveLength(3); + } + }); +} + +test('a count read in flight when the page starts to leave is dropped, then read again if the page stays (#494)', async ({ page }) => { + const held = []; + // The page's first Library count read is held at the server boundary while the page starts to leave. + await page.route('**/prompt_manager/library?limit=1', route => { held.push(route); }); + await page.goto(workspace); + await expect.poll(() => held.length).toBe(1); + const failed = page.waitForEvent('requestfailed', item => item.url().includes('/prompt_manager/library?limit=1')); + await page.evaluate(() => window.dispatchEvent(new Event('beforeunload'))); + await failed; + await page.unroute('**/prompt_manager/library?limit=1'); + await held[0].continue().catch(() => {}); + await expect(page.locator('nav [data-route="library"] .nav-count')).toHaveCount(0); + // The page stayed: the next input reads the dropped count again. + const reread = page.waitForRequest(item => item.url().includes('/prompt_manager/library?limit=1')); + await page.keyboard.press('Shift'); + await reread; + await expect(page.locator('nav [data-route="library"] .nav-count')).toHaveText(/^\d+$/); +}); + test('favorite persists across reload', async ({ page, request }) => { await expect(await request.post('/__test__/reset', { data: { include_video: true } })).toBeOK(); const sent = []; diff --git a/tests/vendor/ci-workflow.test.mjs b/tests/vendor/ci-workflow.test.mjs index c146bb58..19bc5ae8 100644 --- a/tests/vendor/ci-workflow.test.mjs +++ b/tests/vendor/ci-workflow.test.mjs @@ -353,6 +353,26 @@ test('Windows and macOS run 3.12-3.14 on main, nightly and on demand, never on a assert.doesNotMatch(text, /cancel-in-progress: true|cancel-in-progress: \$/, 'no portability run is cancelled'); }); +// #493: the portability limit follows the rule in its comment: ceil(1.5 x the slowest of three +// green main runs) + 3 minutes, never under 5, with those runs and times written above jobs:. +// The times are whole-job wall times. That the runs were green and on main is the author's word. +test('the portability limit is computed from three measured green main runs', () => { + const text = readWorkflow('portability.yml'); + const comment = text.slice(0, text.indexOf('\njobs:')); + const runs = [...comment.matchAll(/^# Measured \d{4}-\d{2}-\d{2} on main runs (\d+) \([0-9a-f]{7}\), (\d+) \([0-9a-f]{7}\), (\d+) \([0-9a-f]{7}\):$/gm)]; + assert.equal(runs.length, 1, 'the comment names the three runs it measured, once'); + assert.equal(new Set(runs[0].slice(1)).size, 3, 'three different runs'); + const time = '(\\d+)m([0-5]\\d)s \\(([^)]+)\\)'; + const slowest = new RegExp(`^#\\s+slowest leg: ${time} / ${time} / ${time} -> (\\d+) min$`, 'm').exec(comment); + assert.ok(slowest, 'the comment gives the slowest leg of each run and the limit'); + const seconds = [1, 4, 7].map(i => Number(slowest[i]) * 60 + Number(slowest[i + 1])); + const expected = Math.max(5, Math.ceil(1.5 * Math.max(...seconds) / 60) + 3); + assert.equal(Number(slowest[10]), expected, 'the written limit follows the rule'); + assert.equal(comment.match(/-> \d+ min/g).length, 1, 'no older measurement stays beside it'); + const limit = Number(/^ {4}timeout-minutes: (\d+)\s*$/m.exec(jobs(text).get('test'))[1]); + assert.equal(limit, expected, 'the job uses the written limit'); +}); + test('real ComfyUI runs only outside pull requests; the other test.yml checks run on every event', () => { const found = testJobs(); assert.match(found.get('comfyui'), QUEUE_ONLY_IF); diff --git a/web/js/workspace/index.mjs b/web/js/workspace/index.mjs index 951abd1c..f75483fb 100644 --- a/web/js/workspace/index.mjs +++ b/web/js/workspace/index.mjs @@ -138,7 +138,13 @@ function mountWorkspace() { onSave: name => savedViews.create(page, name, state) }); // One read per count, so a write that refreshes Library and Tags never supersedes an Images read in flight. const countRequests = new Map(); + // #494: a page that has begun to leave (beforeunload) starts no count read: WebKit fails one with "Fetch API cannot + // load … due to access control checks." (a page error). Counts asked for meanwhile wait; a leave that is called off + // (the page is still here) reads them on its next input, as the legacy admin page does (#290). + let leaving = false; + const deferredCounts = new Set(); function refreshCounts(ids) { + if (leaving) { for (const id of ids) deferredCounts.add(id); return; } for (const id of ids) { countRequests.get(id)?.abort(); const request = new AbortController(); @@ -168,9 +174,29 @@ function mountWorkspace() { let favoriteCountTimer = 0; scope.cleanup(onFavoriteChange(() => { clearTimeout(favoriteCountTimer); - favoriteCountTimer = setTimeout(() => refreshCounts(FAVORITE_WRITE_COUNTS), FAVORITE_COUNT_DELAY_MS); + favoriteCountTimer = setTimeout(() => { favoriteCountTimer = 0; refreshCounts(FAVORITE_WRITE_COUNTS); }, FAVORITE_COUNT_DELAY_MS); })); scope.cleanup(() => clearTimeout(favoriteCountTimer)); + // Any beforeunload: a page that stays (Stop, a download, a "stay" prompt) revives on its next input or when shown. + scope.listen(window, 'beforeunload', () => { + leaving = true; + if (favoriteCountTimer) { clearTimeout(favoriteCountTimer); favoriteCountTimer = 0; for (const id of FAVORITE_WRITE_COUNTS) deferredCounts.add(id); } + for (const [id, request] of countRequests) { request.abort(); deferredCounts.add(id); } + countRequests.clear(); + }); + const revive = () => { + if (!leaving) return; + leaving = false; + const ids = [...deferredCounts]; deferredCounts.clear(); + if (ids.length) refreshCounts(ids); + if (restorePending) { restorePending = false; restoreReload.request(); } + }; + // Capture, as the admin page does, so no handler that stops an input keeps the page from reviving. + for (const type of ['pointerdown', 'keydown']) { + window.addEventListener(type, revive, true); + scope.cleanup(() => window.removeEventListener(type, revive, true)); + } + scope.listen(document, 'visibilitychange', () => { if (document.visibilityState === 'visible') revive(); }); // A job that ends re-reads the counts it can change, once per ended job: a metadata scan may have added items to // Needs review; deleting images (#480) hides or trashes them, with their favorites, review items and collection // memberships (#501). @@ -184,14 +210,17 @@ function mountWorkspace() { } if (!ended.size) return; refreshCounts([...new Set([...ended].flatMap(kind => JOB_END_COUNTS[kind]))]); - if (ended.has('image_delete')) collections.load(); + if (ended.has('image_delete') && !leaving) collections.load(); })); refreshCounts(NAV_COUNT_ROUTES); // S9: a finished restore replaced the library. The cached visits are dropped, the counts and saved views read // again, and the open page is shown again from the restored data; the Activity job view the restore opened stays. // #415: an open review closes (its plan no longer describes storage); while any other dialog is open, all of it // waits until the last one has closed, so the dialog keeps what was typed. + // #494: a restore that ends while the page leaves reloads nothing then; a page that stays reloads on revive. + let restorePending = false; const restoreReload = createRestoreReload(() => { + if (leaving) { restorePending = true; return; } forgetImagesVisit(); forgetLibraryVisit(); refreshCounts(NAV_COUNT_ROUTES);