[Test] Five test cases assert `expect(true).toBe(true)` in place of the behaviour their title names and `CircuitBreaker.test.ts` discards the thrown error at eight sites, so a wrong error type or a silent no-op passes

[pathosDev]

Problem

Five test cases in the suite assert expect(true).toBe(true) as their only statement about the behaviour named in their title. Each is counted in the 3972 of 3972 badge, each is green, and none of them can distinguish the framework working from the framework not working.

The clearest one does not even claim otherwise:

tests/unit/worker/WorkerNode.test.ts:63-71
  test('throws when not running inside a Worker (no self scope)', async () => {
    // Strip `self` so the join() detects "not a Worker".  We have to
    // also strip `globalThis` view of postMessage etc — but
    // WorkerNode.join's check is `if (!selfScope)` which only fires
    // when both `g.self` AND `g` itself fail.  In Node/Bun's main
    // thread `globalThis` always has SOME scope, so the check is
    // best-effort.  Skipping this edge — see commented test below.
    expect(true).toBe(true);
  });

The comment says the edge is being skipped. The mechanism for that is test.skip, which reports as a skip and shows up as a known gap. Instead the case reports as a pass and shows up as coverage of WorkerNode.join's not-a-worker guard, which it is not.

The other four have the same shape: a scenario is set up, the framework is poked, and then the assertion is a tautology standing in for the observation that was not made. In three of them the title names a property — "does not propagate", "silent no-op", "logs a warning but does not throw" — that is asserted by nothing.

A second, softer version of the same problem: tests/unit/pattern/CircuitBreaker.test.ts drives the breaker into failure at eight sites, and at every one of them discards the thrown error with catch { /* */ } before inspecting cb.state. CircuitBreaker distinguishes CircuitBreakerOpenError from CircuitBreakerTimeoutError from the caller's own error, and two tests in the file do assert which one came back — so the type matters and the file knows it. At the other eight sites, a breaker that wrapped a user error in the wrong class, or swallowed it and returned undefined, would still leave cb.state correct and the test green.

Evidence

Mechanically, over tests/**/*.test.ts with comments stripped and each test(…)/it(…) call brace-matched:

files scanned .................................... 319
test cases parsed ................................ 3411
cases whose body contains no `expect(` ...........   66
cases asserting expect(true).toBe(true)/toBeTrue()    5

The 66 are a screen, not a verdict — most of them assert through helpers that throw (probe.receiveOne, probe.expectMessage, awaitCondition, waitFor), which is a legitimate assertion form. The five below are not.

1. tests/unit/worker/WorkerNode.test.ts:63 — quoted in full above.

2. The scheduler's error containment:

tests/unit/Scheduler.test.ts:51-62
  test('exceptions in the callback do not propagate', async () => {
    const originalError = console.error;
    console.error = () => {};
    try {
      const scheduler = new Scheduler();
      scheduler.scheduleOnceFunction(10, () => { throw new Error('boom'); });
      await sleep(30);
      expect(true).toBe(true);
    } finally {
      console.error = originalError;
    }
  });

console.error is stubbed out at line 53, so the one observable the containment produces is discarded before it can be checked. Nothing asserts the scheduler kept running, and nothing asserts anything was logged.

3. The FSM's unknown-state path:

tests/unit/fsm/FSM.test.ts:72-86
  test('unknown state logs a warning but does not throw', async () => {
    class Broken extends FSM<'a' | 'b', null, string> {
      constructor() { super('a', null); /* no handlers registered */ }
    }
    const kitOptions = TestKitOptions.create()
      .withLogger(new NoopLogger())
      .withLogLevel(LogLevel.Off);
    const kit = TestKit.create('fsm-missing', kitOptions);
    const ref = kit.system.spawnAnonymous(Broken);
    ref.tell('anything');
    await Bun.sleep(20);
    // The actor is still alive — subsequent tells don't throw.
    expect(true).toBe(true);
    await kit.system.terminate();
  });

The logger is a NoopLogger at LogLevel.Off, so the "logs a warning" half is unobservable by construction. The "does not throw" half is unobservable too: a throw inside onReceive is caught by the cell and routed to supervision, so it would never surface as a test failure. The comment asserts the actor is still alive; no code checks it.

4. The JetStream unknown-sequence acknowledgment:

tests/integration/in-process/io/broker/JetStreamActor.test.ts:452-471
  test('ack for unknown streamSeq is a silent no-op', async () => {
    const sysOptions = ActorSystemOptions.create()
      .withLogger(new NoopLogger())
      .withLogLevel(LogLevel.Off);
    const sys = ActorSystem.create('js-unknown', sysOptions);
    try {
      const jetstreamOptions = JetStreamOptions.create()
        .withServers(['nats://fake:4222'])
        .withStream({ name: 'S', subjects: ['s.>'] })
        .withConsumer({ durable: 'd' });
      const { actor } = await bootActor(sys, jetstreamOptions);
      // No handle pushed, so no pending entry.  Sending ack should not throw.
      actor.tell({ kind: 'acknowledgment', streamSeq: 999 });
      await sleep(20);
      // Test passes if we get here without unhandled rejection.
      expect(true).toBe(true);
    } finally {
      await sys.terminate();
    }
  });

"Silent" is not checked, and "no-op" is not checked — an implementation that acked an unrelated pending entry would pass.

5. The post-migration replay:

tests/integration/in-process/persistence/migration/wrapLegacy.test.ts:147-161
    try {
      const ref = sys.spawn(Account, 'acct');
      // Send a no-op message so we can wait for recovery to complete.
      // (PersistentActor processes the recovery before the first user
      // message lands.)
      const reply = await ref.ask<{ balance: number; currency: string }>({ kind: 'snapshot' }, 1_000,).catch(() => null);
      void reply;
      // Direct state read via internal API isn't exposed; instead we
      // rely on the journal having been recovered without throwing.
      // The presence of a successful spawn (no MigrationError) is
      // already the assertion this test cares about.
      expect(true).toBe(true);
    } finally {
      await sys.terminate();
    }

This one does have a real assertion earlier — expect(result.wrapped).toBe(2) at line 123 pins the migration count — so unlike the other four the case is not entirely inert. But the half the title promises, "an actor with a defaultsAdapter can replay the journal", is asserted by nothing: the ask is .catch(() => null), the reply is discarded with void reply, and a recovery failure would be routed to supervision rather than thrown. The comment's premise — "the presence of a successful spawn (no MigrationError)" — is not what the code observes, because sys.spawn returns before recovery has run. The expected state is right there ({ balance: 130, currency: 'USD' }) and is never compared against.

The CircuitBreaker sites. Eight discards, from grep -n "catch {" tests/unit/pattern/CircuitBreaker.test.ts:

tests/unit/pattern/CircuitBreaker.test.ts:21-56
  test('opens after maxFailures consecutive failures', async () => {
    const cb = new CircuitBreaker({ maxFailures: 2, resetTimeoutMs: 1_000 });
    for (let i = 0; i < 2; i++) {
      try { await cb.call(async () => { throw new Error('boom'); }); }
      catch { /* expected */ }
    }
    expect(cb.state).toBe('open');
  });

  test('open breaker rejects immediately with CircuitBreakerOpenError', async () => {
    const cb = new CircuitBreaker({ maxFailures: 1, resetTimeoutMs: 1_000 });
    try { await cb.call(async () => { throw new Error('x'); }); } catch { /* */ }
    let caught: unknown = null;
    try { await cb.call(async () => 'never called'); } catch (e) { caught = e; }
    expect(caught).toBeInstanceOf(CircuitBreakerOpenError);
  });

  test('half-opens after resetTimeoutMs; a success closes it', async () => {
    const cb = new CircuitBreaker({ maxFailures: 1, resetTimeoutMs: 40 });
    try { await cb.call(async () => { throw new Error('x'); }); } catch { /* */ }
    expect(cb.state).toBe('open');

    await sleep(60);
    // First call after reset should move to half-open as part of .call().
    const value = await cb.call(async () => 'ok');
    expect(value).toBe('ok');
    expect(cb.state).toBe('closed');
  });

  test('half-open failure re-opens the breaker', async () => {
    const cb = new CircuitBreaker({ maxFailures: 1, resetTimeoutMs: 30 });
    try { await cb.call(async () => { throw new Error('x'); }); } catch { /* */ }
    await sleep(50);
    try { await cb.call(async () => { throw new Error('still flaky'); }); } catch { /* */ }
    expect(cb.state).toBe('open');
  });

plus lines 76, 85 and 101 in the same shape. The contrast at line 34 is the point: when the file cares which error came out it binds it and asserts on it, and the pattern is available everywhere.

A representative case from the 66 whose defect is the same in kind:

tests/integration/in-process/cluster/sharding/ShardedDaemonProcess.test.ts:134
  test('handle.stop() cancels the heartbeat without leaking timers', async () => {

Its body drains two preStart messages through probe.receiveOne(1_000) (a real assertion — it throws on timeout), then calls handle.stop() twice and tears the node down. Nothing observes a timer, leaked or cancelled. If stop() were a no-op the test would pass.

Proposal

• Replace all five tautologies with the observation the title names. WorkerNode.test.ts:63 becomes test.skip with the reason in the title, or is deleted; Scheduler.test.ts:51 captures console.error instead of silencing it, asserts it was called, and asserts a later scheduled task still fires; FSM.test.ts:72 uses a recording logger rather than NoopLogger and asserts the actor answers a subsequent message; JetStreamActor.test.ts:452 asserts no pending entry was consumed and no ack was sent to the fake client; wrapLegacy.test.ts:108 asserts the recovered state equals { balance: 130, currency: 'USD' } and drops the .catch(() => null).
• Bind the error in CircuitBreaker.test.ts. Eight catch { /* */ } become catch (e) { caught = e } with an assertion that it is the caller's own error and not a breaker type. This is a mechanical edit and it is what makes the file able to detect an error-wrapping regression.
• Add a lint-shaped test. Fail the suite when a test(…) body's only assertion is a literal-versus-itself comparison (expect(true).toBe(true), expect(1).toBe(1), expect(x).toBe(x)). The repository already carries this kind of meta-test — tests/unit/config/NoDeadConfigKeys.test.ts asserts a property of the source tree rather than of the runtime — so the mechanism exists and the precedent is set.
• Do not turn "body contains no expect(" into a gate. Two thirds of the 66 assert through throwing helpers and are correct; a rule against them would push tests toward worse shapes.

Acceptance sketch

• No test case in tests/ asserts expect(true).toBe(true) as its statement about the titled behaviour.
• WorkerNode.test.ts:63 either asserts the guard or is a test.skip naming why.
• Every catch in tests/unit/pattern/CircuitBreaker.test.ts binds the error and asserts its type.
• wrapLegacy.test.ts:108 asserts the recovered balance and currency.
• A meta-test fails when a tautological assertion is introduced.

Verification status

Found in the ten-lens production-readiness review of 2026-08-05 (v0.13.0) and re-verified before filing: confirmed by reading, with the original counts corrected. The counts here come from a throwaway parser that strips comments and string literals, then brace-matches each test(/it( call — 3 411 cases across 319 files, 66 without expect(, 5 tautological, 3 calls it could not parse (dynamically-titled test.each). The review's figures were 3 407 / 59 / 5 and the CircuitBreaker discard count was 6; the tautology count matches exactly, the other three are close and the higher numbers are used above. One sub-claim is refuted: the review said wrapLegacy.test.ts "cannot fail". It can — expect(result.wrapped).toBe(2) at line 123 is a real assertion. What cannot fail is the replay half its title is about, and that is what is filed here. The parser was deleted; the tree is unmodified.

Adjacent: #418 replaces sleep-based waits with a shared awaitCondition — a different test-quality axis (timing, not assertion strength), and several of the cases above use awaitCondition correctly already. #541 and #538 concern what CI measures and runs. Nothing open covers assertion strength.

Part of the production-readiness review batch — tracked in #913.