mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
tests(11): make uploadModal-reset zombie-callback test deterministic (correct mock boundary)
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 <[email protected]>
This commit is contained in:
1 parent
a326d04330
commit
567a12c758
3 files changed
+68
-16
No files matched your search
@@ -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
|
||||
|
||||
@@ -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" };
|
||||
|
||||
@@ -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:**
|
||||
|
||||
Reference in new issue
Block a user