Phase 3: Add web-test CI job, harden security gates, add P0 tests#221
Phase 3: Add web-test CI job, harden security gates, add P0 tests#221ComBba wants to merge 1 commit intophase-2/critical-bugsfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces new test suites for the event bus system in the agent and the SSE client and Zero Prompt API in the web frontend. The feedback focuses on improving the robustness and maintainability of these tests. Specifically, it is recommended to use the queue's maxsize property in the event bus tests instead of hardcoding capacity values. Additionally, for the frontend SSE client tests, replacing fixed setTimeout delays with Vitest's waitFor utility is suggested to reduce flakiness and improve execution speed.
| for i in range(300): | ||
| push_zp_event({"session_id": "s1", "type": f"event_{i}"}) | ||
| assert q.qsize() == 300 |
There was a problem hiding this comment.
Avoid hardcoding the queue capacity (300) in the test, as it mirrors a hardcoded value in the implementation. This makes the test fragile to changes in the production code's configuration. It is better to use the maxsize property of the queue object to ensure the test remains valid even if the capacity is adjusted.
| for i in range(300): | |
| push_zp_event({"session_id": "s1", "type": f"event_{i}"}) | |
| assert q.qsize() == 300 | |
| # Fill the queue to max | |
| for i in range(q.maxsize): | |
| push_zp_event({"session_id": "s1", "type": f"event_{i}"}) | |
| assert q.qsize() == q.maxsize |
| @@ -0,0 +1,172 @@ | |||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | |||
There was a problem hiding this comment.
Add waitFor to the Vitest imports to enable more robust and efficient asynchronous testing, replacing the fixed setTimeout delays used throughout the file.
| import { describe, it, expect, vi, beforeEach } from "vitest"; | |
| import { describe, it, expect, vi, beforeEach, waitFor } from "vitest"; |
| }); | ||
|
|
||
| // Wait for async stream processing | ||
| await new Promise((r) => setTimeout(r, 50)); |
There was a problem hiding this comment.
Using a fixed setTimeout delay to wait for asynchronous stream processing is a common source of test flakiness and unnecessarily increases test execution time. Consider using waitFor to poll for the expected condition, which is more robust and efficient. This pattern should be applied to all similar instances in this file.
| await new Promise((r) => setTimeout(r, 50)); | |
| await waitFor(() => expect(events.length).toBe(2)); |
CI Changes: - Add web-test job running vitest (was missing entirely) - bandit: remove || true, enforce --severity-level high - npm audit: remove || true, enforce --audit-level=high - mypy: replace || true with continue-on-error (visible but non-blocking) New Test Files: - web/src/lib/__tests__/zero-prompt-api.test.ts (8 tests) - startSession POST, SSE response parsing, error handling - queueBuild, passCard, deleteCard action payloads - getBuildEventsUrl format - web/src/lib/__tests__/sse-client.test.ts (6 tests) - JSON/plain-text SSE parsing, error handling, stream completion - abort function, comment line filtering - agent/tests/test_event_bus.py (8 tests) - register/unregister, session-scoped fan-out - queue overflow handling, multi-client delivery Total: 5 web test files (31 tests), 82 agent test files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
b14cbca to
7d40167
Compare
a80ed71 to
578bd12
Compare
Summary
CI Changes
web-testjob running vitest (was missing entirely from CI)|| true, enforce--severity-level high|| true, enforce--audit-level=high|| truewithcontinue-on-error: trueNew Tests
zero-prompt-api.test.ts— 8 tests (startSession, actions, SSE parsing)sse-client.test.ts— 6 tests (JSON/text parsing, error, abort, comments)test_event_bus.py— 8 tests (register, fan-out, overflow, multi-client)Changes
Test plan
🤖 Generated with Claude Code