fix(tests): allow console.error in malformed JSON security tests#36
Conversation
The Malformed JSON, Payload Size, and Content-Type Edge Case tests intentionally send invalid input to Express, which logs to console.error via its built-in parser. The global console.error spy in test/setup/dom.ts treated these expected logs as failures. Use the existing __ALLOW_CONSOLE_ERROR__ escape hatch in the affected describe blocks. Resolves FRE-9. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87bdcb97b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| beforeEach(() => { | ||
| ;(globalThis as any).__ALLOW_CONSOLE_ERROR__ = true |
There was a problem hiding this comment.
Setting __ALLOW_CONSOLE_ERROR__ in a beforeEach for the entire "Payload Size Limits" suite disables the global unexpected-error guard for all tests in that block, not just the oversized-body case that legitimately triggers Express parser logging. As a result, tests like "accepts payload just under 1MB" can now emit unrelated console.error output without failing, which masks regressions that test/setup/dom.ts is designed to catch.
Useful? React with 👍 / 👎.
| beforeEach(() => { | ||
| ;(globalThis as any).__ALLOW_CONSOLE_ERROR__ = true |
There was a problem hiding this comment.
The beforeEach here turns off unexpected console.error assertions for the whole "Content-Type Edge Cases" suite, including cases that should parse or no-op cleanly. This broad allowance can let new server-side error logs slip through unnoticed in tests that currently expect normal behavior (e.g., text/plain and form-data requests), reducing the suite's ability to detect regressions.
Useful? React with 👍 / 👎.
|
Nice defensive fix! Even though the failures weren't reproducible on Linux, they may be platform-dependent (Mac/different Node versions). What was good:
Notes:
All 2852 tests pass (186 files, 0 failures). Merged to main. Thanks! 🎉 — Codex CLI |
Removed 5 items per Dan's review: - #14 (FAB) — already exists - #18 (pinch-to-zoom font) — keep default browser zoom - #36 (shake to reconnect) — rejected - #37 (swipeable sessions) — rejected - #44 (low bandwidth mode) — rejected Updated 2 items: - #16/17 (key toolbar) — added draggable/scrollable bar with F1-F12 - #31/33 (scrollback) — changed to one-finger scroll, press-hold for text selection, no browser-level vertical scrollbar Renumbered from 45 to 40 items. Updated priority tiers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
api-edge-cases.test.tson mainconsole.errorspy intest/setup/dom.tsthrows on any unexpected error output, but Express's JSON parser legitimately logs parse errors toconsole.error__ALLOW_CONSOLE_ERROR__escape hatch in the 3 affecteddescribeblocks (Malformed JSON, Payload Size, Content-Type Edge Cases)Resolves FRE-9
Test plan
api-edge-cases.test.tspass (was 78/87)Generated with Claude Code