Skip to content

Phase 1: Testing foundation (schemas, utils, turbo test:unit) - #35

Merged
soorya-u merged 2 commits into
mainfrom
phase-1-testing-foundation
Jul 11, 2026
Merged

Phase 1: Testing foundation (schemas, utils, turbo test:unit)#35
soorya-u merged 2 commits into
mainfrom
phase-1-testing-foundation

Conversation

@soorya-u

@soorya-u soorya-u commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Implements #30

Summary

  • Add root and Turbo test tasks (test:unit, test:integration, test:e2e)
  • Add shared testing docs and tooling/test setup placeholders
  • Add Bun unit tests for @cyrus/schemas, @cyrus/utils, @cyrus/constants, and server signaling broadcast helpers
  • Add CI test-unit job on pull requests

Test plan

  • bun test:unit
  • bun check:types
  • bun check

Closes #30
Part of #29

Establish turbo test tasks, CI unit-test job, shared test tooling, and initial coverage for schemas, utils, constants, and server signaling helpers.

Co-authored-by: Cursor <cursoragent@cursor.com>
@vercel

vercel Bot commented Jul 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cyrus Ready Ready Preview, Comment Jul 11, 2026 6:25am

@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@soorya-u, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 22 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 485c6235-9941-4de2-8f45-c93dd28df106

📥 Commits

Reviewing files that changed from the base of the PR and between 307fb29 and fbebe25.

📒 Files selected for processing (4)
  • .github/workflows/ci.yml
  • apps/server/src/handlers/signaling.test.ts
  • shared/utils/src/conversations/turn-waiters.test.ts
  • shared/utils/src/identity.test.ts
📝 Walkthrough

Walkthrough

Adds a Bun-based unit-testing foundation across the monorepo, including Turbo and CI orchestration, package scripts, shared test setup placeholders, testing documentation, and coverage for schemas, constants, utilities, and server signaling.

Changes

Testing foundation

Layer / File(s) Summary
Test orchestration and shared setup
.github/workflows/ci.yml, package.json, turbo.json, docs/testing.md, tooling/test/...
Adds repository test scripts, Turbo tasks, a CI unit-test job, testing conventions, and shared setup placeholders.
Schema and operation-key coverage
shared/constants/..., shared/schemas/...
Adds Bun test configuration and validates operation-key builders plus RTC, thread, and signaling schema behavior.
Utility behavior coverage
shared/utils/...
Adds Bun test configuration and tests turn waiters, conversation folding, and path helpers.
Server signaling coverage
apps/server/...
Adds a unit-test script and tests signaling event broadcasting, peer filtering, and attachment handling.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Possibly related PRs

  • soorya-u/cyrus#6 — Implements the signaling handler behavior exercised by the new server tests.
  • soorya-u/cyrus#20 — Refactors the conversation folding logic covered by the new fold tests.
  • soorya-u/cyrus#21 — Changes schema modules directly exercised by the new schema tests.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR covers most of the testing foundation, but it does not add the requested identity.ts unit tests from issue #30. Add the missing @cyrus/utils identity.ts tests and any representative snapshot fixtures required by issue #30, then rerun bun run test:unit.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed No clear unrelated changes are introduced; the added testing docs, tooling, tests, and CI job all match the issue scope.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the PR’s main theme: Phase 1 testing foundation with schemas, utils, and test:unit infrastructure.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch phase-1-testing-foundation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
apps/server/src/handlers/signaling.test.ts (1)

20-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider asserting on encoded payload content.

The tests verify that the correct number of string payloads are sent, but don't validate the payload itself. Asserting that each sent payload matches the expected encodeHibernationRPCEvent(eventId, event) output would catch regressions in the encoding pipeline, not just the broadcast/filtering logic.

♻️ Optional: strengthen payload assertions
 test("sends encoded hibernation events to peers with event iterators", () => {
   const sent: unknown[] = [];
   const event: ServerEvent = {
     id: "worker-left",
     type: "peer-left",
   };

   broadcastSignalingEvent(
     [
       connection("controller-1", "event-controller", sent),
       connection("worker-1", "event-worker", sent),
     ],
     event
   );

   expect(sent).toHaveLength(2);
   expect(sent.every((payload) => typeof payload === "string")).toBe(true);
+  // Verify each payload is the encoded hibernation RPC event for the respective peer
+  expect(sent[0]).toBe(encodeHibernationRPCEvent("event-controller", event));
+  expect(sent[1]).toBe(encodeHibernationRPCEvent("event-worker", event));
 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/server/src/handlers/signaling.test.ts` around lines 20 - 37, Strengthen
the test for broadcastSignalingEvent by asserting each sent payload equals
encodeHibernationRPCEvent(event.id, event), in addition to checking the payload
count and string type. Use the existing event and sent values so the test
validates encoded content for both connections.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 43-46: Add a `needs: check-types` dependency to the `test-unit`
job so it runs only after the type-check job succeeds, preserving the intended
CI order.
- Line 49: Update the actions/checkout@v4 step in the test job to set
persist-credentials: false, preventing the GitHub token from being stored in git
configuration while running PR-controlled tests.

In `@shared/utils/src/conversations/turn-waiters.test.ts`:
- Around line 10-18: Await every expect(...).resolves or expect(...).rejects
matcher promise in all five tests in the turn waiter test suite, including the
turn interrupted case, by making the tests async and awaiting each assertion so
no matcher runs after test completion.

---

Nitpick comments:
In `@apps/server/src/handlers/signaling.test.ts`:
- Around line 20-37: Strengthen the test for broadcastSignalingEvent by
asserting each sent payload equals encodeHibernationRPCEvent(event.id, event),
in addition to checking the payload count and string type. Use the existing
event and sent values so the test validates encoded content for both
connections.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 09fb1464-f920-4c24-bbdc-976d050a8d6f

📥 Commits

Reviewing files that changed from the base of the PR and between 6dba6bd and 307fb29.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • .github/workflows/ci.yml
  • apps/server/package.json
  • apps/server/src/handlers/signaling.test.ts
  • docs/testing.md
  • package.json
  • shared/constants/package.json
  • shared/constants/src/operation-keys.test.ts
  • shared/constants/tsconfig.json
  • shared/schemas/package.json
  • shared/schemas/src/rtc/chat.test.ts
  • shared/schemas/src/rtc/threads.test.ts
  • shared/schemas/src/signaling.test.ts
  • shared/schemas/tsconfig.json
  • shared/utils/package.json
  • shared/utils/src/conversations/turn-waiters.test.ts
  • shared/utils/src/fold.test.ts
  • shared/utils/src/path.test.ts
  • shared/utils/tsconfig.json
  • tooling/test/mocks/README.md
  • tooling/test/setup/bun.setup.ts
  • tooling/test/setup/vitest.shared.ts
  • turbo.json

Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml
Comment thread shared/utils/src/conversations/turn-waiters.test.ts Outdated
@soorya-u

Copy link
Copy Markdown
Owner Author

Tracks sub-issue #30 (epic #29).

Run unit tests after typecheck with non-persisted checkout credentials, await async test assertions, add identity helper coverage, and assert encoded signaling payloads.

Co-authored-by: Cursor <cursoragent@cursor.com>
@soorya-u

Copy link
Copy Markdown
Owner Author

Addressed CodeRabbit review feedback in fbebe25:

  • CItest-unit now needs: check-types; checkout uses persist-credentials: false
  • turn-waiters.test.ts — all expect(...).resolves/.rejects assertions are awaited
  • identity.test.ts — added missing #30 coverage for randomId and generateName
  • signaling.test.ts — assertions now verify encoded hibernation payloads

Skipped the docstring-coverage bot warning (not applicable to test-only additions).

@soorya-u
soorya-u merged commit 9dddff2 into main Jul 11, 2026
6 checks passed
@soorya-u
soorya-u deleted the phase-1-testing-foundation branch July 11, 2026 06:30
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.

Phase 1: Testing foundation (schemas, utils, turbo test:unit)

1 participant