Add comprehensive test infrastructure with Jest, mocks, and 640 passing tests - #8
Conversation
- Installed Jest with TypeScript support - Created test directory structure with mocks and fixtures - Added comprehensive tests for utility functions: * barChart (renderBar, renderTextBar, renderResultsText) * debounce (debouncedUpdate) * emojiPrefix (getNumberEmoji, getStarEmoji, getOptionEmoji) * slackRetry (withRetry with exponential backoff) - All 44 tests passing Co-authored-by: jnahian <11865446+jnahian@users.noreply.github.com>
- Created GitHub Actions workflow for automated testing - Added test coverage reporting with Codecov integration - Updated README.md with Testing section - Created comprehensive TESTING.md guide with: * Running tests * Writing tests * Test utilities and mocks * Best practices * CI/CD integration details * Troubleshooting tips Co-authored-by: jnahian <11865446+jnahian@users.noreply.github.com>
- Added 10 tests for channelError utility (isNotInChannelError, notInChannelText) - Created example service layer tests for voteService (8 tests) - Demonstrates testing pattern with mocked Prisma client - Fixed type issues in Prisma mock for TypeScript compatibility - Added test badge to README - All 62 tests passing Co-authored-by: jnahian <11865446+jnahian@users.noreply.github.com>
- Created TEST_COVERAGE_SUMMARY.md documenting all completed work - Summarizes infrastructure setup, test utilities, and coverage - Documents 62 passing tests (54 utility + 8 service tests) - Provides usage examples and best practices - Outlines remaining work and future improvements Co-authored-by: jnahian <11865446+jnahian@users.noreply.github.com>
- Added comment explaining advanceTimersByTimeAsync usage - This method is available in Jest 28+ and works correctly - Addresses code review feedback while keeping working implementation Co-authored-by: jnahian <11865446+jnahian@users.noreply.github.com>
- Added permissions block to test workflow - Limited to contents:read and pull-requests:write - Follows security best practice of least privilege - Resolves CodeQL security alert Co-authored-by: jnahian <11865446+jnahian@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR establishes comprehensive test infrastructure for the Askify Slack poll bot using Jest 30 and TypeScript. It provides mock utilities for Prisma and Slack clients, test data factories, and 62 passing unit tests with 100% coverage of the utility layer. The infrastructure includes CI/CD integration via GitHub Actions with Codecov reporting.
Changes:
- Added Jest 30 + ts-jest testing framework with 60% coverage thresholds
- Created mock utilities for Prisma and Slack APIs with test data factories
- Implemented 54 utility tests (100% coverage) and 8 service layer example tests
- Configured GitHub Actions workflow for automated testing and coverage reporting
- Added comprehensive testing documentation (TESTING.md, TEST_COVERAGE_SUMMARY.md)
Reviewed changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Added test scripts and dependencies; CRITICAL: incorrectly moved @slack/bolt to devDependencies |
| jest.config.js | Jest configuration with TypeScript support, coverage thresholds, and custom settings |
| tests/setup.ts | Global test setup with environment variables and console mocking |
| tests/mocks/prisma.ts | Mock Prisma client with CRUD operations and transaction support |
| tests/mocks/slack.ts | Mock Slack API client with factory functions for test data |
| tests/fixtures/testData.ts | Test data factories for polls, votes, and templates; CRITICAL: uses wrong field name |
| tests/utils/*.test.ts | Comprehensive utility function tests (barChart, debounce, emojiPrefix, slackRetry, channelError) |
| tests/services/voteService.test.ts | Example service layer tests demonstrating testing patterns |
| .github/workflows/test.yml | CI/CD workflow for automated testing with coverage reporting |
| TESTING.md | Comprehensive testing guide with examples and best practices |
| TEST_COVERAGE_SUMMARY.md | Detailed implementation summary and coverage metrics |
| README.md | Updated with testing section and commands |
| .gitignore | Added coverage directory exclusions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| global.console = { | ||
| ...console, | ||
| log: jest.fn(), | ||
| debug: jest.fn(), | ||
| info: jest.fn(), | ||
| warn: jest.fn(), | ||
| error: jest.fn(), |
There was a problem hiding this comment.
Silencing console.error in tests can hide legitimate errors and make debugging difficult. Consider removing error: jest.fn() from the console mock or making this behavior configurable via an environment variable like DEBUG=true npm test. This will help developers identify real errors during test development.
| export function resetSlackMocks() { | ||
| Object.values(mockSlackClient).forEach((namespace) => { | ||
| Object.values(namespace).forEach((fn) => { | ||
| if (typeof fn === 'function' && 'mockClear' in fn) { | ||
| fn.mockClear(); | ||
| } | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
The resetSlackMocks function only calls mockClear(), which clears call history but doesn't reset mock implementations or return values. Consider using mockReset() instead to prevent test pollution where a mock implementation from one test affects another test.
There was a problem hiding this comment.
Fixed in commit 1c5dc55. Changed to use mockReset() instead of mockClear() to prevent test pollution where mock implementations from one test affect another.
| * Provides a jest-mocked Prisma client with common operations | ||
| */ | ||
|
|
||
| export const mockPrismaClient: any = { |
There was a problem hiding this comment.
The use of any type for the mock Prisma client reduces type safety. Consider using a more specific type like DeepMockProxy<PrismaClient> from jest-mock-extended or creating a proper type definition that matches the Prisma client interface.
| export function createMockUser(overrides: Partial<any> = {}) { | ||
| return { | ||
| id: 'U123456', | ||
| name: 'testuser', | ||
| real_name: 'Test User', | ||
| is_bot: false, | ||
| ...overrides, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Create a mock Slack channel | ||
| */ | ||
| export function createMockChannel(overrides: Partial<any> = {}) { | ||
| return { | ||
| id: 'C123456', | ||
| name: 'test-channel', | ||
| is_channel: true, | ||
| is_private: false, | ||
| ...overrides, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Create a mock Slack message response | ||
| */ | ||
| export function createMockMessageResponse(overrides: Partial<any> = {}) { |
There was a problem hiding this comment.
The use of any type in function parameters reduces type safety. Consider using specific Slack types from @slack/web-api like UsersInfoResponse, ConversationsInfoResponse, etc. This will provide better type checking and IDE support.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Added @slack/bolt version 4.6.0 to dependencies - Removed @slack/bolt from devDependencies - Updated @types/jest from 30.0.0 to 29.5.12 - Updated jest from 30.2.0 to 29.7.0
Covers all 13 functions in pollService: createPoll, getPoll, closePoll, updatePollMessageTs, getExpiredPolls, getScheduledPolls, activatePoll, getUserPolls, updatePoll, repostPoll, cancelScheduledPoll, getPollsNeedingReminders, and markReminderSent. Includes tests for CRUD operations, query filters, edge cases, and vote count tracking with proper Prisma mocking infrastructure. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Adds 14 tests covering all 4 template service functions (saveTemplate, getTemplates, getTemplate, deleteTemplate) with full coverage of poll types, settings, and ownership verification. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Adds 33 passing tests covering buildPollMessage and buildResultsDM functions. Includes coverage for all poll types, live results, anonymous settings, rating averages, voter names, action buttons, and Block Kit message structure. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Covers buildResultsDMBlocks and buildCreatorNotifyDM with 29 test cases including rating averages, voter names, anonymous settings, recovery notes, and action buttons (Share Results, Repost, Save as Template, Close Poll). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Tests cover successful vote handling (single/multi-select), message updates with debouncing, voter name fetching, error handling (closed polls, rejected votes, missing data, Slack API errors), action type validation, and vote parsing. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add comprehensive integration test suite for close poll and add option actions with 20 tests covering registration, permission enforcement, error handling, duplicate detection, and action tracking. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
24 test cases covering all subcommands: help, list with date filters, templates, inline poll creation with flags (--multi, --yesno, --anon, --close), default modal opening, argument parsing, and error handling. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
13 test cases covering validation, successful creation, options extraction, settings configuration, and different poll types (single/multi/yes_no/rating). Tests verify error handling, Slack API integration, and creator notifications. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
13 test cases covering auto-close and scheduled poll jobs including cron scheduling, poll activation, message posting, DM notifications, voter name fetching, error handling, and multi-poll processing. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add 21 tests covering DM message handler and App Home handler: - DM handler: greeting detection, help requests, default responses - App Home handler: tab filtering, view publishing - Update mockSlackClient to include views.publish method Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add 9 tests covering remaining action handler branches: - modalActions early returns (5 tests) - 55% → 95% branches - Error rethrow paths (3 tests) - repost, share actions - Poll not found scenarios (2 tests) - share, template actions Coverage: 90.6%/83.15%/92%/92.34% → 91.79%/84.64%/95.33%/93.24% Tests: 475 → 484 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add error rethrow test for non-channel errors. Final coverage: 91.86% / 84.64% / 95.33% / 93.31% Total tests: 485 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add 3 tests for rarely hit edge cases: - askify list: closesAt formatting, scheduled poll edit/cancel buttons - askify inline poll: non-channel error handling Coverage: 91.86%/84.64%/95.33%/93.31% → 92.42%/85.69%/95.33%/93.91% Tests: 485 → 488 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update to reflect final achievement: - 488 total tests (426 new) - 92.42% statements, 85.69% branches, 95.33% functions, 93.91% lines - Average 91.84% coverage - Near 95% for 3 out of 4 metrics - Document practical maximum and remaining gaps Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 35 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function createTestTemplate(overrides: Partial<any> = {}) { | ||
| return { | ||
| id: 'tmpl-123', | ||
| creatorId: 'U123456', | ||
| name: 'Daily Standup', | ||
| question: 'What are you working on today?', | ||
| pollType: 'single_choice' as PollType, | ||
| options: ['Task A', 'Task B', 'Task C'], | ||
| settings: { | ||
| anonymous: false, | ||
| allowVoteChange: true, | ||
| liveResults: true, | ||
| }, | ||
| createdAt: new Date('2024-01-01T12:00:00Z'), | ||
| ...overrides, | ||
| }; | ||
| } |
There was a problem hiding this comment.
createTestTemplate()’s default shape (creatorId, question, pollType, options, settings) doesn’t match the PollTemplate Prisma model/service interface (userId, name, config, createdAt). Even though tests override some fields, keeping the factory aligned with the real model will prevent accidental reliance on non-existent properties. Consider redefining the defaults to match src/services/templateService.ts’s PollTemplate interface (e.g., userId + config).
There was a problem hiding this comment.
Fixed in commit 1c5dc55. Updated createTestTemplate() to match the PollTemplate schema with userId and config fields instead of creatorId, question, pollType, and options.
| ``` | ||
| Test Suites: 23 passed, 23 total | ||
| Tests: 488 passed, 488 total | ||
| Time: ~8 seconds |
There was a problem hiding this comment.
This summary says the project uses “Jest 30.x” and reports “488 tests”, but package.json pins Jest ^29.7.0 and the PR title/description mention 62 tests. Please reconcile these numbers/versions (either update the docs or the PR description) so readers don’t get conflicting information.
There was a problem hiding this comment.
Fixed in commit 1c5dc55. Updated TEST_COVERAGE_SUMMARY.md to correctly reference Jest 29.x (not 30.x) which matches package.json. The 488 tests count is accurate and reflects the comprehensive test coverage added.
| services/ # Tests for service layer (coming soon) | ||
| actions/ # Tests for action handlers (coming soon) |
There was a problem hiding this comment.
The testing section still labels several directories as “coming soon” (e.g., services/, actions/), but this PR adds substantial coverage in those areas. Update this structure snippet to reflect what’s actually present so new contributors aren’t misled.
| services/ # Tests for service layer (coming soon) | |
| actions/ # Tests for action handlers (coming soon) | |
| services/ # Tests for service layer (Slack, Prisma, domain logic) | |
| actions/ # Tests for action handlers and Slack actions |
There was a problem hiding this comment.
Fixed in commit 1c5dc55. Updated the test structure in README.md to reflect all implemented test directories (services, actions, commands, views, events, jobs, middleware) without "coming soon" labels.
| ``` | ||
| __tests__/ | ||
| ├── setup.ts # Global test setup | ||
| ├── fixtures/ | ||
| │ └── testData.ts # Test data factories | ||
| ├── mocks/ | ||
| │ ├── prisma.ts # Prisma client mock | ||
| │ └── slack.ts # Slack client mock | ||
| ├── utils/ | ||
| │ ├── barChart.test.ts # Utility function tests | ||
| │ ├── debounce.test.ts | ||
| │ ├── emojiPrefix.test.ts | ||
| │ └── slackRetry.test.ts | ||
| ├── services/ # Service layer tests (coming soon) | ||
| ├── blocks/ # Block builder tests (coming soon) | ||
| ├── actions/ # Action handler tests (coming soon) | ||
| └── commands/ # Command handler tests (coming soon) | ||
| ``` |
There was a problem hiding this comment.
The “Test Structure” section says service/block/action/command tests are “coming soon”, but this PR adds many of those suites. Please update this section to match the current repository structure (and optionally add a brief note on how to run a subset of tests by directory).
There was a problem hiding this comment.
Fixed in commit 1c5dc55. Updated the test structure in TESTING.md to show all implemented test directories without "coming soon" labels, reflecting the comprehensive coverage achieved.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…move unused imports - Changed mockReset() instead of mockClear() in resetPrismaMocks() and resetSlackMocks() to prevent test pollution - Fixed createTestTemplate() to match PollTemplate schema (userId, config fields) - Removed unused mockPrismaClient imports from voteAction.test.ts and pollManagement.test.ts - Updated documentation to reflect actual test coverage (Jest 29.x, current test structure) - Updated README.md, TESTING.md, and TEST_COVERAGE_SUMMARY.md test structure sections All 488 tests passing ✅ Co-authored-by: jnahian <11865446+jnahian@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Summary
Establishes comprehensive test infrastructure for the bot with Jest 29/TypeScript, mock utilities, and near-complete coverage across all layers: utilities, services, blocks, actions, commands, views, events, jobs, middleware, and lib.
Changes
Test Infrastructure
createTestPoll(),createTestVote()(voterId/votedAt),createTestTemplate()(userId/config)jest.spyOn()for console mockingmockReset()to prevent test pollutionTests (640 total, 34 suites, all passing)
truncate,escapeMrkdwn,mentions,debounceflush), services (claim APIs, transactional voting, unique-violation idempotency), blocks, actions (creator-permission checks, error/edge branches, handler registration), commands, views, events, jobs (claim-based close/schedule/reminder semantics, startup recovery), middleware, andsrc/lib(prisma init, health server)Synced with current
mainmain(conflicts inpackage.json/lockfile resolved; lockfile regenerated)main:claimPollClose/claimScheduledPoll/claimReminderSend, transactional vote handlers,getSettings(), creator-only checks on edit/repost/shareCI/CD
Documentation
TESTING.mdguide,TEST_COVERAGE_SUMMARY.mdwith current statistics, README test commandsCommands:
Type
Testing
npx tsc --noEmit)npm run test:cipasses coverage thresholdsOriginal prompt:
https://claude.ai/code/session_01SDbX28ZreWVBP2GaGydXzc