-
Notifications
You must be signed in to change notification settings - Fork 10
feat: add comprehensive tests for API routes #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…figure Vitest for testing environment - Implemented extensive unit tests for the timelapse router covering queries, draft creation, committing, updating, deleting, and publishing functionalities. - Added tests for the tracing router to ensure authentication and input validation. - Developed tests for the user router, including user retrieval, updating, device management, and authentication checks. - Configured Vitest with setup files, coverage reporting, and module resolution for improved testing experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request introduces a comprehensive test suite for backend API routes, adding unit and integration tests using Vitest, along with supporting infrastructure for mocking and test data generation.
- Adds Vitest configuration and test setup infrastructure
- Implements mock factories for database, S3, encryption, and server utilities
- Creates test data factories using Faker.js for generating realistic test entities
- Adds comprehensive test coverage for user, timelapse, snapshot, global, tracing, and comment API routers
Reviewed changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Configures Vitest with jsdom environment, coverage reporting, and path aliases |
| apps/web/src/tests/setup.ts | Global test setup file that resets mocks before each test |
| apps/web/src/tests/factories.ts | Factory functions for generating test data matching the Prisma schema |
| apps/web/src/tests/mocks/* | Mock implementations for database, S3, encryption, environment, and server utilities |
| apps/web/src/tests/*.test.ts | Test suites covering various API routers with unit and integration tests |
| apps/web/package.json | Adds testing dependencies including vitest, faker, testing-library, and msw |
| apps/web/src/server/routers/api/user.ts | Reorders union type for consistency (UserSchema before PublicUserSchema) |
| .gitignore | Excludes test coverage artifacts from version control |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@typescript-eslint/eslint-plugin": "^8.50.0", | ||
| "@typescript-eslint/parser": "^8.50.0", | ||
| "@vitejs/plugin-react": "^5.1.2", | ||
| "@vitest/coverage-v8": "4.0.16", |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version specification 4.0.16 should use a caret (^) to allow minor and patch updates. This prevents the project from receiving important bug fixes and improvements. Using an exact version without a caret is uncommon and typically only done for security-critical dependencies.
| "@vitest/coverage-v8": "4.0.16", | |
| "@vitest/coverage-v8": "^4.0.16", |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Thank you for the PR, @Efe-Cal! <3 |
|
Well, I committed before seeing your edit. After the merge it will be fine right? So, no need to pull main and re-commit? |
|
Pull from |
|
@ascpixi hi! I pulled from main and (I think) fixed the tests workflow. But it still fails due to real errors in the code. I don't remember changing anything in the source code, so I don't know what to do. Please help. |
ascpixi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type check is failing cuz we don't have hackatimeId - I left a comment to point that out! Some minor changes requested as well, but overall this looks good! :D
ascpixi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just some minor nitpicks and we should be good to merge!
| SLACK_REDIRECT_URI: "http://localhost:3000/api/auth/slack/callback", | ||
| JWT_SECRET: "test-jwt-secret-key-that-is-long-enough", | ||
| // Used by `timelapse.syncWithHackatime` in production. | ||
| HACKATIME_ADMIN_KEY: "hka_test-hackatime-admin-key", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're no longer using the admin API - we OAuth with Hackatime now. This is a minor nit-pick, we don't use this anywhere anyways
|
Thank you! 💖 |
Hi!
This pull request introduces a comprehensive test suite for the backend logic in the web app, adding unit and integration tests for core API routers and supporting robust test data generation.