ci: fix linting pipeline + add Vitest test infrastructure - #31
Merged
Yuvraj-Sarathe merged 1 commit intoJul 30, 2026
Merged
Conversation
|
@Rudrx17 is attempting to deploy a commit to the Yuvraj Sarathe's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Reviewer's GuideFixes the PR verification CI by making test execution conditional and adding a full Vitest test setup, while tightening ESLint configuration and adding a basic homepage test. Sequence diagram for Vitest test execution with Next.js mockssequenceDiagram
actor Developer
participant Npm
participant Vitest
participant Vitest_setup
participant JSDOM
participant TestingLibraryReact
participant Home_page
participant Next_mocks
Developer->>Npm: npm test
Npm->>Vitest: run
Vitest->>Vitest_setup: load setupFiles (vitest.setup.tsx)
Vitest_setup->>Vitest: register mocks and console overrides
Vitest->>JSDOM: create jsdom test environment
Vitest->>TestingLibraryReact: execute Home page tests
TestingLibraryReact->>Home_page: render(Home)
Home_page->>Next_mocks: call next/navigation and next/link
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe pull request updates ESLint configuration, adds Vitest and React Testing Library infrastructure, introduces Home page tests, and adjusts CI to skip non-code pull requests and conditionally run tests. ChangesLint and Vitest CI improvements
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- Ignoring
.github/**inpaths-ignoremeans CI changes won’t trigger this workflow, which can make it harder to validate workflow edits; consider narrowing the ignored paths to avoid hiding CI config changes. - The global
console.errorsuppression invitest.setup.tsxis quite broad and may hide genuine issues; consider filtering on more specific message patterns or scoping the override to individual tests instead of globally.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Ignoring `.github/**` in `paths-ignore` means CI changes won’t trigger this workflow, which can make it harder to validate workflow edits; consider narrowing the ignored paths to avoid hiding CI config changes.
- The global `console.error` suppression in `vitest.setup.tsx` is quite broad and may hide genuine issues; consider filtering on more specific message patterns or scoping the override to individual tests instead of globally.
## Individual Comments
### Comment 1
<location path=".github/workflows/ci.yml" line_range="36-45" />
<code_context>
run: npm run lint
- # Step E: Run your test suite (verifies the code actually works)
+ - name: Check for test files
+ id: check-tests
+ run: |
+ if find . -name "*.test.ts" -o -name "*.test.tsx" -o -name "*.spec.ts" -o -name "*.spec.tsx" | grep -q .; then
+ echo "has_tests=true" >> $GITHUB_OUTPUT
+ else
+ echo "has_tests=false" >> $GITHUB_OUTPUT
+ fi
+
- name: Run Tests
+ if: steps.check-tests.outputs.has_tests == 'true'
run: npm test
\ No newline at end of file
</code_context>
<issue_to_address>
**suggestion (performance):** The `find` used to detect tests searches the entire tree, including `node_modules`, which may slow down CI.
Because `find` starts at repo root, it will walk `node_modules` and other large directories, which can significantly slow CI on bigger projects. Consider restricting the search to known test locations (e.g., `src`, `app`, `components`) or using a more targeted glob so the check remains fast while still correctly detecting whether tests exist.
</issue_to_address>
### Comment 2
<location path="package.json" line_range="45-49" />
<code_context>
"tw-animate-css": "^1.4.0",
- "typescript": "5.9.3"
+ "typescript": "5.9.3",
+ "vitest": "^2.0.0",
+ "@testing-library/react": "^16.0.0",
+ "@testing-library/jest-dom": "^6.4.0",
+ "jsdom": "^24.0.0",
+ "@vitejs/plugin-react": "^4.3.0"
}
}
</code_context>
<issue_to_address>
**suggestion (performance):** Test-only tooling is added under `dependencies` instead of `devDependencies`, which can bloat production installs.
These packages are only used for local development and CI, so they don’t need to be present in production images. Installing them via `dependencies` causes unnecessary install time and larger containers. Please move these test/build tools to `devDependencies` and reserve `dependencies` for runtime requirements only.
</issue_to_address>
### Comment 3
<location path="vitest.setup.tsx" line_range="31-40" />
<code_context>
+}));
+
+// Suppress console.error for known warnings in tests
+const originalError = console.error;
+beforeAll(() => {
+ console.error = (...args: any[]) => {
+ if (
+ typeof args[0] === 'string' &&
+ (args[0].includes('Warning: ReactDOM.render is no longer supported') ||
+ args[0].includes('act(...)'))
+ ) {
+ return;
+ }
+ originalError.call(console, ...args);
+ };
+});
+afterAll(() => {
+ console.error = originalError;
+});
\ No newline at end of file
</code_context>
<issue_to_address>
**suggestion (testing):** Globally overriding `console.error` in tests can hide unexpected errors and make debugging harder.
This wrapper not only filters the targeted React warnings, it changes `console.error` globally for the full test run. Any new warnings/errors that contain those substrings—or other test code that relies on `console.error`—could be unintentionally suppressed. Prefer scoping this behavior (e.g., per suite or per test) and using `vi.spyOn(console, 'error')` with precise message matching to avoid hiding real issues.
Suggested implementation:
```typescript
const originalError = console.error;
/**
* Helper to suppress specific, known React warnings in a scoped way.
*
* Usage in tests:
* const errorSpy = suppressReactConsoleErrors();
* // ...run test...
* errorSpy.mockRestore();
*/
export function suppressReactConsoleErrors() {
const errorSpy = vi
.spyOn(console, 'error')
.mockImplementation((...args: unknown[]) => {
const [message] = args;
if (
typeof message === 'string' &&
(
// Specific ReactDOM.render deprecation warning
message.startsWith('Warning: ReactDOM.render is no longer supported') ||
// Specific act(...) warnings
message.includes('Warning:') &&
message.includes('act(...)')
)
) {
return;
}
// Delegate all other errors to the original console.error
originalError(...(args as Parameters<typeof console.error>));
});
return errorSpy;
}
// Optionally expose helper globally for convenience in test suites
// so tests can call `const spy = suppressReactConsoleErrors();`
(globalThis as any).suppressReactConsoleErrors = suppressReactConsoleErrors;
```
To fully adopt the scoped behavior suggested in the review:
1. In individual test files or describe blocks that need these warnings suppressed, call `const errorSpy = suppressReactConsoleErrors();` in `beforeEach` (or a specific test) and `errorSpy.mockRestore();` in `afterEach` to avoid leaking the spy across tests.
2. If you prefer using the global helper, you can instead call `const errorSpy = (globalThis as any).suppressReactConsoleErrors();` in those same hooks.
3. If your TypeScript setup requires it, you may want to add a global type declaration (e.g., in `vitest.d.ts`) for `suppressReactConsoleErrors` on `globalThis` to avoid `any` casting.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
vitest.setup.tsx (1)
30-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not suppress every
act(...)diagnostic.This filter hides any
console.errorwhose first argument containsact(...), including actionable warnings from broken async or state-update handling. Match only a specific known warning, or remove the suppression.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vitest.setup.tsx` around lines 30 - 46, Update the console.error override in the beforeAll test setup so it no longer suppresses every message containing “act(...)”. Remove that broad match or narrow it to the exact known warning text, while preserving handling for the ReactDOM.render warning and restoration via afterAll.app/page.test.tsx (1)
4-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen the homepage assertions.
getByText(/GitDeep/i)and “any level-one heading” are too broad to validate the intended page. Assert the actual accessible heading name and at least one primary input or action so regressions are detected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/page.test.tsx` around lines 4 - 13, Strengthen the Home Page tests by replacing the broad GitDeep text and generic level-one heading assertions with an exact accessible heading name, and add an assertion for at least one primary input or action exposed by Home. Keep the tests scoped to the intended homepage elements so unrelated matching text or headings cannot satisfy them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.eslintrc.json:
- Around line 2-27: Update eslint.config.mjs, which ESLint 9 loads for the
package.json eslint . command, to include the .eslintrc.json rule overrides and
ignorePatterns so those ignores and disabled rules are applied under flat
config. Prefer moving the configuration into eslint.config.mjs rather than
relying on ESLINT_USE_FLAT_CONFIG=false.
In @.github/workflows/ci.yml:
- Around line 6-10: Remove the `.github/**` entry from the `paths-ignore` list
in the workflow trigger configuration, while preserving the other ignored paths
so pull requests modifying only workflow files still run CI.
- Around line 21-22: Update the actions/checkout@v4 step in the CI workflow to
disable persisted credentials by configuring its persist-credentials option to
false, while leaving the existing checkout behavior unchanged.
In `@vitest.config.ts`:
- Line 10: Align test discovery between vitest.config.ts at lines 10-10 and
.github/workflows/ci.yml at lines 36-47: either make Vitest’s include patterns
and the CI npm test condition cover the same .test and .spec files, or remove
the Vitest override and restrict CI discovery to files Vitest runs.
In `@vitest.setup.tsx`:
- Line 1: Update the setup import in vitest.setup.tsx to use the Vitest-specific
`@testing-library/jest-dom/vitest` entrypoint instead of the generic jest-dom
entrypoint.
---
Nitpick comments:
In `@app/page.test.tsx`:
- Around line 4-13: Strengthen the Home Page tests by replacing the broad
GitDeep text and generic level-one heading assertions with an exact accessible
heading name, and add an assertion for at least one primary input or action
exposed by Home. Keep the tests scoped to the intended homepage elements so
unrelated matching text or headings cannot satisfy them.
In `@vitest.setup.tsx`:
- Around line 30-46: Update the console.error override in the beforeAll test
setup so it no longer suppresses every message containing “act(...)”. Remove
that broad match or narrow it to the exact known warning text, while preserving
handling for the ReactDOM.render warning and restoration via afterAll.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e6de29b-3456-4b8b-af6f-6c4378f62d21
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
.eslintrc.json.github/workflows/ci.ymlapp/page.test.tsxcomponents/DotField.tsxpackage.jsonvitest.config.tsvitest.setup.tsx
Owner
|
@Rudrx17, please resolve merge conflicts and look into suggestions given by CodeRabbit's and Sourcery's reviews. |
Rudrx17
added a commit
to Rudrx17/GitDeep
that referenced
this pull request
Jul 30, 2026
- Remove .github/** from CI paths-ignore - Add persist-credentials: false to checkout - Fix test discovery to exclude node_modules - Move test dependencies to devDependencies - Replace global console.error override with scoped helper - Align Vitest include patterns with CI - Use vitest-specific jest-dom entrypoint - Migrate ESLint to flat config - Update tests to use scoped suppression helper
Rudrx17
force-pushed
the
fix/ci-pipeline-linting-exceptions
branch
from
July 30, 2026 07:21
0135275 to
16051bb
Compare
Rudrx17
added a commit
to Rudrx17/GitDeep
that referenced
this pull request
Jul 30, 2026
- Remove .github/** from CI paths-ignore - Add persist-credentials: false to checkout - Fix test discovery to exclude node_modules - Move test dependencies to devDependencies - Replace global console.error override with scoped helper - Align Vitest include patterns with CI - Use vitest-specific jest-dom entrypoint - Migrate ESLint to flat config - Update tests to use scoped suppression helper
- Remove .github/** from CI paths-ignore - Add persist-credentials: false to checkout - Fix test discovery to exclude node_modules - Move test dependencies to devDependencies - Replace global console.error override with scoped helper - Align Vitest include patterns with CI - Use vitest-specific jest-dom entrypoint - Migrate ESLint to flat config (remove .eslintrc.json) - Update tests to use scoped suppression helper
Rudrx17
force-pushed
the
fix/ci-pipeline-linting-exceptions
branch
from
July 30, 2026 08:02
d2b4802 to
4783774
Compare
Yuvraj-Sarathe
added
enhancement
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(ci): resolve failing lint/test pipeline + add Vitest infrastructure
paths-ignorefor docs/markdown/config filesvitest.config.tswith Next.js 15 supportvitest.setup.tswith Next.js mocks.eslintrc.jsonwithignorePatternsfor non-code fileseslint-disabledirective inDotField.tsx🔗 Related Issue
Closes #29
📝 Description of Changes
This PR resolves the CI pipeline failures that occurred on every pull request by introducing proper test infrastructure, improving lint configuration, and optimizing workflow execution.
Root Cause
npm testscript inpackage.json, causing the test step to fail on every CI run.Changes Implemented
CI Pipeline
paths-ignoreto skip CI for documentation and non-code changes:**/*.mddocs/**.github/**CHANGELOG.mdLICENSE*.txtpublic/***.test.*or*.spec.*) are present.Test Infrastructure
@vitejs/plugin-reactnpm testnpm run test:watchvitest.config.tswith:vitest.setup.tscontaining mocks for:next/navigationnext/imagenext/linkESLint
ignorePatternsfor:@next/next/no-img-elementreact/no-unescaped-entitiesTesting
app/page.test.tsxcontaining two passing tests verifying that the home page renders successfully.Code Quality
eslint-disabledirective fromcomponents/DotField.tsx.🏷️ Proposed Labels
📂 Core Files Changed
📸 Verification & Screenshots
npm run lintpasses (0 errors, expected warnings only)npm testpasses (1 test file, 2 tests)Local Verification
🤖 AI Assistance Declaration
Did you use an AI tool to write or assist with this code OR Pull Request?
<img>warnings shown during linting are expected. They originate from Next.js recommending the<Image />component and are existing warnings in the codebase rather than new issues.✅ The "I Swear I Didn't Break Anything" Pledge
Summary by Sourcery
Update CI configuration to avoid unnecessary runs and conditionally execute tests while introducing a Vitest-based testing setup and aligning linting with the new test infrastructure.
New Features:
Enhancements:
CI:
Tests:
Summary by CodeRabbit
Testing
Developer Experience
CI