Skip to content

test(machine): rewrite State.spec.ts β€” drop smoke tests, strengthen withOverrodeHaltState#131

Merged
mellonis merged 1 commit into
masterfrom
test/rewrite-state
May 9, 2026
Merged

test(machine): rewrite State.spec.ts β€” drop smoke tests, strengthen withOverrodeHaltState#131
mellonis merged 1 commit into
masterfrom
test/rewrite-state

Conversation

@mellonis
Copy link
Copy Markdown
Owner

@mellonis mellonis commented May 9, 2026

Task 7/10. Two audit categories: drop smoke tests, strengthen `withOverrodeHaltState`.

Smoke tests dropped

  • `'State'` β€” `expect(new State()).toBeTruthy()`.
  • `'has id'` β€” `expect(...).toBeDefined()`.
  • `'getSymbol exists'`, `'getCommand exists'`, `'getNextState exists'` β€” all `toBeTruthy` smoke checks.
  • Several redundant `expect(state).toBeTruthy()` lines next to behavior assertions in the constructor happy-path tests.

Method existence is already asserted by every test that calls these methods or constructs a State β€” these were nominal checks that didn't add coverage.

withOverrodeHaltState strengthening

Audit caught: the previous test only asserted the wrapper's name pattern. Five focused tests now pin the actual contract:

  • Wrapper exposes the override target on `.overrodeHaltState`; original is unchanged.
  • Wrapper proxies `getCommand` and `getNextState` to the original's symbol-to-data map.
  • Wrapper shares `debugRef` with the original (v4 ref-share contract β€” assignment on either visible from both).
  • Wrapper has its own `id` distinct from the original.
  • Wrapper name encodes the override target ("${original.name}>${override.name}").

Numbers

  • Test count: 21 β†’ 18 (net -3, but coverage of the actual contract is significantly stronger).
  • Total: 381 tests pass.
  • Coverage: unchanged.

…ithOverrodeHaltState

Audit findings addressed:

1. **Drop method-existence smoke tests.** Removed:
   - \`'has id'\` (\`expect(...).toBeDefined()\`)
   - \`'getSymbol exists'\`, \`'getCommand exists'\`, \`'getNextState exists'\`
     (all \`toBeTruthy\` smoke tests)
   - \`'State'\` (just \`expect(new State()).toBeTruthy()\`)
   Method existence is already asserted by every test that calls these
   methods or constructs a State.

2. **Drop redundant \`.toBeTruthy()\` lines next to behavior assertions.**
   The constructor happy-path tests had \`expect(state).toBeTruthy()\`
   followed by behavior checks β€” if construction failed, the next
   assertion would throw anyway.

3. **Strengthen \`withOverrodeHaltState\`.** Audit-flagged: the previous
   test only asserted the wrapper's name pattern. The wrapping contract
   has more facets β€” five focused tests now pin:
   - The wrapper exposes the override target on \`.overrodeHaltState\`,
     and the original is left unchanged.
   - The wrapper proxies \`getCommand\` and \`getNextState\` to the
     original's symbolToDataMap (verifies the \`#symbolToDataMap\` share).
   - The wrapper shares debugRef with the original (assignment on
     either is visible from both β€” pins the v4 ref-share contract).
   - The wrapper has its own id distinct from the original.
   - The wrapper name encodes the override target (was the only
     pre-rewrite check; kept).

Test count: 21 β†’ 18 (net -3, but coverage of the actual contract is
significantly stronger). Total: 381 tests pass.
@mellonis mellonis merged commit 66fc514 into master May 9, 2026
5 checks passed
@mellonis mellonis deleted the test/rewrite-state branch May 9, 2026 18:10
@github-project-automation github-project-automation Bot moved this from Todo to Done in @mellonis's machines May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant