From 567a12c7584d5a084cd26351d96ac4fe9c83bc7e Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Sun, 5 Jul 2026 11:29:36 -0400 Subject: [PATCH] tests(11): make uploadModal-reset zombie-callback test deterministic (correct mock boundary) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "guard against zombie callbacks" subtest passed its asserts but leaked async activity after the test ended (node:test flags that as failure). Root cause: it mocked modal.generateTorrentMetadata / resolveUploadIdentifier, but handleVideoSelection drives the upload through this.mediaUploader.uploadVideo — a DIFFERENT object with its own helpers — so the mediaUploader still ran real createTorrentMetadata()/hashing on a synthetic {name:"video.mp4"} file ("invalid input type" async) and captureVideoMetadata() ran URL.createObjectURL(file) ("must be an instance of Blob"). Not intermittent — a consistent wrong-boundary mock. Fix: mock at the real DI boundary — modal.mediaUploader.uploadVideo = () => delayedUpload (controllable pending promise) — and stub captureVideoMetadata. Hermetic, deterministic (3/3 exit 0, # pass 3). Mutation-verified: breaking the videoUploadId zombie guard makes the test fail. Production code untouched. See TEST_INTEGRITY.md. NOTE: the runner is fail-fast, so the full suite surfaces pre-existing failures one at a time. tests/unit/hashChangeHandler.test.mjs is next (consistent AssertionError on "loads default view if hash is empty" — likely the same stale-default bug the sibling tests/app/hash-change-handler.test.mjs already had when TRENDING became the logged-out default). Enumerating full scope next. Co-Authored-By: Claude Opus 4.8 --- TEST_INTEGRITY.md | 40 +++++++++++++++++++++++++++++ tests/ui/uploadModal-reset.test.mjs | 26 ++++++++++--------- todo/TODO_2026-06-20_pre-launch.md | 18 ++++++++++--- 3 files changed, 68 insertions(+), 16 deletions(-) diff --git a/TEST_INTEGRITY.md b/TEST_INTEGRITY.md index a8c03581..eda16051 100644 --- a/TEST_INTEGRITY.md +++ b/TEST_INTEGRITY.md @@ -510,6 +510,46 @@ test_integrity_note: if_true_explain_spec_basis: "" ``` +## 2026-07-05 — todo-11: uploadModal-reset zombie-callback test made deterministic (correct mock boundary) + +```yaml +test_integrity_note: + change_type: ["flake_fix", "refactor_tests"] + scenarios: + - id: SCN-upload-zombie-callback-guard + given: "An upload is in progress (videoUploadState.status === 'uploading') via a controllable pending mediaUploader.uploadVideo" + when: "resetUploads() runs mid-upload (bumping videoUploadId), then the in-flight upload later resolves (a 'zombie' completion)" + then: "The stale completion is ignored — status stays 'idle' and results.videoUrl stays '' — because handleVideoSelection guards on videoUploadId !== currentUploadId" + observable_outcomes: + - "videoUploadState.status: 'uploading' after start, 'idle' after reset, still 'idle' after the zombie resolve" + - "results.videoUrl.value === '' after the zombie resolve" + - "test exits 0 with no async-after-test activity (was leaking webtorrent/Blob errors)" + determinism_controls: + - "mediaUploader.uploadVideo replaced with a manually-resolved promise; captureVideoMetadata stubbed; jsdom DOM; no webtorrent, no network, no real Blob ops" + anti_cheat_rationale: + prevents: + - "over-mocking internal logic" + - "retry/sleep-based flake masking" + note: | + FLAKE_FIX + REFACTOR_TESTS, no assertion weakened. The subtest PASSED its asserts but + leaked async activity after the test ended, which node:test flags as a failure. Root + cause: it mocked modal.generateTorrentMetadata / resolveUploadIdentifier, but + handleVideoSelection drives the upload through this.mediaUploader.uploadVideo — a + DIFFERENT object with its OWN helpers — so the mediaUploader still ran real + createTorrentMetadata()/hashing on a synthetic {name:'video.mp4'} file ("invalid input + type" async), and captureVideoMetadata() ran URL.createObjectURL(file) ("must be an + instance of Blob"). Fixed by mocking at the correct DI boundary + (modal.mediaUploader.uploadVideo returns the controllable pending promise) and stubbing + captureVideoMetadata. The unit under test is the modal's videoUploadId zombie-guard, so + mocking the uploader boundary is correct isolation, not over-mocking (real upload/torrent + paths are covered by the mediaUploader / s3-upload suites). + mutation_verification: + - "Broke the guard (`if (this.videoUploadId !== currentUploadId) return` → `if (false)`) → the test FAILS ('State should remain idle after zombie completion', # fail 1). Reverted." + relaxation: + did_relax_any_assertion: false + if_true_explain_spec_basis: "" +``` + ## 2026-07-05 — todo-11b: un-quarantine dm-block-filter (hermetic subscription; no force-exit) ```yaml diff --git a/tests/ui/uploadModal-reset.test.mjs b/tests/ui/uploadModal-reset.test.mjs index 1e4ff83d..40629d60 100644 --- a/tests/ui/uploadModal-reset.test.mjs +++ b/tests/ui/uploadModal-reset.test.mjs @@ -151,17 +151,9 @@ describe("UploadModal Reset Logic", () => { let resolveUpload; const delayedUpload = new Promise(r => { resolveUpload = r; }); - const slowR2Service = { - prepareUpload: async () => ({ - settings: {}, - bucketEntry: { bucket: 'test', publicBaseUrl: 'https://test.com' } - }), - uploadFile: () => delayedUpload // Return promise that waits - }; - const modal = new UploadModal({ storageService: mockStorageService, - r2Service: slowR2Service, + r2Service: mockR2Service, getCurrentPubkey: () => "pk1", removeTrackingScripts: () => {}, setGlobalModalState: () => {}, @@ -175,9 +167,19 @@ describe("UploadModal Reset Logic", () => { modal.activeProvider = "cloudflare_r2"; modal.activeCredentials = { accountId: 'acc', accessKeyId: 'key', secretAccessKey: 'sec' }; - // Mock generateTorrentMetadata BEFORE calling handleVideoSelection - modal.generateTorrentMetadata = async () => ({ hasValidInfoHash: false }); - modal.resolveUploadIdentifier = async () => "mock-identifier"; + // handleVideoSelection() drives the upload through this.mediaUploader.uploadVideo + // — NOT the modal's own generate/resolve helpers — so mock at THAT boundary. + // Returning the controllable pending promise lets us reset mid-upload and then + // release a "zombie" completion. This isolates the unit under test (the modal's + // videoUploadId versioning guard) from real webtorrent/S3, which is what the + // test intends. Previously the test mocked modal.generateTorrentMetadata / + // resolveUploadIdentifier (the wrong object), so the mediaUploader still ran + // real createTorrentMetadata()/hashing on a synthetic file — throwing "invalid + // input type" as async activity after the test ended. + modal.mediaUploader.uploadVideo = () => delayedUpload; + // captureVideoMetadata() does URL.createObjectURL(file), which needs a real + // Blob; stub it so the synthetic file below doesn't throw an async Blob error. + modal.captureVideoMetadata = async () => {}; // Start upload const file = { name: "video.mp4" }; diff --git a/todo/TODO_2026-06-20_pre-launch.md b/todo/TODO_2026-06-20_pre-launch.md index 1a3b576a..67080b04 100644 --- a/todo/TODO_2026-06-20_pre-launch.md +++ b/todo/TODO_2026-06-20_pre-launch.md @@ -1241,10 +1241,20 @@ Reported 2026-06-25. Relates to #17 (NIP-71 interop) / the bitvid→NIP-71 mirro local patch is guesswork — the update is the honest fix. ### 11. Harden flaky tests (CI gate reliability) -- [ ] `tests/ui/uploadModal-reset.test.mjs` ("UploadModal Reset Logic") intermittently - hangs/cancels (jsdom/webtorrent async-hang flake; documented in KNOWN_ISSUES, - reproduced on pre-refactor `1b11cb1b`). Make it deterministic so it can be a - trusted release gate. Audit e2e parallel-load flakiness too. +- [x] `tests/ui/uploadModal-reset.test.mjs` ("UploadModal Reset Logic") — FIXED 2026-07-05. + The "guard against zombie callbacks" subtest mocked `modal.generateTorrentMetadata` / + `resolveUploadIdentifier`, but handleVideoSelection drives the upload through + `this.mediaUploader.uploadVideo` (a DIFFERENT object with its own helpers) — so the + mediaUploader still ran real `createTorrentMetadata()`/hashing on a synthetic + `{name:"video.mp4"}` file, throwing "invalid input type" as async activity AFTER the + test ended (node:test flags that as an uncaughtException) plus + `captureVideoMetadata()` calling `URL.createObjectURL(file)` → "must be an instance of + Blob". Not intermittent anymore — it was a consistent, wrong-boundary mock. Fix: mock + at the real DI boundary — `modal.mediaUploader.uploadVideo = () => delayedUpload` (the + controllable pending promise) + stub `modal.captureVideoMetadata`. Now hermetic, no + webtorrent/Blob, deterministic (3/3 exit 0, # pass 3). Mutation-verified: breaking the + videoUploadId zombie guard makes the test fail (# fail 1). See TEST_INTEGRITY.md. +- [ ] Audit e2e parallel-load flakiness too. ### 11b. SILENTLY-EXCLUDED unit tests — 20 files never run in CI (found 2026-06-23) > **2026-07-03 addendum — stale-default failure was hiding in local runs:**