-
Notifications
You must be signed in to change notification settings - Fork 0
fix: merge src and tests #283
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
WalkthroughTests were relocated into Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #283 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 23 23
Branches 4 4
=========================================
Hits 23 23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/config.defaults.test.ts (1)
25-25: Fix incorrect dynamic import path.The dynamic import path
../src/config.jsis incorrect for a test file located atsrc/config.defaults.test.ts. This would try to resolve to a path outside the repository structure.Apply this diff:
- const { default: actual } = await import('../src/config.js') + const { default: actual } = await import('./config.js')src/middleware/postgraphile.test.ts (1)
3-4: Fix incorrect import and mock paths.The import and mock paths still reference
../../src/, which is incorrect for a test file now located atsrc/middleware/postgraphile.test.ts. These paths would resolve outside the repository structure.Apply this diff to fix all the incorrect paths:
-import config from '../../src/config.js' -import { postgres } from '../../src/database/index.js' +import config from '../config.js' +import { postgres } from '../database/index.js'-vi.mock('../../src/config.js', () => ({ +vi.mock('../config.js', () => ({ default: { schema: 'schema', postGraphile: vi.fn().mockName('config.postGraphile'), }, }))-vi.mock('../../src/database/index.js', () => ({ +vi.mock('../database/index.js', () => ({ postgres: vi.fn().mockName('postgres'), }))Also applies to: 6-6, 15-15
src/config.overwrite.test.ts (1)
30-30: Fix incorrect dynamic import path.The dynamic import path
../src/config.jsis incorrect for a test file located atsrc/config.overwrite.test.ts. This would try to resolve to a path outside the repository structure.Apply this diff:
- const { default: actual } = await import('../src/config.js') + const { default: actual } = await import('./config.js')
🧹 Nitpick comments (1)
vitest.config.ts (1)
12-14: Consider whether 100% statement coverage is realistic.A 100% statement coverage threshold is quite strict and may block legitimate changes. Consider whether a slightly lower threshold (e.g., 90-95%) would be more pragmatic while still maintaining high code quality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/config.defaults.test.ts(1 hunks)src/config.overwrite.test.ts(1 hunks)src/database/index.test.ts(2 hunks)src/database/postgres.test.ts(3 hunks)src/index.test.ts(2 hunks)src/middleware/bodyparser.test.ts(1 hunks)src/middleware/compress.test.ts(1 hunks)src/middleware/helmet.test.ts(1 hunks)src/middleware/index.test.ts(1 hunks)src/middleware/postgraphile.test.ts(1 hunks)src/router/health.test.ts(1 hunks)src/router/index.test.ts(2 hunks)tests/middleware/compress.test.ts(0 hunks)tests/middleware/helmet.test.ts(0 hunks)tests/middleware/index.test.ts(0 hunks)tsconfig.json(1 hunks)tsconfig.test.json(1 hunks)vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (3)
- tests/middleware/compress.test.ts
- tests/middleware/helmet.test.ts
- tests/middleware/index.test.ts
🔇 Additional comments (17)
tsconfig.json (1)
10-11: LGTM!Excluding test files from compilation is appropriate when co-locating tests with source code. This ensures test files don't end up in the production build.
tsconfig.test.json (1)
6-6: LGTM!Narrowing the include pattern correctly reflects that unit tests now live in
src/and are discovered through vitest configuration, while integration tests remain intestsInt/.vitest.config.ts (2)
5-5: LGTM!The test include pattern correctly points to the new test location in
src/, aligning with the co-location strategy.
6-18: Good test hygiene improvements.The new test options (
clearMocks,unstubEnvs,unstubGlobals,restoreMocks) and coverage configuration improve test isolation and visibility.src/database/index.test.ts (1)
2-14: LGTM!Import paths correctly use relative paths from the test's new location in
src/database/.src/database/postgres.test.ts (1)
3-25: LGTM!All import and mock paths correctly adjusted for the test's location in
src/database/.src/config.defaults.test.ts (1)
3-3: LGTM!The type import correctly uses a relative path from the test's location in
src/.src/middleware/postgraphile.test.ts (1)
28-28: LGTM!The dynamic import correctly uses a relative path for the co-located middleware module.
src/config.overwrite.test.ts (1)
3-3: LGTM!The type import correctly uses a relative path from the test's location in
src/.src/router/health.test.ts (1)
4-5: LGTM! Import paths correctly updated.The imports and mock paths have been correctly updated to use relative paths that reflect the new test location.
Also applies to: 7-7
src/middleware/helmet.test.ts (1)
1-15: LGTM! Test structure is sound.The test correctly validates helmet middleware initialization and export using dynamic imports and mocking.
src/middleware/bodyparser.test.ts (1)
11-11: LGTM! Import path correctly updated.The relative import path correctly reflects the new co-located test location.
src/middleware/index.test.ts (1)
1-35: LGTM! Comprehensive middleware index test.The test properly validates that all middlewares are exported with correct structure and order, using well-named mocks for debugging clarity.
src/router/index.test.ts (1)
2-2: LGTM! Import paths correctly updated.All import and mock paths have been correctly updated to relative paths for the co-located test location.
Also applies to: 4-4, 14-14
src/middleware/compress.test.ts (1)
1-16: LGTM! Compress middleware test is thorough.The test correctly validates that compress is invoked with the expected
threshold: 0option and that the export matches the mocked result.src/index.test.ts (2)
3-3: LGTM! Import paths correctly updated.All import and mock paths have been correctly updated to use relative paths for the new test location.
Also applies to: 9-10, 22-22, 27-27, 33-33, 48-48
41-43: Good practice: console.log mocked in beforeEach.Moving the console.log mock setup to
beforeEachensures a fresh mock for each test, which is a testing best practice.
|
🎉 This PR is included in version 2.0.8 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |



Summary by CodeRabbit
Tests
Chores