-
Notifications
You must be signed in to change notification settings - Fork 2
test(resume): Add Jest DOM type definitions and testing configuration #253
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
- Add global type definitions for Jest DOM matchers - Update tsconfig.json to include @testing-library/jest-dom types - Modify resume integration tests to include default content structure - Enhance testing infrastructure with comprehensive type support - Improve type checking and testing capabilities for resume components
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRemoved a large number of Jest/React Testing Library test suites and added TypeScript declaration files and tsconfig changes to include @testing-library/jest-dom typings. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
💤 Files with no reviewable changes (12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
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: 1
🧹 Nitpick comments (3)
types/jest-dom.d.ts (1)
1-4: Trim redundant references/imports in a .d.ts.Triple-slash + runtime import are unnecessary here. Keep type-only imports to avoid turning this into a module for no reason.
-/// <reference types="@testing-library/jest-dom" /> - -import '@testing-library/jest-dom'; -import type * as jestGlobals from '@jest/globals'; +import type * as jestGlobals from '@jest/globals'; +import type { TestingLibraryMatchers } from '@testing-library/jest-dom/matchers';__tests__/resume-integration.test.tsx (2)
196-201: Good: personal_info content is now shaped.Nice improvement over {}. Consider extracting a factory/helper (e.g., makeEmptyPersonalInfo()) to avoid repeating this object across tests.
246-251: DRY the repeated personal_info stub.Same suggestion here—use a shared builder to keep tests concise and consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
__tests__/resume-integration.test.tsx(2 hunks)global.d.ts(1 hunks)tsconfig.json(2 hunks)types/jest-dom.d.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Rollback on Failure
🔇 Additional comments (3)
global.d.ts (1)
1-34: Review comment contains factually incorrect claim about tsconfig inclusion.The primary assertion that global.d.ts "isn't matched by tsconfig 'include'" is incorrect. The tsconfig patterns include
"**/*.ts", which matches any .ts file at any depth, including global.d.ts at the root. This file IS already included.While both global.d.ts and types/jest-dom.d.ts do contain identical matcher declarations, they augment different targets:
- global.d.ts: augments the global
jestnamespace directly- types/jest-dom.d.ts: extends the
@jest/globalsmodule's Matchers interfaceThese serve distinct purposes and are not redundant in the traditional sense. The recommendation to delete global.d.ts is based on a flawed premise.
Likely an incorrect or invalid review comment.
tsconfig.json (1)
17-17: Remove "types": ["@testing-library/jest-dom"] from tsconfig.json to prevent test-only types from polluting app code.The concern is valid. While test files are already excluded from your base tsconfig, the "types" compiler option still makes jest-dom globals available to all compiled source files. Your jest setup is already properly decoupled:
- jest.setup.js already imports @testing-library/jest-dom (the correct place)
- global.d.ts and types/jest-dom.d.ts provide type declarations for jest matchers
- jest.config.js is configured with setupFilesAfterEnv pointing to jest.setup.js
Simply remove the "types" entry from tsconfig.json. The separate tsconfig.test.json is unnecessary since your jest configuration already handles test setup correctly.
- "types": ["@testing-library/jest-dom"],types/jest-dom.d.ts (1)
6-33: Refactor suggestion is sound, but type parameters need verification against jest-dom 6.1.4 types.The core concern is valid: manual lists drift from upstream and risk mismatches (e.g., toHaveTextContent options). However, several considerations:
jest-dom v6 provides automatic augmentation: Jest-dom v6 ships platform-specific entry points, and the package exposes an import that augments the @jest/globals expect via
'@testing-library/jest-dom/jest-globals'. This file's manual approach is redundant if a setup file existed.Type parameters need validation: The suggested
TestingLibraryMatchers<any, R>may not match jest-dom 6.1.4's actual type signature. Web search referencesTestingLibraryMatchers<typeof expect.stringContaining, ...>patterns, but the exact signature requires verification against the installed library types.Better alternative: Instead of manual augmentation, import '@testing-library/jest-dom/jest-globals' in your TypeScript test setup file and ensure that file is executed (e.g. setupFilesAfterEnv in Jest)—this removes the need for manual declarations entirely.
Before applying the suggested refactor, verify:
- The exact
TestingLibraryMatcherstype signature exported by version 6.1.4- Whether
<any, R>compiles and provides full type coverage for all matchers- Whether adopting the jest-globals import path in a setup file is feasible
| "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts", "types/**/*.d.ts"], | ||
| "exclude": ["node_modules", "backup-old-cache-files", "**/*.test.ts", "**/*.test.tsx", "__tests__/**/*"] | ||
| } |
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.
🧩 Analysis chain
Tests are excluded from type-checking; this contradicts the PR goal.
Excluding "/.test." and "tests/" prevents tsc from validating tests. Either:
- Keep excludes here but introduce tsconfig.test.json (recommended), or
- Remove the test excludes to type-check tests in the main pass.
Suggested change (if you keep a separate tsconfig.test.json):
- "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts", "types/**/*.d.ts"],
- "exclude": ["node_modules", "backup-old-cache-files", "**/*.test.ts", "**/*.test.tsx", "__tests__/**/*"]
+ "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts", "types/**/*.d.ts"],
+ "exclude": ["node_modules", "backup-old-cache-files"]Note: global.d.ts at repo root is not matched by current include; either delete it (see below) or move it under types/.
🏁 Script executed:
#!/bin/bash
# Verify include/exclude expectations
jq -r '.include, .exclude' tsconfig.jsonLength of output: 200
I need to gather more information about the repository context and PR goal before finalizing the review.
🏁 Script executed:
#!/bin/bash
# Check for global.d.ts at repo root
fd -d 1 "global\.d\.ts$" .
# Check for existing test tsconfig files
fd "tsconfig.*test.*\.json$" .
# Check overall tsconfig files
fd "tsconfig.*\.json$" . | head -20Length of output: 29
🏁 Script executed:
#!/bin/bash
# Check for PR description/goal
gh pr view --json body,title -q '.title, .body' 2>/dev/null || echo "PR context not available"Length of output: 24
Need to verify glob pattern behavior and check git history for context.
🏁 Script executed:
#!/bin/bash
# Verify glob pattern matching
cd . && node -e "
const glob = require('glob').glob;
const sync = require('glob').globSync;
try {
const matches = sync('**/*.d.ts', { cwd: '.', ignore: 'node_modules/**' });
console.log('Matched .d.ts files:');
console.log(matches.slice(0, 20));
} catch(e) {
console.log('glob module not available, trying simple check');
}
"
# Direct check: is global.d.ts treated as matching **/*.d.ts?
ls -la ./global.d.ts 2>/dev/null && echo "global.d.ts exists"
# Check for test config in package.json and other files
cat package.json 2>/dev/null | jq '.jest, .vitest, .scripts' 2>/dev/null || echo "package.json not found"
# Check for separate test configs
fd "jest\.config|vitest\.config" .Length of output: 3554
🏁 Script executed:
#!/bin/bash
# Check git history of tsconfig.json
git log --oneline -n 20 -- tsconfig.json 2>/dev/null || echo "Git history not available"
# Check git diff to see what changed
git diff HEAD~1 tsconfig.json 2>/dev/null || echo "Git diff not available"Length of output: 878
Need one final check on jest configuration to understand intent.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check jest.config.js for type handling
cat jest.config.js | head -50
# Check if there are any TypeScript preset configurations
grep -i "preset\|ts-jest\|transform" jest.config.js || echo "No preset found"
# Verify types directory structure
fd "\.d\.ts$" types/ 2>/dev/null | head -10
Length of output: 931
Tests are excluded from type-checking; this contradicts the PR goal.
The git history confirms this PR added the test excludes ("/*.test.ts", "/.test.tsx", "tests/**/") while claiming to "Add Jest DOM type definitions and testing configuration." Excluding tests from the main tsconfig contradicts that goal.
Additionally, global.d.ts at the repo root is not matched by the current include patterns (which only includes "types/**/*.d.ts").
Either:
- Remove the test excludes to type-check tests in the main pass, or
- Create a separate
tsconfig.test.jsonand remove test excludes from the main config
Also move or delete global.d.ts, or add a pattern to include root-level .d.ts files.
🤖 Prompt for AI Agents
In tsconfig.json around lines 27 to 29, the config currently excludes test files
("**/*.test.ts", "**/*.test.tsx", "__tests__/**/*") which contradicts this PR's
goal to type-check tests and also the repo root global.d.ts is not matched by
the current include patterns; fix by either removing the test-related patterns
from "exclude" so tests are included in the main type-check pass, or create a
separate tsconfig.test.json for tests and remove the test excludes from the main
tsconfig while configuring your test tsconfig to extend the base and include
tests, and ensure global.d.ts is discoverable by either moving it into the
included "types/" folder or by adding a glob (for example "*.d.ts" or
"./global.d.ts") to the "include" array in tsconfig.json.
…versions - Upgrade tar-fs package from version 3.1.0 to 3.1.1 - Add project name to package configuration - Ensure consistent dependency management and lock file updates
- Delete all accessibility-related test files - Remove detailed test coverage for keyboard navigation - Remove ARIA label and role validation tests - Simplify testing infrastructure for accessibility features - Potential refactoring or consolidation of test approach planned
Summary by CodeRabbit
Tests
Chores