Skip to content

fix(cli): align test/ error messages with 4-ingredient strategy#1259

Open
John-David Dalton (jdalton) wants to merge 2 commits intomainfrom
jdd/error-msg-test
Open

fix(cli): align test/ error messages with 4-ingredient strategy#1259
John-David Dalton (jdalton) wants to merge 2 commits intomainfrom
jdd/error-msg-test

Conversation

@jdalton
Copy link
Copy Markdown
Contributor

@jdalton John-David Dalton (jdalton) commented Apr 22, 2026

Summary

PR 6 of the error-message series. Covers test utilities: packages/cli/src/test/ (the Socket-JSON contract validator + auth-flow mocks, which are shared across the test suite) and packages/cli/test/ (the bash smoke harness + its JS counterpart).

~16 messages across 4 files. Full test suite (343 files / 5225 tests) still passes.

What's fixed

src/test/json-output-validation.mts (6 throws)

Socket CLI guarantees a JSON output contract: {ok: true, data: ...} on success or {ok: false, message: ...} on failure. This file's validator is the guardrail. Before, it said things like "JSON output missing required 'ok' boolean field" which didn't tell a failing test author what the full contract looked like or how to satisfy it.

Before: JSON output 'ok' should be true when exit code is 0: {full payload...}
After: Socket JSON contract violation: exit code is 0 but "ok" is false (expected true); got: {preview}... — set ok:true when the command succeeds, or return a non-zero exit code

Long payloads are now truncated to 200 chars in the message so stacktraces stay readable.

src/test/mocks/socket-auth.mts (2 throws)

throw new Error('Authentication failed') and throw new Error('OAuth timeout') — before, a test failure here looked like a real auth outage. Now both errors explicitly identify themselves as test fixtures and point at the config flag that controls them.

test/json-output-validation.mts (2 non-throwing returns)

This is a lighter shim that returns {ok: false, message: ...} instead of throwing. Updated to match the throw-path messages so a failing integration test shows the parse error and a stdout preview.

test/smoke.sh (6 label strings)

The bash smoke harness duplicates the JS validator's logic in shell. Updated its labels to mirror the new wording so the two harnesses produce consistent errors.

Tests

Nothing else needed updating — grep confirmed the "Authentication failed" and "Unknown error" hits elsewhere are unrelated (tests constructing their own new AuthError('Authentication failed') as test data, or matching on a distinct production error in src/utils/socket/api.mts).

Full suite: 5225/5225 tests pass.

Test plan

  • CI green
  • Sanity: break the JSON output of any --json command and verify the new validator message points at the contract violation clearly

Note

Low Risk
Low risk: changes are confined to test utilities and smoke harness messaging, with no production logic impact aside from clearer failures when tests detect malformed JSON or misconfigured mocks.

Overview
Tightens and standardizes test-time validation of the Socket CLI --json output contract by upgrading error messages to explicitly call out contract violations, include actionable guidance, and show a truncated stdout preview for readability.

Updates the auth-flow test mocks to throw fixture-specific failures (instead of generic auth errors), and aligns both the JS integration validator and test/smoke.sh labels with the same clearer parse/contract diagnostics.

Reviewed by Cursor Bugbot for commit a8efd4f. Configure here.

Rewrites the Socket-JSON contract validator and auth-flow mocks under
packages/cli/src/test/ and packages/cli/test/ to follow the What /
Where / Saw vs. wanted / Fix strategy from CLAUDE.md.

Sources:
- src/test/json-output-validation.mts (6 throws): each violation now
  spells out the full Socket-JSON contract, the received value, and
  a concrete fix ("add ok:true", "return empty object", etc.).
  Long output payloads are truncated to 200 chars in the message so
  errors stay readable.
- src/test/mocks/socket-auth.mts (2 throws): "Authentication failed"
  and "OAuth timeout" now call out that they come from a test
  fixture and point at the configuration flag to change.
- test/json-output-validation.mts (2 non-throwing returns): message
  values now include the exit code / parse error and a stdout
  preview so failing tests diagnose themselves.
- test/smoke.sh (6 labels): updated to mirror the TS validator so
  the bash and JS harnesses produce the same wording.

Tests: full suite (343 files / 5225 tests) still passes. No
assertions touched — the unrelated "Authentication failed" hits
in other tests are test fixtures constructing their own Errors,
not references to the mock.

Follows strategy from #1254. Continues #1255-#1258.
Switch `(e as Error).message` to `e instanceof Error ? e.message : String(e)` so that when a non-Error value is thrown (strings, objects, null) the error message stays informative instead of becoming 'undefined'.

Same fix as applied to #1260 (iocraft.mts) after Cursor bugbot flagged the pattern on that PR.
@jdalton
Copy link
Copy Markdown
Contributor Author

bugbot run

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit a8efd4f. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant