[ci] Fix the stale tests that had CI red

CI has been failing on unstable for weeks, across four unit files and a set of
e2e specs. None of it was a product regression: every failure is a test that
encodes behavior a later commit deliberately changed. Each is corrected against
the spec that superseded it, not adjusted until it passed.

Unit:

- video-modal-moderation: the fixture set flat `blurThumbnail`/`trustedMuteCount`,
  but normalizeVideoModerationContext derives shouldShow from
  `original.blurThumbnail`/`trustedCount`, so the assertion ran against a
  context whose early return had just deleted the blur it was checking. 4525c1c6
  also made a lone trusted mute a non-blocking ranking signal (its sibling test
  was renamed to say exactly that; this one was missed). Fixture now mirrors
  what moderationDecorator emits.
- trustBootstrap: asserted a subscription to onWhitelistChange that 18e08bf2
  deliberately removed -- buildTrustedSeeds derives seeds from the super admin,
  editors and fallback seeds, so a whitelist change cannot alter them. The
  removal is now pinned so it is not reinstated by reflex.
- moderation-service: 18e08bf2 removed `status.whitelisted ||` from
  isTrustedReporter (being allowed to publish is not a statement that your
  reports carry weight). A whitelisted reporter now counts toward the report
  total but not toward trusted.
- playbackService_forcedSource: forced P2P now falls back to a hosted URL when
  the torrent definitively fails -- playbackService documents the change and its
  rollback. The old assertion did not merely fail, it HUNG: forced sources skip
  the start timeout, so the now-reachable hosted attempt waited forever on a
  <video> that never emits.

E2E:

- ui-modals: asserted exactly one `.bv-modal` per component page, but
  profile-modal.html also bundles #storageCorsModal (d7ad1516). Requires at
  least one and drives the primary, which is what the test's own injected
  querySelector already used.
- coverage-utility-modules: used `author:abc123`, which 9afbc0cb correctly began
  rejecting as undecodable. Uses a real pubkey for the success path and pins the
  rejection separately.
- webseed-playback: asserted the FINAL src equals the CDN URL while also
  tolerating "No playable source found" in its status check -- contradictory,
  since a failed attempt calls resetVideoElement() and clears src. Headless CI
  cannot decode the archive.org H.264 sample. Now records src attachments and
  asserts the URL was attached, which is the behavior under test.
- nostr-runtime-coverage: asserted a legacy plaintext session-actor entry reads
  back as null. Since 8692dac6 added per-account storage, readV1Entry PURGES the
  plaintext entry and the reader falls back to the sole remaining encrypted
  account -- so null no longer describes the security property. Asserts the
  property directly: no plaintext key survives, in the entry or in raw storage.

Also drops the syncAuthUiState share-gate wiring added in d141c316. It was
redundant: Application.updateShareNostrAuthState() already fires on init, login,
logout, pubkey-change and signer-change and forwards to
videoModal.setShareNostrAuthState(). That call short-circuited on its own
`if (!this.videoModal?.setShareNostrAuthState) return;` guard purely because the
method did not exist -- adding it (d141c316) was the whole fix. The tests now
pin that contract instead of a second, competing code path.

All 416 unit files, all three CI shards, both DM suites and lint are green.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
thePR0M3TH3ANandClaude Opus 5 committed 2026-07-28 17:37:24 -04:00
1 parent 244f527cf6
commit f48150ed6b
10 files changed
+187 -167

No files matched your search

+1 -43
View File
@@ -9,7 +9,6 @@
* Methods use `this` which is bound to the Application instance.
*/
import { SHORT_TIMEOUT_MS, DEBOUNCE_DELAY_MS } from "../constants.js";
import { getActiveSigner } from "../nostrClientRegistry.js";
export function createUiCoordinator(deps) {
const {
@@ -188,52 +187,11 @@ export function createUiCoordinator(deps) {
},
syncAuthUiState() {
// Auth-dependent chrome has to follow login/logout/account switches, but
// syncAuthUiState() was only ever called at bootstrap — so everything it
// gates (notably the share menu's "Share on Nostr" item) kept its
// logged-out state for the entire session. Self-install a listener on the
// first run rather than adding a call site in js/app.js, which is at its
// file-size cap.
if (
!this.__authUiSyncBound &&
typeof window !== "undefined" &&
typeof window?.addEventListener === "function"
) {
this.__authUiSyncBound = true;
window.addEventListener("bitvid:auth-changed", () => {
try {
this.syncAuthUiState();
} catch (error) {
devLogger.warn("[Application] Auth UI resync failed:", error);
}
});
}
const loggedIn = this.isUserLoggedIn();
if (loggedIn) {
if (this.isUserLoggedIn()) {
this.applyAuthenticatedUiState();
} else {
this.applyLoggedOutUiState();
}
// Gate the share menu's "Share on Nostr" item. Without this the modal's
// shareNostrAuthState stays at its initial `false` forever and the item
// renders permanently disabled — indistinguishable from plain text.
if (typeof this.videoModal?.setShareNostrAuthState === "function") {
let signer = null;
try {
signer = getActiveSigner();
} catch (error) {
devLogger.warn("[Application] Failed to read active signer:", error);
}
this.videoModal.setShareNostrAuthState({
isLoggedIn: loggedIn,
// Mirrors ShareNostrController.handleShare(): a signer that cannot
// sign is no signer at all, and enabling the item would only produce
// an error toast on click.
hasSigner: !!signer && typeof signer.signEvent === "function",
});
}
},
refreshChromeElements() {
+17 -4
View File
@@ -13,12 +13,20 @@ test.describe("runtime utility module coverage", () => {
"/js/search/searchFilters.js"
);
// 9afbc0cb ("Decode npub authors to hex") made the author filter validate
// its value, because npub-form authors silently matched nothing. The old
// fixture used `author:abc123`, which is neither a decodable npub nor
// 64-char hex, so the filter is now (correctly) rejected and
// authorPubkeys comes back empty. Use a real pubkey to exercise the
// success path, and pin the rejection separately below.
const AUTHOR_HEX = "a".repeat(64);
const parsed = parseFilterQuery(
'author:abc123 tag:#music,nostr kind:30078 relay:wss://relay.example after:2024-01-02 before:1707000000 duration:>=1.5m has:magnet nsfw:safe "exact phrase" looseTerm',
`author:${AUTHOR_HEX} tag:#music,nostr kind:30078 relay:wss://relay.example after:2024-01-02 before:1707000000 duration:>=1.5m has:magnet nsfw:safe "exact phrase" looseTerm`,
);
const withErrors = parseFilterQuery(
"duration:oops has:bad unknown:value before:not-a-date",
"duration:oops has:bad unknown:value before:not-a-date author:abc123",
);
const serialized = serializeFiltersToQuery(parsed.filters);
@@ -30,7 +38,7 @@ test.describe("runtime utility module coverage", () => {
};
});
expect(result.parsed.filters.authorPubkeys).toEqual(["abc123"]);
expect(result.parsed.filters.authorPubkeys).toEqual(["a".repeat(64)]);
expect(result.parsed.filters.tags).toEqual(["music", "nostr"]);
expect(result.parsed.filters.kind).toBe(30078);
expect(result.parsed.filters.relay).toBe("wss://relay.example");
@@ -44,8 +52,13 @@ test.describe("runtime utility module coverage", () => {
expect(result.withErrors.errors.some((e: any) => /Duration/.test(e.message))).toBe(true);
expect(result.withErrors.errors.some((e: any) => /Has filter/.test(e.message))).toBe(true);
expect(result.withErrors.errors.some((e: any) => /Date value/.test(e.message))).toBe(true);
// An undecodable author must be reported rather than silently dropped.
expect(
result.withErrors.errors.some((e: any) => /Could not decode author/.test(e.message)),
).toBe(true);
expect(result.withErrors.filters.authorPubkeys).toEqual([]);
expect(result.serialized).toContain("author:abc123");
expect(result.serialized).toContain(`author:${"a".repeat(64)}`);
expect(result.serialized).toContain("tag:music");
expect(result.serialized).toContain("kind:30078");
expect(result.serialized).toContain("has:magnet");
+20 -2
View File
@@ -491,7 +491,21 @@ test.describe("nostr runtime coverage (deterministic)", () => {
createdAt: 42,
}),
);
// A legacy plaintext entry must be PURGED, not honored. readV1Entry()
// removes it from storage; readStoredSessionActorEntry() then falls back
// to the sole remaining (encrypted) account, so the old `=== null`
// assertion no longer describes the security property -- it just happened
// to hold before 8692dac6 added per-account storage. Assert the property
// directly instead: no plaintext key survives, in the returned entry or
// in raw storage.
const migratedLegacyEntry = readStoredSessionActorEntry();
const legacyRaw = localStorage.getItem(SESSION_ACTOR_STORAGE_KEY);
let legacyRawPlaintextSurvives = false;
try {
legacyRawPlaintextSurvives = Boolean(legacyRaw && JSON.parse(legacyRaw)?.privateKey);
} catch (error) {
legacyRawPlaintextSurvives = false;
}
const actorMatch = isSessionActor({
pubkey: actor,
@@ -524,7 +538,10 @@ test.describe("nostr runtime coverage (deterministic)", () => {
wrongPassphraseErrorCode,
metadataValid: metadata && metadata.salt === encrypted.salt,
metadataInvalidIsNull: invalidMetadata === null,
migratedLegacyEntryIsNull: migratedLegacyEntry === null,
migratedLegacyEntryHasPlaintextKey:
typeof (migratedLegacyEntry as any)?.privateKey === "string" &&
(migratedLegacyEntry as any).privateKey.length > 0,
legacyRawPlaintextSurvives,
actorMatch,
actorMismatch,
nsecActor,
@@ -541,7 +558,8 @@ test.describe("nostr runtime coverage (deterministic)", () => {
expect(result.wrongPassphraseErrorCode).toBe("decrypt-failed");
expect(result.metadataValid).toBeTruthy();
expect(result.metadataInvalidIsNull).toBe(true);
expect(result.migratedLegacyEntryIsNull).toBe(true);
expect(result.migratedLegacyEntryHasPlaintextKey).toBe(false);
expect(result.legacyRawPlaintextSurvives).toBe(false);
expect(result.actorMatch).toBe(true);
expect(result.actorMismatch).toBe(false);
expect(result.nsecActor).toBe(false);
+10 -2
View File
@@ -211,8 +211,16 @@ test.describe("component modal pages", () => {
waitUntil: "networkidle",
});
const modal = page.locator(".bv-modal");
await expect(modal).toHaveCount(1);
// A component page may legitimately ship more than one modal root:
// profile-modal.html also bundles #storageCorsModal, the B2/S3 CORS setup
// helper added in d7ad1516. Asserting exactly one was incidental to what
// this test is actually for -- "the page renders a modal that opens and
// closes" -- so require at least one and drive the primary one, which is
// what the injected script's document.querySelector(".bv-modal") already
// operates on.
const modals = page.locator(".bv-modal");
expect(await modals.count()).toBeGreaterThan(0);
const modal = modals.first();
// Inject logic to initialize the modal since we are viewing a partial/component in isolation.
await page.addScriptTag({
+29 -3
View File
@@ -29,6 +29,31 @@ test.describe("Webseed Stream Playback", () => {
});
test("plays directly from HTML link (CDN mode)", async ({ page }) => {
// Record every src the player attaches. Asserting the FINAL src is wrong:
// this test already tolerates "No playable source found" in its status
// check (headless CI often cannot decode the archive.org H.264 sample), and
// when the attempt fails resetVideoElement() clears src back to "". The
// behaviour under test is that CDN mode ATTACHES the hosted URL, which is
// observable regardless of whether the remote file then decodes.
await page.evaluate(() => {
(window as any).__attachedSrcs = [];
new MutationObserver((mutations) => {
for (const mutation of mutations) {
const target = mutation.target as HTMLElement;
if (mutation.attributeName === "src" && target.tagName === "VIDEO") {
const value = target.getAttribute("src");
if (value) {
(window as any).__attachedSrcs.push(value);
}
}
}
}).observe(document.body, {
subtree: true,
attributes: true,
attributeFilter: ["src"],
});
});
// 1. Trigger playback with just the URL
await page.evaluate(async (url) => {
const { getApplication } = await import("/js/applicationContext.js");
@@ -55,9 +80,10 @@ test.describe("Webseed Stream Playback", () => {
const urlToggle = page.locator('[data-source-toggle="url"]');
await expect(urlToggle).toHaveAttribute("aria-pressed", "true");
// Check if video is attempting to play from source
const video = page.locator("#modalVideo");
await expect(video).toHaveAttribute("src", VIDEO_URL);
// Check the player attached the hosted URL (see the observer above).
await expect
.poll(async () => await page.evaluate(() => (window as any).__attachedSrcs))
.toContain(VIDEO_URL);
});
test("falls back to WebTorrent using HTML link as webseed when CDN fails", async ({ page }) => {
+20 -6
View File
@@ -92,18 +92,32 @@ test("trusted report summaries respect personal blocks and admin lists", async (
service.recomputeSummaryForEvent(eventId);
// 18e08bf2 ("refine trust controls") removed `status.whitelisted ||` from
// isTrustedReporter and updated the doc comment from "trustedContacts set (or
// Admin Whitelist)" to "trustedContacts set". Being on the admin whitelist is
// permission to publish, not a statement that your reports carry weight, so a
// whitelisted reporter now counts toward the report TOTAL but not toward
// TRUSTED.
//
// Counted reporters: trusted (t=100), whitelisted (t=400), random (t=300).
// Excluded entirely: blacklisted (admin list) and blocked (personal block).
const summary = service.getTrustedReportSummary(eventId);
assert.equal(summary.totalTrusted, 2);
assert.equal(summary.types.nudity.total, 3);
assert.equal(summary.types.nudity.trusted, 2);
assert.equal(summary.types.nudity.latest, 400);
assert.equal(summary.totalTrusted, 1, "only the trusted contact is trusted");
assert.equal(summary.types.nudity.total, 3, "whitelisted still counts as a report");
assert.equal(summary.types.nudity.trusted, 1);
assert.equal(
summary.types.nudity.latest,
400,
"latest still reflects the whitelisted reporter's timestamp"
);
const reporters = service.getTrustedReporters(eventId, "nudity");
assert.deepEqual(
reporters.map((entry) => entry.pubkey),
[whitelistedHex, trustedHex]
[trustedHex],
"the whitelisted reporter is no longer surfaced as a trusted reporter"
);
assert.equal(service.trustedReportCount(eventId, "nudity"), 2);
assert.equal(service.trustedReportCount(eventId, "nudity"), 1);
});
test("user block updates recompute summaries and emit notifications", async (t) => {
@@ -267,8 +267,18 @@ describe("PlaybackService Forced Source Logic", () => {
assert.equal(probeUrl.mock.callCount(), 0);
});
test("Forced Source 'torrent': Does NOT fallback to URL even if Torrent fails", async () => {
const probeUrl = mock.fn(async () => ({ outcome: "good" }));
test("Forced Source 'torrent': falls back to the hosted URL when the torrent definitively fails", async () => {
// Spec changed deliberately in playbackService.js: "Even when P2P was
// explicitly forced, a hosted URL is a better outcome than a hard 'No
// playable source found' if the torrent DEFINITIVELY failed." The code
// carries the rollback note (`restore && forcedSource !== "torrent"` to
// make forced P2P hard-fail), so the old assertion here -- source null,
// probe never called -- encodes behavior that was intentionally removed.
//
// It did not merely fail: because forced sources skip the start timeout,
// the now-reachable hosted attempt waited forever on a <video> that never
// emits, and the test hung ("Promise resolution is still pending").
const probeUrl = mock.fn(async () => ({ outcome: "good", status: 200 }));
const playViaWebTorrent = mock.fn(async () => {
throw new Error("Torrent failed");
@@ -283,9 +293,18 @@ describe("PlaybackService Forced Source Logic", () => {
forcedSource: "torrent",
});
const result = await session.start();
const startPromise = session.start();
assert.equal(result.source, null);
assert.equal(probeUrl.mock.callCount(), 0);
// Let the torrent attempt reject and the hosted fallback attach, then
// signal that the element actually started playing.
await new Promise(resolve => process.nextTick(resolve));
mock.timers.tick(10);
await new Promise(resolve => process.nextTick(resolve));
video.dispatchEvent(new window.Event("playing"));
const result = await startPromise;
assert.equal(playViaWebTorrent.mock.callCount(), 1, "P2P is still tried first");
assert.equal(result.source, "url", "a working CDN beats a hard failure");
});
});
+12 -2
View File
@@ -133,14 +133,24 @@ describe("trustBootstrap", () => {
assert.ok(seeds.has("editor1"));
});
it("should subscribe to accessControl changes", async () => {
it("should subscribe to the accessControl lists that affect seeds", async () => {
await bootstrapTrustedSeeds();
assert.equal(accessControl.onWhitelistChange.mock.callCount(), 1);
assert.equal(accessControl.onEditorsChange.mock.callCount(), 1);
assert.equal(accessControl.onBlacklistChange.mock.callCount(), 1);
});
it("should NOT subscribe to whitelist changes", async () => {
// buildTrustedSeeds() derives seeds from the super admin, the editors list
// and the fallback seeds -- the whitelist is not an input, so re-applying
// seeds on a whitelist change is pure churn. 18e08bf2 ("refine trust
// controls") removed that subscription deliberately; this pins the removal
// so it is not reinstated by reflex.
await bootstrapTrustedSeeds();
assert.equal(accessControl.onWhitelistChange.mock.callCount(), 0);
});
it("should handle accessControl timeout and apply seeds anyway", async () => {
// Mock waitForAccessControl to timeout (reject)
accessControl.waitForReady.mock.mockImplementation(async () => {
+35 -100
View File
@@ -13,13 +13,7 @@
import test from "node:test";
import assert from "node:assert/strict";
import { createUiCoordinator } from "../js/app/uiCoordinator.js";
import { VideoModal } from "../js/ui/components/VideoModal.js";
import {
clearActiveSigner,
registerSigner,
setActiveSigner,
} from "../js/nostrClientRegistry.js";
const PUBKEY = "b".repeat(64);
@@ -71,103 +65,44 @@ test("setShareNostrAuthState tolerates no popover", () => {
);
});
// --- syncAuthUiState wiring ----------------------------------------------
// --- app wiring ----------------------------------------------------------
// bitvid ALREADY routed auth changes to the modal: Application
// .updateShareNostrAuthState() is called on init, login, logout, pubkey-change
// and signer-change, and it forwards to videoModal.setShareNostrAuthState().
// That method simply did not exist, so the call short-circuited on its own
// `if (!this.videoModal?.setShareNostrAuthState) return;` guard and the gate
// stayed shut forever. These pin the contract that guard depends on.
function makeApp({ loggedIn }) {
const coordinator = createUiCoordinator({
devLogger: { warn() {}, error() {}, log() {} },
});
const pushed = [];
const app = {
...coordinator,
isUserLoggedIn: () => loggedIn,
// Stub the DOM-touching branches; this test is about the share gate.
applyAuthenticatedUiState() {},
applyLoggedOutUiState() {},
videoModal: {
setShareNostrAuthState(state) {
pushed.push({ ...state });
},
},
};
return { app, pushed };
}
test("the app's updateShareNostrAuthState contract is satisfied", async () => {
const { readFile } = await import("node:fs/promises");
const source = await readFile(new URL("../js/app.js", import.meta.url), "utf8");
test("syncAuthUiState pushes the gate into the video modal", () => {
const { app, pushed } = makeApp({ loggedIn: true });
app.syncAuthUiState();
assert.equal(pushed.length, 1);
assert.equal(pushed[0].isLoggedIn, true);
});
// The guard short-circuits when the method is absent, which is exactly how
// this stayed broken silently. If the method is ever renamed, fail loudly.
assert.match(source, /videoModal\?\.setShareNostrAuthState/);
assert.match(source, /videoModal\.setShareNostrAuthState\(\{/);
test("logged out propagates as a closed gate", () => {
const { app, pushed } = makeApp({ loggedIn: false });
app.syncAuthUiState();
assert.deepEqual(pushed[0], { isLoggedIn: false, hasSigner: false });
});
test("hasSigner reflects a registered, signing-capable signer", () => {
clearActiveSigner();
const { app, pushed } = makeApp({ loggedIn: true });
app.syncAuthUiState();
assert.equal(pushed[0].hasSigner, false, "no signer registered yet");
registerSigner(PUBKEY, { pubkey: PUBKEY, signEvent: async (e) => e });
setActiveSigner(PUBKEY);
app.syncAuthUiState();
assert.equal(pushed[1].hasSigner, true);
clearActiveSigner();
app.syncAuthUiState();
assert.equal(pushed[2].hasSigner, false, "logout must close the gate again");
});
test("a signer that cannot sign does not open the gate", () => {
// Mirrors ShareNostrController.handleShare(): enabling the item for a signer
// without signEvent would only produce an error toast on click.
clearActiveSigner();
registerSigner(PUBKEY, { pubkey: PUBKEY });
setActiveSigner(PUBKEY);
const { app, pushed } = makeApp({ loggedIn: true });
app.syncAuthUiState();
assert.equal(pushed[0].hasSigner, false);
clearActiveSigner();
});
test("syncAuthUiState re-runs on bitvid:auth-changed, and binds only once", () => {
// The original bug's second half: the gate was computed once at bootstrap and
// never again, so logging in mid-session left the button disabled.
const listeners = [];
const originalWindow = globalThis.window;
globalThis.window = {
addEventListener(type, handler) {
listeners.push({ type, handler });
},
};
try {
const { app, pushed } = makeApp({ loggedIn: false });
app.syncAuthUiState();
assert.equal(pushed.length, 1);
const bound = listeners.filter((l) => l.type === "bitvid:auth-changed");
assert.equal(bound.length, 1, "listener installed on first sync");
bound[0].handler();
assert.equal(pushed.length, 2, "auth change re-syncs the gate");
// Re-entrancy: the resync must not stack a new listener every time.
assert.equal(
listeners.filter((l) => l.type === "bitvid:auth-changed").length,
1
// The lifecycle points that must keep the gate in sync.
for (const reason of ["init", "signer-change", "pubkey-change"]) {
assert.ok(
source.includes(`reason: "${reason}"`),
`app.js should refresh the share gate on ${reason}`
);
} finally {
if (originalWindow === undefined) {
delete globalThis.window;
} else {
globalThis.window = originalWindow;
}
}
assert.equal(
typeof VideoModal.prototype.setShareNostrAuthState,
"function",
"VideoModal must expose the method app.js calls"
);
});
test("login and logout both refresh the gate", async () => {
const { readFile } = await import("node:fs/promises");
const source = await readFile(
new URL("../js/app/authSessionCoordinator.js", import.meta.url),
"utf8"
);
assert.ok(source.includes('reason: "auth-login"'));
assert.ok(source.includes('reason: "auth-logout"'));
});
+19
View File
@@ -125,14 +125,33 @@ test("VideoModal blurs and restores playback when moderation overlay toggles", a
});
await t.test("trusted mute blur state matches moderation context", async () => {
// Fixture mirrors what moderationDecorator actually emits. Two things
// made the old hand-rolled shape stop exercising this path:
//
// 1. normalizeVideoModerationContext derives `shouldShow` from
// `original.blurThumbnail` and `trustedCount`, NOT from a flat
// `blurThumbnail`/`trustedMuteCount`. The old fixture set fields the
// helper never reads, so shouldShow was false and applyModerationOverlay
// took its early return and deleted the blur it was asserting on.
// 2. 4525c1c6 made a lone trusted mute a non-blocking ranking signal (see
// "VideoCard keeps a single trusted mute as a non-blocking ranking
// signal"), so trusted-mute alone no longer forces the overlay.
//
// The scenario under test is still "a trusted mute that DID trigger a
// blur renders blurred with a trusted-mute badge", so the fixture now
// carries the decorator's `original` block that represents exactly that.
const video = {
id: "event-trusted-mute",
title: "Trusted mute video",
moderation: {
blurThumbnail: true,
blurReason: "trusted-mute",
trustedMuted: true,
trustedCount: 2,
trustedMuteCount: 2,
trustedMuterDisplayNames: ["Carol", "Dave"],
trustedMuteDisplayNames: ["Carol", "Dave"],
original: { blurThumbnail: true, blurReason: "trusted-mute" },
},
};