Skip to content

feat(tests): add Trello and GitHub payload factories to tests/helpers/factories.ts#999

Merged
aaight merged 1 commit intodevfrom
feature/trello-github-payload-factories
Mar 23, 2026
Merged

feat(tests): add Trello and GitHub payload factories to tests/helpers/factories.ts#999
aaight merged 1 commit intodevfrom
feature/trello-github-payload-factories

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Mar 23, 2026

Summary

  • Add Trello payload factories: createTrelloActionPayload(), createTrelloCard(), createTrelloTriggerContext()
  • Add GitHub payload factories: createMockPR(), createCheckSuitePayload(), createReviewPayload(), createGitHubTriggerContext()
  • All 7 new factory functions follow the existing createMockProject() override pattern (sensible defaults + shallow-merge overrides param)
  • Zero changes to existing test files — purely additive

Test plan

  • npm test passes with all 332 test files and 6326 tests green
  • Biome lint passes with no errors
  • No existing test files were modified

Card

https://trello.com/c/69c166bd870f89b4887a6ba8

🤖 Generated with Claude Code

🕵️ claude-code · claude-sonnet-4-6 · 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.

Summary

LGTM — Clean, purely additive change that centralizes test payload construction into shared factories. All 7 CI checks pass, no existing code is modified, and the factories follow the established createMockProject() override pattern.

Should Fix (non-blocking)

  • Custom GitHub interfaces instead of source types: The PR defines MockPR, CheckSuitePayload, and ReviewPayload as custom interfaces in the test helper rather than importing the existing source types (GitHubPullRequestPayload, GitHubCheckSuitePayload, GitHubPullRequestReviewPayload from src/triggers/github/types.ts). This creates a drift risk — if the source types evolve (e.g., new required fields), these factory interfaces won't break at compile time. Since TriggerContext.payload is unknown, the factories would still compile even if they produce structurally incompatible payloads. Consider importing and using the source types (or Partial<> of them) to keep the factories coupled to the real contract. The Trello factories already do this correctly with TrelloWebhookPayload and TrelloCard.

  • Misleading @example on createTrelloActionPayload: The JSDoc example shows createTrelloActionPayload({ action: { data: { listAfter: ... } } }), implying you can surgically override action.data.listAfter. But with the shallow ...overrides spread, this actually replaces the entire action object — losing id, idMemberCreator, type, date, and all other data fields. This could surprise consumers and lead to subtle test bugs. Consider either (a) noting the shallow-merge limitation in the docstring, or (b) using a deep-merge for these nested payloads.

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

@aaight aaight merged commit 43d4d80 into dev Mar 23, 2026
9 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.

2 participants