Repository navigation
fix(web): improved test coverage to 93% - #17
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughAdds multiple new Vitest + React Testing Library test suites for pages and layouts, along with test setup and vitest.config updates; no runtime or application logic changes. Changes
Sequence Diagram(s)(omitted — changes are tests and config, no new multi-component runtime control flow introduced) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/web/src/pages/LoginPage.spec.tsx`:
- Around line 33-46: The tests fail because LoginPage reads
import.meta.env.VITE_API_URL on submit and throws if it's unset; update the test
setup in the beforeEach (where useNavigate, useAuth, and global.fetch are
mocked) to stub import.meta.env.VITE_API_URL to a valid URL string so the submit
path proceeds and fetch is called. Ensure you set VITE_API_URL early in
beforeEach (alongside mocks for useNavigate/useAuth/global.fetch) so LoginPage's
submit handler sees it during the test.
In `@apps/web/src/pages/SignupPage.spec.tsx`:
- Around line 24-38: The tests fail because SignupPage reads VITE_API_URL at
module load; update the test setup by stubbing process.env.VITE_API_URL inside
the beforeEach (where useNavigate, useAuth, useSearchParams and global.fetch are
mocked) so the module-level read succeeds; add setting like
process.env.VITE_API_URL = 'http://test' in beforeEach (and optionally clear or
restore it in an afterEach) to ensure SignupPage can render during tests.
In `@apps/web/src/pages/UsersPage.spec.tsx`:
- Around line 67-80: The test is flaky because
authorizedFetch.mockRejectedValueOnce is set after render and can be consumed by
the initial GET /users call; update the UsersPage spec to make the
authorizedFetch mock conditional so only the POST /users call fails: modify the
mock for authorizedFetch (used in the 'handles add user failure' test) to a
mockImplementation that inspects the request (e.g., method or URL) and returns
the normal users GET response for the initial fetch but throws/rejects with new
Error('Fetch Failed') only when the POST to create a user is invoked, ensuring
the rejection is targeted and preventing the race with the initial GET.
🧹 Nitpick comments (1)
apps/web/vitest.config.ts (1)
23-37: Align layouts coverage include/exclude to avoid confusion.Layouts are explicitly included and then fully excluded, so they never contribute to coverage. If layouts are intentionally excluded, remove them from
include; if they should be covered, drop or narrow the exclude.
e80e438 to
ed1826e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/vitest.config.ts`:
- Around line 29-39: The coverage config is excluding layouts despite adding
them to include: in vitest.config.ts remove the conflicting pattern
'src/layouts/**' from the exclude array so the include pattern
'src/layouts/**/*.tsx' is honored; locate the include/exclude arrays in the
exported Vitest config and delete the 'src/layouts/**' entry (or narrow it) to
ensure layout .tsx files are counted in coverage.
🧹 Nitpick comments (1)
apps/web/src/test/setup.ts (1)
9-26: Consolidateimport.meta.envstubbing to avoid divergent values.
Object.defineProperty(global, 'import', …)does not affect moduleimport.meta, whilevi.stubGlobal('import.meta', …)only setsVITE_API_URL. If any code readsimport.meta.env.DEV|MODE|SSR, those will beundefined. Prefer a single, consistent stub. Please verify the recommended approach against your Vitest version.♻️ Proposed refactor (single stub)
-// Mock import.meta.env -Object.defineProperty(global, 'import', { - value: { - meta: { - env: { - VITE_API_URL: 'http://localhost:3000/api', - DEV: true, - MODE: 'test', - SSR: false - }, - }, - }, -}); - -vi.stubGlobal('import.meta', { - env: { - VITE_API_URL: 'http://localhost:3000/api', - }, -}); +// Mock import.meta.env +vi.stubGlobal('import.meta', { + env: { + VITE_API_URL: 'http://localhost:3000/api', + DEV: true, + MODE: 'test', + SSR: false, + }, +});
ed1826e to
9847850
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.coderabbit.yaml:
- Around line 2-4: The comment incorrectly states the default reviews.profile is
"assertive"; update the .coderabbit.yaml to either correct the comment to say
the default is "chill" or explicitly set reviews.profile: assertive to match the
comment, and ensure request_changes_workflow remains set as intended; look for
the "reviews" block and the keys "request_changes_workflow" and
"reviews.profile" to apply the fix.
In `@apps/web/src/pages/LoginPage.spec.tsx`:
- Around line 120-124: The test overrides window.location directly which can
leak state; capture the original location in a variable (e.g., originalLocation)
and move the Object.defineProperty call into beforeEach (inside the
describe('LoginPage') block) to set value: { origin: 'http://localhost:3000',
search: '' } with writable and configurable true, then restore the
originalLocation in afterEach by redefining window.location back to
originalLocation; remove the inline Object.defineProperty from the individual
test so setup/teardown live in beforeEach/afterEach.
- Around line 36-39: Replace the incorrect environment stubbing using
vi.stubGlobal('import.meta', ...) in the LoginPage tests with Vitest's env
helper by calling vi.stubEnv('VITE_API_URL', 'http://test-api.com') where the
stub is set up, and add an afterEach() cleanup that calls vi.unstubAllEnvs() to
restore environment state; update references around the existing test setup (the
code that currently calls vi.stubGlobal('import.meta', ...)) and ensure the new
vi.stubEnv and vi.unstubAllEnvs calls are used instead.
In `@apps/web/src/test/setup.ts`:
- Around line 8-26: Remove the conflicting mocks and use Vitest's environment
stubbing: delete the Object.defineProperty(global, 'import', ...) block and the
vi.stubGlobal('import.meta', ...) call, and replace them with a single
vi.stubEnv({...}) call that sets VITE_API_URL and any needed flags (DEV, MODE,
SSR) so import.meta.env is consistent; ensure you call vi.stubEnv with the exact
variable names used elsewhere and that vi is imported from 'vitest' in setup.ts.
♻️ Duplicate comments (1)
apps/web/src/pages/SignupPage.spec.tsx (1)
27-30: Thevi.stubGlobal('import.meta', ...)approach won't work for mocking environment variables.
import.metais a module-level meta-property, not a global.vi.stubGlobal()sets properties onglobalThis(orwindowin jsdom), so it cannot modifyimport.meta. The correct approach for Vitest is to usevi.stubEnv(), which properly sets bothprocess.envandimport.meta.env.✅ Use vi.stubEnv instead
beforeEach(() => { vi.clearAllMocks(); - // Stub VITE_API_URL - vi.stubGlobal('import.meta', { - env: { VITE_API_URL: 'http://test-api.com' } - }); + // Stub VITE_API_URL using Vitest's env stubbing + vi.stubEnv('VITE_API_URL', 'http://test-api.com');Add cleanup in
afterEach:afterEach(() => { vi.unstubAllEnvs(); });Alternatively, enable automatic cleanup in
vitest.config.ts:export default { test: { unstubEnvs: true } }
🧹 Nitpick comments (2)
apps/web/src/layouts/TenantLayout.spec.tsx (1)
115-123: Remove inline debugging commentary; consider less brittle selectors.
The “re-read” notes (Line 115–Line 119) add noise, and counting “Dashboard” occurrences is fragile if layout changes. Consider removing the comments and using role/test-id assertions instead.🧹 Suggested cleanup (remove commentary)
- // It renders twice (Desktop + Mobile sidebars) - // Wait, sidebar has user name? NO, sidebar shows Org Name. Dropdown shows User Name. - // Let's re-read TenantLayout.tsx carefully. - // SidebarContent: line 56 shows user.organizationName. - // Topbar Dropdown: Line 193 shows user.name. - expect(screen.getByText('John Doe')).toBeInTheDocument(); expect(screen.getByText('john@example.com')).toBeInTheDocument(); expect(screen.getAllByText('Acme Corp')).toHaveLength(2); // Desktop sidebar + Mobile sidebarapps/web/src/pages/LoginPage.spec.tsx (1)
86-90: Consider using an assertion instead of throwing an error.The manual throw makes test failure messages less informative. Using an assertion provides better error output.
♻️ Suggested improvement
// Strict check on body const fetchCall = (global.fetch as Mock).mock.calls.find(call => call[0].includes('/auth/login')); - if (!fetchCall) throw new Error("Fetch not called"); + expect(fetchCall).toBeDefined(); const body = JSON.parse(fetchCall[1].body);
| // Mock import.meta.env | ||
| Object.defineProperty(global, 'import', { | ||
| value: { | ||
| meta: { | ||
| env: { | ||
| VITE_API_URL: 'http://localhost:3000/api', | ||
| DEV: true, | ||
| MODE: 'test', | ||
| SSR: false | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| vi.stubGlobal('import.meta', { | ||
| env: { | ||
| VITE_API_URL: 'http://localhost:3000/api', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and examine its content
cat -n apps/web/src/test/setup.ts | head -40Repository: pramodnarayana/nexiom
Length of output: 851
🏁 Script executed:
# Check the Vitest version in the project
fd -t f package.json | xargs grep -l vitest | head -5Repository: pramodnarayana/nexiom
Length of output: 99
🏁 Script executed:
# Look for vitest version specifically
fd -t f package.json -x cat {} \; | grep -A 5 -B 5 vitestRepository: pramodnarayana/nexiom
Length of output: 1365
🏁 Script executed:
# Let's check Vitest documentation for the correct API in 2.1.9
# First, let me search for any vitest config or usage patterns in the repo
fd -t f "vitest.config" "vitest.setup" "vite.config"Repository: pramodnarayana/nexiom
Length of output: 222
🌐 Web query:
Vitest 2.1.9 vi.stubEnv API documentation import.meta.env
💡 Result:
vi.stubEnv(name, value) — API (Vitest 2.x)
- Signature: function stubEnv(name: T, value: T extends 'PROD' | 'DEV' | 'SSR' ? boolean : string | undefined): Vitest.
- Effect: sets the environment variable on both process.env and import.meta.env.
- Restore: call vi.unstubAllEnvs() to restore values changed by vi.stubEnv.
- Notes: assigning import.meta.env directly also works but won't be restored by unstubAllEnvs. Example:
- vi.stubEnv('NODE_ENV', 'production') → process.env.NODE_ENV and import.meta.env.NODE_ENV become 'production'.
- vi.stubEnv('NODE_ENV', undefined) → both become undefined. [1][2]
Sources: Vitest API docs. [1] https://vitest.dev/api/vi [2] https://v2.vitest.dev/api/vi
🌐 Web query:
Vitest 2.1.9 stubGlobal vs stubEnv import.meta environment variables
💡 Result:
Short answer: use vi.stubEnv to change import.meta.env values; vi.stubGlobal only changes globals on globalThis (window/global) and does NOT affect import.meta.env. vi.stubEnv updates both process.env and import.meta.env and records originals so vi.unstubAllEnvs can restore them; vi.stubGlobal records/restores globals via vi.unstubAllGlobals. [1][2]
Examples:
- vi.stubEnv('NODE_ENV', 'test') → process.env.NODE_ENV and import.meta.env.NODE_ENV become 'test'. [1]
- vi.stubGlobal('innerWidth', 100) → globalThis.innerWidth (and window.innerWidth in jsdom) becomes 100 but import.meta.env is untouched. [1][2]
Sources:
- Vitest API (v2) — vi.stubEnv / vi.unstubAllEnvs and vi.stubGlobal / vi.unstubAllGlobals. [1]
- Vitest docs (stable) — vi.stubGlobal details. [2]
Avoid conflicting import.meta.env mocks.
Lines 8–20's Object.defineProperty(global, 'import', …) and Line 22's vi.stubGlobal('import.meta', …) are redundant and conflicting. The vi.stubGlobal call does not actually affect import.meta.env—it only affects globals on globalThis. Use vi.stubEnv() (supported in Vitest 2.1.9) for a single, correct source of truth:
🛠️ Suggested fix
-// Mock import.meta.env
-Object.defineProperty(global, 'import', {
- value: {
- meta: {
- env: {
- VITE_API_URL: 'http://localhost:3000/api',
- DEV: true,
- MODE: 'test',
- SSR: false
- },
- },
- },
-});
-
-vi.stubGlobal('import.meta', {
- env: {
- VITE_API_URL: 'http://localhost:3000/api',
- },
-});
+// Mock import.meta.env using Vitest API
+vi.stubEnv('VITE_API_URL', 'http://localhost:3000/api');
+vi.stubEnv('DEV', true);
+vi.stubEnv('MODE', 'test');
+vi.stubEnv('SSR', false);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Mock import.meta.env | |
| Object.defineProperty(global, 'import', { | |
| value: { | |
| meta: { | |
| env: { | |
| VITE_API_URL: 'http://localhost:3000/api', | |
| DEV: true, | |
| MODE: 'test', | |
| SSR: false | |
| }, | |
| }, | |
| }, | |
| }); | |
| vi.stubGlobal('import.meta', { | |
| env: { | |
| VITE_API_URL: 'http://localhost:3000/api', | |
| }, | |
| }); | |
| // Mock import.meta.env using Vitest API | |
| vi.stubEnv('VITE_API_URL', 'http://localhost:3000/api'); | |
| vi.stubEnv('DEV', true); | |
| vi.stubEnv('MODE', 'test'); | |
| vi.stubEnv('SSR', false); |
🤖 Prompt for AI Agents
In `@apps/web/src/test/setup.ts` around lines 8 - 26, Remove the conflicting mocks
and use Vitest's environment stubbing: delete the Object.defineProperty(global,
'import', ...) block and the vi.stubGlobal('import.meta', ...) call, and replace
them with a single vi.stubEnv({...}) call that sets VITE_API_URL and any needed
flags (DEV, MODE, SSR) so import.meta.env is consistent; ensure you call
vi.stubEnv with the exact variable names used elsewhere and that vi is imported
from 'vitest' in setup.ts.
9847850 to
207638c
Compare
1 participant
@CodeRabbit show flow diagram
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.