Skip to content

feat(tests): add shared mock factories and migrate heaviest test files#825

Merged
aaight merged 2 commits intodevfrom
feature/shared-mock-factories
Mar 14, 2026
Merged

feat(tests): add shared mock factories and migrate heaviest test files#825
aaight merged 2 commits intodevfrom
feature/shared-mock-factories

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Mar 14, 2026

Summary

What was implemented

tests/helpers/sharedMocks.ts (new file)

Exports reusable mock objects for the top 5 mocked modules: mockLogger (src/utils/logging.js, ~47 files), mockConfigProvider (src/config/provider.js, ~26 files), mockGitHubClientModule + mockWithGitHubToken (src/github/client.js, ~19 files), mockTriggerCheckModule (src/triggers/shared/trigger-check.js, ~17 files), mockDbClientModule (src/db/client.js, ~18 files).

Migrated test files (5 files)

  1. tests/unit/backends/adapter.test.ts (18 vi.mock calls) - uses mockLogger, mockConfigProvider, mockWithGitHubToken
  2. tests/unit/triggers/agent-execution.test.ts - uses mockLogger, mockTriggerCheckModule
  3. tests/unit/triggers/status-changed.test.ts - uses mockLogger, mockTriggerCheckModule
  4. tests/unit/triggers/check-suite-success.test.ts - uses mockTriggerCheckModule, mockGitHubClientModule
  5. tests/unit/router/adapters/github.test.ts (18 vi.mock calls) - uses mockLogger, mockConfigProvider, mockWithGitHubToken

Key decisions

  • vi.mock() calls remain in each test file (vitest requires this - factories are hoisted)
  • Shared mock imports must appear before vi.mock() calls in test files to avoid vitest hoisting issues
  • Follows the same patterns as tests/helpers/mockDb.ts and tests/helpers/factories.ts

Test plan

  • All 5 migrated test files pass (140 tests passing)
  • Full unit test suite passes (pre-existing failures in 3 unrelated gadget test files are unchanged)
  • TypeScript type checking passes
  • Lint passes (biome formatter)

Generated with Claude Code

🕵️ claude-code · claude-sonnet-4-6 · run details

@nhopeatall
Copy link
Copy Markdown
Collaborator

nhopeatall commented Mar 14, 2026

⚠️ review agent failed

Error: Model "claude-opus-4-6" is not compatible with the Codex engine. Configure a Codex-compatible model (e.g. "gpt-5.4") or switch to a different engine.

Manual intervention may be required.

@zbigniewsobiecki
Copy link
Copy Markdown
Member

@aaight review this

@aaight
Copy link
Copy Markdown
Collaborator Author

aaight commented Mar 14, 2026

⚠️ respond-to-pr-comment agent failed

Error: Model "claude-opus-4-6" is not compatible with the Codex engine. Configure a Codex-compatible model (e.g. "gpt-5.4") or switch to a different engine.

Manual intervention may be required.

@zbigniewsobiecki
Copy link
Copy Markdown
Member

@aaight review this, pretty please

@aaight
Copy link
Copy Markdown
Collaborator Author

aaight commented Mar 14, 2026

⚠️ respond-to-pr-comment agent failed

Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again.

Manual intervention may be required.

@zbigniewsobiecki
Copy link
Copy Markdown
Member

@aaight review this PR

@aaight
Copy link
Copy Markdown
Collaborator Author

aaight commented Mar 14, 2026

Review findings:

  1. tests/helpers/sharedMocks.ts:90
    The new mockGithubClient does not mirror the actual githubClient contract in src/github/client.ts. It adds methods that do not exist on the real client (getPRComments, postPRComment, getFailedWorkflowRuns, updatePR, getRepository, createWebhook, deleteWebhook, listWebhooks, requestReview) and it omits real ones that do exist (getPRReviewComments, getPRIssueComments, createPRComment, getFailedWorkflowRunJobs, getOpenPRByBranch, addIssueCommentReaction, addReviewCommentReaction, mergePR, branchExists). Because this file is positioned as the shared reusable mock for future migrations, that mismatch makes it easy to write passing tests against a fake API surface instead of the production one. I would align this helper to the real export surface before using it more broadly.

I did not find another blocking issue in the migrated test files.

🕵️ codex · gpt-5.4 · run details

@zbigniewsobiecki
Copy link
Copy Markdown
Member

@aaight tests/helpers/sharedMocks.ts:90
The new mockGithubClient does not mirror the actual githubClient contract in src/github/client.ts. It adds methods that do not exist on the real client (getPRComments, postPRComment, getFailedWorkflowRuns, updatePR, getRepository, createWebhook, deleteWebhook, listWebhooks, requestReview) and it omits real ones that do exist (getPRReviewComments, getPRIssueComments, createPRComment, getFailedWorkflowRunJobs, getOpenPRByBranch, addIssueCommentReaction, addReviewCommentReaction, mergePR, branchExists). Because this file is positioned as the shared reusable mock for future migrations, that mismatch makes it easy to write passing tests against a fake API surface instead of the production one. I would align this helper to the real export surface before using it more broadly.

@aaight
Copy link
Copy Markdown
Collaborator Author

aaight commented Mar 14, 2026

Aligned \ to the real \ surface and pushed it in e80b967. The shared mock now includes the actual exported methods that were missing, removes the non-existent ones, and is guarded with \ so future drift is caught by typecheck.\n\nVerification I ran here:\n- \n- \n-
RUN v3.2.4 /workspace/cascade-cascade-1773502611879\n-
RUN v3.2.4 /workspace/cascade-cascade-1773502611879

✓ |unit-api| tests/unit/router/adapters/github.test.ts (24 tests) 60ms
✓ |unit-triggers| tests/unit/triggers/agent-execution.test.ts (31 tests) 23ms
✓ |unit-backends| tests/unit/backends/adapter.test.ts (49 tests) 38ms

Test Files 3 passed (3)
Tests 104 passed (104)
Start at 15:41:22
Duration 1.88s (transform 973ms, setup 139ms, collect 2.15s, tests 121ms, environment 0ms, prepare 156ms)\n\nI also ran ; it still has unrelated existing failures in comment-posting gadget tests in this environment because run-details footers are being appended to outbound comment bodies during execution.

🕵️ codex · gpt-5.4 · run details

@aaight
Copy link
Copy Markdown
Collaborator Author

aaight commented Mar 14, 2026

Aligned mockGithubClient to the real src/github/client.ts surface and pushed it in e80b967.

The shared mock now includes the actual exported methods that were missing, removes the non-existent ones, and is guarded with satisfies typeof import('../../src/github/client.js').githubClient so future drift is caught by typecheck.

Verification I ran here:

  • npm run typecheck
  • npm run lint
  • npx vitest run tests/unit/triggers/check-suite-success.test.ts
  • npx vitest run tests/unit/backends/adapter.test.ts tests/unit/router/adapters/github.test.ts tests/unit/triggers/agent-execution.test.ts tests/unit/triggers/status-changed.test.ts

I also ran npm run test:unit; it still has unrelated existing failures in comment-posting gadget tests in this environment because run-details footers are being appended to outbound comment bodies during execution.

🕵️ codex · gpt-5.4 · run details

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

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

LGTM - the shared mock helpers preserve the export shapes used by the migrated tests, the touched test set passes (140/140), and I did not find any architectural or behavioral regressions in the refactor.

🕵️ codex · gpt-5.4 · run details

@aaight aaight merged commit 6eac9ae into dev Mar 14, 2026
6 checks passed
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.

3 participants