Feature: Core Component & Service API Test Coverage (vertex app)#3694
Conversation
Add comprehensive unit tests for proxyConfig, vertex-config, useContextAwareRouting, api-routing, and utils to improve coverage and validate edge cases. Refactor useContextAwareRouting by extracting isRouteAccessible into a standalone exported helper and update the hook to call it with the sidebar config, making routing logic easier to test and reuse. Tests include environment mocking and logger stubs for validateProxyConfig and various cases for API URL resolution, path versioning, class name utilities, and elapsed duration calculations.
Add comprehensive unit tests for core hooks (useBannerWithDelay, useClipboard, useCohorts, useDevices, useGroups, usePermissions, useRecentlyVisited, useRoles, useServerSideTableState, useSites, useUserContext, useWindow). Introduce test helpers and mocks: renderWithProviders, renderHookWithProviders, createTestQueryClient, and setupStore (configuring relevant Redux slices). Also add simple test factories (device, organization, user) and various API/adapter/permission/banner mocks to standardize the testing environment and improve coverage for hook behavior.
Replace the inline reducer object with a combined `rootReducer` using Redux Toolkit's `combineReducers`. Adds the `combineReducers` import and constructs a single root reducer from user, sites, devices, grids, cohorts, and groups slices before passing it to `configureStore`. This centralizes reducer composition and makes extending or replacing reducers easier in tests and runtime.
Update useBannerWithDelay test to expect the banner payload to include severity: "info". The assertion in src/vertex/core/hooks/useBannerWithDelay.test.tsx now checks for { message: "Second", severity: "info" }, reflecting the banner payload change to include a default severity.
Add Vitest + React Testing Library unit tests for multiple shared and UI components. New tests cover shared components (ReusableButton, ReusableFileUpload, ReusableInputField, ReusableSelectInput, ReusableTable, ReusableToast, CardWrapper) and many UI primitives (accordion, alert, avatar, badge, banner, button, calendar, card, checkbox, combobox, command, date-picker, dialog, dropdown-menu, empty-state, form, input, label, popover, progress, select, skeleton, switch, tabs, textarea, tooltip). Tests assert accessibility roles, user interactions, and component states; and include necessary mocks for browser APIs (clipboard, PointerEvent, ResizeObserver, scrollIntoView) and app contexts (next/navigation, banner-context) to stabilize JSDOM test runs.
Add comprehensive Vitest + React Testing Library unit tests for various UI components under src/vertex/components/ui. Tests cover ErrorBoundary, alert dialog, chart container, collapsible, custom dialog, forbidden error/page, hcaptcha widget, hover card, multi-select, pagination, permission tooltip, scroll area, select field, separator, sheet, sonner/toaster, stepper, table, and toast. Includes necessary test-time mocks/stubs (ResizeObserver, PointerEvent, next/navigation, react-redux, env constants, tooltip/provider, next-themes, use-toast) and uses userEvent for interaction tests to validate rendering and basic behaviors.
Modify ReusableTable to use a plain <style> tag (remove styled-jsx) and remove the defaultProps block. Update several unit tests: ErrorBoundary test now prevents & cleans up window error events to avoid noisy logs; chart test imports vi, mocks recharts' ResponsiveContainer and wraps ChartContainer in a sized div to stabilize rendering; custom-dialog test adds Dialog title/description nodes; stepper test simplifies children to plain spans and removes unused StepTitle/StepDescription imports. These changes address test stability and clean up component defaults.
Add comprehensive unit tests and test utilities for core services and utils. New tests cover network-service (axios/fetch flows, errors, timeouts), onboardingService (secureApiProxy patch calls), proxyClient (request forwarding, auth/api token injection, error handling), secureApiProxyClient (axios interceptors, token handling, response error behavior), and platform (Mac architecture detection fallbacks). Also add test factories (apiResponseFactory) for mocking Axios/Fetch responses and next-auth mocks (nextAuth) to simplify session and server-session stubbing.
Refactor multiple UI test files to improve TypeScript types and remove unused imports. Replaced broad any casts with stricter types (unknown, never, Record<string, unknown>, and React.ReactNode) including vi.importActual typing and mock component props. Unified MockPointerEvent assignment to window.PointerEvent as never and adjusted ReusableTable test to use String(val) and a never cast for columns. Also removed unused imports (AvatarImage, Banner variants, userEvent) and added typed Tooltip mocks to reduce TS errors and improve type safety across tests.
|
Warning Review limit reached
More reviews will be available in 15 minutes and 9 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
📝 WalkthroughWalkthroughAdds a comprehensive Vitest + React Testing Library test suite across the ChangesVitest Testing Infrastructure and Coverage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (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 |
|
New azure vertex changes available for preview here |
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/vertex/components/shared/table/ReusableTable.tsx (1)
1475-1479: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the scrollbar rule scoped to this table
<style>makes.scrollbar-hideglobal, so any other element using that class will pick up this scrollbar suppression too. Switch back tostyle jsxunless the global leak is intentional.💡 Minimal fix
- <style>{` + <style jsx>{` .scrollbar-hide::-webkit-scrollbar { display: none; } `}</style>🤖 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 `@src/vertex/components/shared/table/ReusableTable.tsx` around lines 1475 - 1479, The scrollbar suppression rule is leaking globally because the plain style block makes .scrollbar-hide apply outside ReusableTable. Update the style usage in ReusableTable to be component-scoped again by switching back to style jsx, keeping the scrollbar-hiding rule local to this table only. Use the existing ReusableTable style block and the .scrollbar-hide selector as the place to fix.
🟡 Minor comments (12)
src/vertex/app/changelog.md-15-16 (1)
15-16: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win"Replaced legacy test runners" is misleading.
The PR objectives state this "expands the frontend test suite" and "adds a comprehensive Vitest + RTL test suite" — there's no evidence that existing runners were removed or replaced. This wording could mislead consumers into thinking a migration occurred rather than additive coverage. Consider "Added Vitest & React Testing Library alongside existing test infrastructure" or similar.
🤖 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 `@src/vertex/app/changelog.md` around lines 15 - 16, Update the changelog entry in changelog.md to avoid implying a migration: the phrase in the “Vitest & React Testing Library” bullet should not say “Replaced legacy test runners.” Instead, revise it to reflect that Vitest and React Testing Library were added alongside the existing test infrastructure. Keep the “CI/CD Pipeline” bullet unchanged unless needed for wording consistency.src/vertex/test/utils/renderHookWithProviders.tsx-8-14 (1)
8-14: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDon't let
renderOptions.wrapperreplace the providers.Line 32 spreads
renderOptionsafterwrapper: Wrapper, so any caller-provided wrapper drops the Redux and Query providers entirely. This helper should either omitwrapperfrom its public options or compose it explicitly.Suggested fix
-interface ExtendedRenderHookOptions<Props> extends RenderHookOptions<Props> { +interface ExtendedRenderHookOptions<Props> + extends Omit<RenderHookOptions<Props>, "wrapper"> { preloadedState?: any; } @@ return { store, queryClient, - ...renderHook(renderCallback, { wrapper: Wrapper, ...renderOptions }), + ...renderHook(renderCallback, { ...renderOptions, wrapper: Wrapper }), }; }Also applies to: 32-32
🤖 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 `@src/vertex/test/utils/renderHookWithProviders.tsx` around lines 8 - 14, The renderHookWithProviders helper is letting a caller-supplied renderOptions.wrapper override the internal provider wrapper, which removes the Redux and Query providers. Update renderHookWithProviders so wrapper cannot be replaced from the public options, or explicitly compose any external wrapper with the existing Wrapper used for providers. Keep the fix centered on renderHookWithProviders and the Wrapper/renderOptions spread so the providers are always preserved.src/vertex/test/utils/renderWithProviders.tsx-14-17 (1)
14-17: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the provider wrapper authoritative.
ExtendedRenderOptionsstill allowswrapper, and Line 52 lets it overrideWrapper. That means a custom wrapper can remove the Redux/React Query providers and make tests stop reflecting the real app setup.Suggested fix
-interface ExtendedRenderOptions extends Omit<RenderOptions, "queries"> { +interface ExtendedRenderOptions extends Omit<RenderOptions, "queries" | "wrapper"> { preloadedState?: any; route?: string; } @@ - return { store, queryClient, ...render(ui, { wrapper: Wrapper, ...renderOptions }) }; + return { store, queryClient, ...render(ui, { ...renderOptions, wrapper: Wrapper }) }; }Also applies to: 52-52
🤖 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 `@src/vertex/test/utils/renderWithProviders.tsx` around lines 14 - 17, `ExtendedRenderOptions` currently still inherits `wrapper`, and `renderWithProviders` lets the caller-supplied wrapper override the built-in `Wrapper`, which can bypass the Redux and React Query providers. Update `ExtendedRenderOptions` to exclude `wrapper` and keep `Wrapper` authoritative inside `renderWithProviders` so tests always use the app-style provider setup.src/vertex/test/mocks/nextAuth.ts-13-28 (1)
13-28: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDeep-merge
useroverrides here.Right now a partial
useroverride replaces the whole defaultuserobject. A call likecreateMockSession({ user: { privilege: "admin" } })dropsid,accessToken, and the rest of the defaults, which makes shared auth tests easy to misconfigure.Suggested fix
-export const createMockSession = (overrides = {}) => ({ - user: { - id: "test-user-id", - accessToken: "test-access-token", - userName: "testuser", - organization: "test-org", - privilege: "user", - firstName: "Test", - lastName: "User", - country: "UG", - timezone: "Africa/Kampala", - phoneNumber: "123456789", - }, - expires: "2099-01-01T00:00:00.000Z", - accessToken: "test-access-token", - ...overrides, -}); +export const createMockSession = (overrides: Record<string, any> = {}) => ({ + user: { + id: "test-user-id", + accessToken: "test-access-token", + userName: "testuser", + organization: "test-org", + privilege: "user", + firstName: "Test", + lastName: "User", + country: "UG", + timezone: "Africa/Kampala", + phoneNumber: "123456789", + ...(overrides.user ?? {}), + }, + expires: "2099-01-01T00:00:00.000Z", + accessToken: "test-access-token", + ...overrides, +});🤖 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 `@src/vertex/test/mocks/nextAuth.ts` around lines 13 - 28, The createMockSession helper is shallow-merging overrides, so a partial user override replaces the default user object instead of merging into it. Update createMockSession in nextAuth.ts to deep-merge the nested user field by combining the default user object with overrides.user while still allowing top-level overrides like expires and accessToken to apply normally.src/vertex/core/hooks/useRecentlyVisited.test.tsx-36-69 (1)
36-69: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWait for the effect-driven rerender before asserting
visitedPages.Lines 43-49 and Lines 66-68 assert synchronously after
renderHookWithProviders(...), butuseRecentlyVisitedfillsvisitedPagesfromuseEffectafter mount and then updates again onceisLoadedflips true. That makes these checks timing-dependent and prone to flaky failures. Wrap the expectations inwaitFor(...)so they run after the hook settles.🤖 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 `@src/vertex/core/hooks/useRecentlyVisited.test.tsx` around lines 36 - 69, The `useRecentlyVisited` tests are asserting `visitedPages` too early, before the hook’s `useEffect`-driven state updates have completed. Update the specs in `useRecentlyVisited.test.tsx` to wait for the hook to settle after `renderHookWithProviders(...)`, especially around the `visitedPages` checks in the two affected test cases. Use `waitFor(...)` around the expectations so the assertions run after `useRecentlyVisited` finishes loading and rerendering.src/vertex/components/shared/toast/ReusableToast.test.tsx-7-10 (1)
7-10: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert a toast result, not the render container.
render()always attachescontainerto the document, so this test passes even if<Toaster />renders nothing. Trigger a toast and assert the message appears instead.🤖 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 `@src/vertex/components/shared/toast/ReusableToast.test.tsx` around lines 7 - 10, The Toaster test currently only asserts that render() returned a container, which does not verify any toast behavior. Update ReusableToast.test.tsx to trigger an actual toast through the shared toast API used by the Toaster component, then assert the expected toast message is rendered. Keep the test focused on verifying visible toast output rather than the render container in the Toaster component test.src/vertex/components/ui/dialog.test.tsx-47-55 (1)
47-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe close path is never asserted.
After Line 54 the test exits immediately, so it still passes if the dialog never closes. Add a post-click assertion that the dialog is removed.
Suggested fix
-import { render, screen } from "`@testing-library/react`"; +import { render, screen, waitFor } from "`@testing-library/react`"; @@ expect(screen.getByText("Dialog Title")).toBeInTheDocument(); expect(screen.getByText("Dialog Description")).toBeInTheDocument(); await userEvent.click(screen.getByRole("button", { name: "Close" })); + await waitFor(() => + expect(screen.queryByRole("dialog")).not.toBeInTheDocument() + ); });🤖 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 `@src/vertex/components/ui/dialog.test.tsx` around lines 47 - 55, The dialog test in dialog.test.tsx only verifies opening and never checks the close behavior, so update the dialog interaction test around the “Open Dialog” and “Close” clicks to assert the dialog is removed after closing. Use the existing dialog query with the “dialog” role and add a post-click expectation after the Close action so the test fails if the dialog remains visible.src/vertex/components/ui/dropdown-menu.test.tsx-24-27 (1)
24-27: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRestore these JSDOM shims after the test.
These assignments mutate global browser APIs for the rest of the worker, so later suites can pass or fail depending on execution order. Please scope them with setup/teardown and restore the originals once this spec finishes.
🤖 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 `@src/vertex/components/ui/dropdown-menu.test.tsx` around lines 24 - 27, The JSDOM shims in dropdown-menu.test.tsx are mutating global browser APIs for the whole worker, so move them into test setup/teardown and restore the original values after the spec runs. Update the dropdown menu test suite to capture the existing PointerEvent, scrollIntoView, hasPointerCapture, and releasePointerCapture implementations before overriding them, apply the mocks in the suite setup, and cleanly restore them in teardown so later tests are unaffected.src/vertex/components/ui/multi-select.test.tsx-10-17 (1)
10-17: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMake the
PointerEventshim accept an optional init object.
new PointerEvent(type)is valid, but this mock requirespropsand dereferences it immediately. Default it to{}so tests don’t fail on an argument shape the browser allows.Suggested fix
class MockPointerEvent extends Event { button: number; ctrlKey: boolean; pointerType: string; - constructor(type: string, props: PointerEventInit) { + constructor(type: string, props: PointerEventInit = {}) { super(type, props); - this.button = props.button || 0; - this.ctrlKey = props.ctrlKey || false; - this.pointerType = props.pointerType || "mouse"; + this.button = props.button ?? 0; + this.ctrlKey = props.ctrlKey ?? false; + this.pointerType = props.pointerType ?? "mouse"; } }🤖 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 `@src/vertex/components/ui/multi-select.test.tsx` around lines 10 - 17, The PointerEvent shim in MockPointerEvent is too strict because its constructor requires a props object and immediately reads from it, but native PointerEvent allows new PointerEvent(type) with no init. Update the MockPointerEvent constructor to accept an optional init object by defaulting the parameter to an empty object, and keep the existing field assignments for button, ctrlKey, and pointerType using that fallback so the multi-select tests don’t break on valid browser-like usage.src/vertex/components/ui/stepper.test.tsx-6-22 (1)
6-22: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe test never proves
indexaffects rendering.Right now it only checks that child text exists, so it would still pass if
Stepperignoredindexcompletely. Please add at least one assertion that changes with the active step state.🤖 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 `@src/vertex/components/ui/stepper.test.tsx` around lines 6 - 22, The Stepper test only verifies that the child content renders and does not confirm that the active step changes with the index prop. Update the test in stepper.test.tsx around Stepper and Step so it asserts a state that depends on index, such as the active/current step styling or aria state for the step at index 1 versus the others, using the rendered Stepper output to prove the prop affects rendering.src/vertex/components/ui/select-field.test.tsx-20-23 (1)
20-23: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the emitted value, not just that a callback happened.
toHaveBeenCalled()will still pass ifSelectFieldsends the wrong payload shape or wrong selected value. Since this component’s contract isevent.target.value, please assert the first call includes"1"so the test actually protects theonChangeAPI.Possible tightening
- expect(handleChange).toHaveBeenCalled(); + expect(handleChange).toHaveBeenCalledWith( + expect.objectContaining({ + target: expect.objectContaining({ value: "1" }), + }) + );🤖 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 `@src/vertex/components/ui/select-field.test.tsx` around lines 20 - 23, The SelectField test only checks that the change handler was called, which can miss an incorrect payload shape or value; update the assertion in the test around the userEvent.click flow to verify the first onChange call receives the expected event.target.value of "1". Use the existing handleChange mock in src/vertex/components/ui/select-field.test.tsx and tighten the expectation so it validates the SelectField contract instead of just invocation.src/vertex/components/ui/sonner.test.tsx-10-12 (1)
10-12: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThis assertion doesn't verify the
Toasterrendered anything.
render()always gives you a container attached to the document, so this test still passes ifToasterreturnsnull. Please assert for the rendered toaster node itself (for example the.toasterelement wired bysrc/vertex/components/ui/sonner.tsx).Possible tightening
it("renders without crashing", () => { const { container } = render(<Toaster />); - expect(container).toBeInTheDocument(); + expect(container.querySelector(".toaster")).toBeInTheDocument(); });🤖 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 `@src/vertex/components/ui/sonner.test.tsx` around lines 10 - 12, The current test only checks the container from render(), so it can pass even if Toaster renders nothing. Update the Toaster test in sonner.test.tsx to assert on the actual toaster node produced by the Toaster component from sonner.tsx, such as the .toaster element or another unique rendered selector, instead of checking container presence.
🧹 Nitpick comments (6)
src/vertex/components/shared/table/ReusableTable.test.tsx (1)
23-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the test fixture instead of casting
columnstonever.
That cast sidesteps theReusableTablecolumn contract and can let this test stay green even if the public API breaks. Use a typedTableColumn<{ id: number; name: string }>fixture (orsatisfies) here instead.🤖 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 `@src/vertex/components/shared/table/ReusableTable.test.tsx` around lines 23 - 31, The test fixture in ReusableTable.test.tsx is bypassing the column contract by casting columns to never, which can hide API breakage. Replace the cast with a properly typed TableColumn fixture for the { id: number; name: string } shape, or use satisfies to validate the columns object against ReusableTable’s expected column type while keeping the test data aligned.src/vertex/components/ui/combobox.test.tsx (1)
6-27: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRestore these DOM shims after the test.
Lines 18-27 mutate
windowandHTMLElement.prototypefor the whole worker, so later UI tests can become order-dependent. Save the originals and restore them in teardown, or move the shims into shared test setup.🤖 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 `@src/vertex/components/ui/combobox.test.tsx` around lines 6 - 27, The DOM shims added in the combobox test setup mutate global browser APIs for the whole worker, which can make later tests order-dependent. In the combobox test file, save the original values for PointerEvent, scrollIntoView, hasPointerCapture, releasePointerCapture, and ResizeObserver before overriding them, then restore them in teardown (for example in afterEach/afterAll); alternatively, move these shims into a shared test setup so they are applied consistently across tests.src/vertex/components/ui/banner.test.tsx (1)
7-10: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the wrapper-specific severity contract.
These assertions only prove shared
Bannerbehavior. IfSuccessBannerorInfoBannerforwarded the wrongseverity, both tests would still pass. Add one expectation that distinguishes each wrapper from a plainBanner.Also applies to: 13-18
🤖 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 `@src/vertex/components/ui/banner.test.tsx` around lines 7 - 10, The wrapper tests for SuccessBanner and InfoBanner only verify shared Banner rendering, so they miss the severity contract on each wrapper. Update the tests around SuccessBanner and InfoBanner to assert the forwarded severity differs from a plain Banner by checking the rendered severity-specific behavior or prop-driven output. Use the wrapper component names SuccessBanner and InfoBanner, and the shared Banner component, to add one distinguishing expectation per test that would fail if the wrapper passed the wrong severity.src/vertex/components/ui/progress.test.tsx (1)
6-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the
Progressoutput, not the render container.
containeralways exists afterrender(), so this spec won't catch a brokenvaluemapping. A DOM assertion tied to the rendered progress state would give this test real signal.🤖 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 `@src/vertex/components/ui/progress.test.tsx` around lines 6 - 8, The current `Progress` test only checks that `render()` returns a container, which won’t validate the component’s `value` behavior. Update the spec in `progress.test.tsx` to assert the rendered `Progress` output itself, using the `Progress` component and its state/value mapping rather than `container`, so the test fails if the displayed progress is incorrect.src/vertex/components/ui/scroll-area.test.tsx (1)
5-9: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winScope the
ResizeObservershim to the test lifecycle.This mutates a global at module load and never restores it, so later tests can inherit the stub accidentally. Prefer a per-test/per-suite stub with cleanup (
vi.stubGlobalor save/restore the original) to keep the test worker isolated.🤖 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 `@src/vertex/components/ui/scroll-area.test.tsx` around lines 5 - 9, The `ResizeObserver` shim is being assigned to `window` at module load in `scroll-area.test.tsx`, which leaves a global stub behind for other tests. Move this setup into the test lifecycle around the affected suite (for example in the same test file near the `ResizeObserver` usage) and restore the original global afterward, or use `vi.stubGlobal` with cleanup in `afterEach`/`afterAll` so the worker stays isolated.src/vertex/components/ui/select.test.tsx (1)
25-28: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRestore these DOM shims after the test.
These prototype/global patches leak beyond this file and can mask failures in other Radix tests. Please scope them with setup/teardown rather than assigning them once at module load.
🤖 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 `@src/vertex/components/ui/select.test.tsx` around lines 25 - 28, The DOM shims in the select test are being applied at module load and leak into other tests; move the window.PointerEvent and HTMLElement.prototype patches into test setup/teardown so they are scoped per test. Use the select test’s existing fixture hooks (for example around the select component tests) to save the original values before patching and restore them after each test, ensuring the global/prototype changes do not persist beyond this file.
🤖 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 `@src/vertex/app/changelog.md`:
- Line 24: Update the changelog entry in the Core UI Components section to avoid
the unverifiable “full coverage” claim; in the changelog markdown, revise the
wording near the Core UI Components bullet so it accurately reflects the work as
comprehensive tests for 30+ shadcn/radix UI components rather than exhaustive
coverage.
- Around line 39-52: The changelog entry is inconsistent with the Vitest setup
path referenced by vitest.config.ts. Update src/vertex/app/changelog.md so the
new setup file path matches the actual setup file used by setupFiles in
vitest.config.ts, or adjust the config reference if the intended file is
src/vertex/test/setup.ts. Use the vitest.config.ts setupFiles setting and the
changelog’s new file list as the symbols to reconcile.
In `@src/vertex/components/ui/date-picker.test.tsx`:
- Around line 6-21: Scope the JSDOM shims in date-picker.test.tsx so they do not
leak across the worker: the MockPointerEvent assignment and the HTMLElement
prototype overrides should be saved/restored around the test lifecycle. Update
the test setup near MockPointerEvent and the
window.PointerEvent/scrollIntoView/hasPointerCapture/releasePointerCapture
patches to capture the originals and restore them in teardown (or extract them
into a helper with explicit cleanup). Ensure the cleanup runs after the test
suite so later tests see the real globals.
In `@src/vertex/components/ui/ErrorBoundary.test.tsx`:
- Around line 11-24: The ErrorBoundary test leaves `consoleError` and the window
`"error"` listener active if an assertion fails, which can affect later tests.
Update the `ErrorBoundary.test.tsx` flow so the setup and teardown around
`vi.spyOn(console, "error")`, `window.addEventListener("error", preventError)`,
and `window.removeEventListener("error", preventError)` always run in guaranteed
cleanup, such as a `try/finally` or test hooks, while keeping the `render` and
assertion for `ThrowError` inside the protected body.
In `@src/vertex/components/ui/hcaptcha-widget.test.tsx`:
- Around line 12-15: The current test in HCaptchaWidget is only asserting on
Testing Library’s container, so it will pass even if the widget renders nothing.
Update the test around HCaptchaWidget to verify a concrete captcha render
instead, preferably by mocking the hCaptcha component and asserting it is
rendered with the expected props for the enabled/site-key path. Use the
HCaptchaWidget and the mocked hCaptcha component symbols to locate and replace
the ineffective container-based assertion.
In `@src/vertex/components/ui/popover.test.tsx`:
- Around line 2-21: The global DOM shims added in the popover test setup are not
being restored, so they leak into later tests. In the popover test suite,
capture the original values for window.PointerEvent and the patched
HTMLElement.prototype methods before overriding them, then restore them in an
afterEach or afterAll cleanup within the same test file. Keep the fix localized
to the popover test helpers so the behavior of Popover, PopoverContent, and
PopoverTrigger stays unchanged while preventing cross-test pollution.
In `@src/vertex/core/hooks/useContextAwareRouting.ts`:
- Around line 29-36: The base-route lookup in useContextAwareRouting is only
using the first path segment, so nested routes under multi-segment parents are
not matching the intended sidebar config. Update the route matching logic around
basePath/baseRoute to derive the correct configured parent for routes like
/admin/networks/requests/42 and /devices/my-devices/123, then use that key to
check sidebarConfig instead of falling back to true. Keep the fix localized to
the useContextAwareRouting hook and its routeToSidebarConfig lookup.
In `@src/vertex/core/services/network-service.test.ts`:
- Around line 66-120: The negative-path tests in
networkService.updateNetworkRequestStatus and
networkService.submitNetworkRequest only assert inside catch blocks, so they can
pass even if the promise resolves. Add an explicit failure path after each
awaited call (for example, in the try block or immediately after the await) so
the test fails when the request unexpectedly succeeds, while keeping the
existing error assertions for the catch path.
In `@src/vertex/core/utils/platform.test.ts`:
- Around line 19-28: The test for getMacArchitecture mutates global.window and
only restores it after the await/assertion, so a failure can leave the global
deleted and break later cleanup. Update this test case to wrap the deletion and
assertion in a try/finally block so global.window is always restored, even if
getMacArchitecture or expect throws.
In `@src/vertex/core/utils/secureApiProxyClient.test.ts`:
- Around line 82-96: Prime the cached auth state in secureApiProxyClient.test.ts
before exercising getResponseError(), since this test currently verifies 401
invalidation without first storing a token. Use the existing hasValidToken and
interceptors.getResponseError symbols to set up a valid token/session state
before calling the interceptor, then assert that the 401 path clears it by
checking hasValidToken() becomes false after the auth-token-expired event is
dispatched.
In `@src/vertex/lib/api-routing.test.ts`:
- Around line 32-42: Isolate the resolveApiOrigin precedence tests from ambient
environment variables so the cases don’t inherit host values like API_BASE_URL
or NEXT_PUBLIC_*. Update the envCases loop in api-routing.test.ts to clear the
routing-related keys before applying each test env, or temporarily replace
process.env with only the vars under test, so resolveApiOrigin() is validated
against just the intended precedence inputs.
---
Outside diff comments:
In `@src/vertex/components/shared/table/ReusableTable.tsx`:
- Around line 1475-1479: The scrollbar suppression rule is leaking globally
because the plain style block makes .scrollbar-hide apply outside ReusableTable.
Update the style usage in ReusableTable to be component-scoped again by
switching back to style jsx, keeping the scrollbar-hiding rule local to this
table only. Use the existing ReusableTable style block and the .scrollbar-hide
selector as the place to fix.
---
Minor comments:
In `@src/vertex/app/changelog.md`:
- Around line 15-16: Update the changelog entry in changelog.md to avoid
implying a migration: the phrase in the “Vitest & React Testing Library” bullet
should not say “Replaced legacy test runners.” Instead, revise it to reflect
that Vitest and React Testing Library were added alongside the existing test
infrastructure. Keep the “CI/CD Pipeline” bullet unchanged unless needed for
wording consistency.
In `@src/vertex/components/shared/toast/ReusableToast.test.tsx`:
- Around line 7-10: The Toaster test currently only asserts that render()
returned a container, which does not verify any toast behavior. Update
ReusableToast.test.tsx to trigger an actual toast through the shared toast API
used by the Toaster component, then assert the expected toast message is
rendered. Keep the test focused on verifying visible toast output rather than
the render container in the Toaster component test.
In `@src/vertex/components/ui/dialog.test.tsx`:
- Around line 47-55: The dialog test in dialog.test.tsx only verifies opening
and never checks the close behavior, so update the dialog interaction test
around the “Open Dialog” and “Close” clicks to assert the dialog is removed
after closing. Use the existing dialog query with the “dialog” role and add a
post-click expectation after the Close action so the test fails if the dialog
remains visible.
In `@src/vertex/components/ui/dropdown-menu.test.tsx`:
- Around line 24-27: The JSDOM shims in dropdown-menu.test.tsx are mutating
global browser APIs for the whole worker, so move them into test setup/teardown
and restore the original values after the spec runs. Update the dropdown menu
test suite to capture the existing PointerEvent, scrollIntoView,
hasPointerCapture, and releasePointerCapture implementations before overriding
them, apply the mocks in the suite setup, and cleanly restore them in teardown
so later tests are unaffected.
In `@src/vertex/components/ui/multi-select.test.tsx`:
- Around line 10-17: The PointerEvent shim in MockPointerEvent is too strict
because its constructor requires a props object and immediately reads from it,
but native PointerEvent allows new PointerEvent(type) with no init. Update the
MockPointerEvent constructor to accept an optional init object by defaulting the
parameter to an empty object, and keep the existing field assignments for
button, ctrlKey, and pointerType using that fallback so the multi-select tests
don’t break on valid browser-like usage.
In `@src/vertex/components/ui/select-field.test.tsx`:
- Around line 20-23: The SelectField test only checks that the change handler
was called, which can miss an incorrect payload shape or value; update the
assertion in the test around the userEvent.click flow to verify the first
onChange call receives the expected event.target.value of "1". Use the existing
handleChange mock in src/vertex/components/ui/select-field.test.tsx and tighten
the expectation so it validates the SelectField contract instead of just
invocation.
In `@src/vertex/components/ui/sonner.test.tsx`:
- Around line 10-12: The current test only checks the container from render(),
so it can pass even if Toaster renders nothing. Update the Toaster test in
sonner.test.tsx to assert on the actual toaster node produced by the Toaster
component from sonner.tsx, such as the .toaster element or another unique
rendered selector, instead of checking container presence.
In `@src/vertex/components/ui/stepper.test.tsx`:
- Around line 6-22: The Stepper test only verifies that the child content
renders and does not confirm that the active step changes with the index prop.
Update the test in stepper.test.tsx around Stepper and Step so it asserts a
state that depends on index, such as the active/current step styling or aria
state for the step at index 1 versus the others, using the rendered Stepper
output to prove the prop affects rendering.
In `@src/vertex/core/hooks/useRecentlyVisited.test.tsx`:
- Around line 36-69: The `useRecentlyVisited` tests are asserting `visitedPages`
too early, before the hook’s `useEffect`-driven state updates have completed.
Update the specs in `useRecentlyVisited.test.tsx` to wait for the hook to settle
after `renderHookWithProviders(...)`, especially around the `visitedPages`
checks in the two affected test cases. Use `waitFor(...)` around the
expectations so the assertions run after `useRecentlyVisited` finishes loading
and rerendering.
In `@src/vertex/test/mocks/nextAuth.ts`:
- Around line 13-28: The createMockSession helper is shallow-merging overrides,
so a partial user override replaces the default user object instead of merging
into it. Update createMockSession in nextAuth.ts to deep-merge the nested user
field by combining the default user object with overrides.user while still
allowing top-level overrides like expires and accessToken to apply normally.
In `@src/vertex/test/utils/renderHookWithProviders.tsx`:
- Around line 8-14: The renderHookWithProviders helper is letting a
caller-supplied renderOptions.wrapper override the internal provider wrapper,
which removes the Redux and Query providers. Update renderHookWithProviders so
wrapper cannot be replaced from the public options, or explicitly compose any
external wrapper with the existing Wrapper used for providers. Keep the fix
centered on renderHookWithProviders and the Wrapper/renderOptions spread so the
providers are always preserved.
In `@src/vertex/test/utils/renderWithProviders.tsx`:
- Around line 14-17: `ExtendedRenderOptions` currently still inherits `wrapper`,
and `renderWithProviders` lets the caller-supplied wrapper override the built-in
`Wrapper`, which can bypass the Redux and React Query providers. Update
`ExtendedRenderOptions` to exclude `wrapper` and keep `Wrapper` authoritative
inside `renderWithProviders` so tests always use the app-style provider setup.
---
Nitpick comments:
In `@src/vertex/components/shared/table/ReusableTable.test.tsx`:
- Around line 23-31: The test fixture in ReusableTable.test.tsx is bypassing the
column contract by casting columns to never, which can hide API breakage.
Replace the cast with a properly typed TableColumn fixture for the { id: number;
name: string } shape, or use satisfies to validate the columns object against
ReusableTable’s expected column type while keeping the test data aligned.
In `@src/vertex/components/ui/banner.test.tsx`:
- Around line 7-10: The wrapper tests for SuccessBanner and InfoBanner only
verify shared Banner rendering, so they miss the severity contract on each
wrapper. Update the tests around SuccessBanner and InfoBanner to assert the
forwarded severity differs from a plain Banner by checking the rendered
severity-specific behavior or prop-driven output. Use the wrapper component
names SuccessBanner and InfoBanner, and the shared Banner component, to add one
distinguishing expectation per test that would fail if the wrapper passed the
wrong severity.
In `@src/vertex/components/ui/combobox.test.tsx`:
- Around line 6-27: The DOM shims added in the combobox test setup mutate global
browser APIs for the whole worker, which can make later tests order-dependent.
In the combobox test file, save the original values for PointerEvent,
scrollIntoView, hasPointerCapture, releasePointerCapture, and ResizeObserver
before overriding them, then restore them in teardown (for example in
afterEach/afterAll); alternatively, move these shims into a shared test setup so
they are applied consistently across tests.
In `@src/vertex/components/ui/progress.test.tsx`:
- Around line 6-8: The current `Progress` test only checks that `render()`
returns a container, which won’t validate the component’s `value` behavior.
Update the spec in `progress.test.tsx` to assert the rendered `Progress` output
itself, using the `Progress` component and its state/value mapping rather than
`container`, so the test fails if the displayed progress is incorrect.
In `@src/vertex/components/ui/scroll-area.test.tsx`:
- Around line 5-9: The `ResizeObserver` shim is being assigned to `window` at
module load in `scroll-area.test.tsx`, which leaves a global stub behind for
other tests. Move this setup into the test lifecycle around the affected suite
(for example in the same test file near the `ResizeObserver` usage) and restore
the original global afterward, or use `vi.stubGlobal` with cleanup in
`afterEach`/`afterAll` so the worker stays isolated.
In `@src/vertex/components/ui/select.test.tsx`:
- Around line 25-28: The DOM shims in the select test are being applied at
module load and leak into other tests; move the window.PointerEvent and
HTMLElement.prototype patches into test setup/teardown so they are scoped per
test. Use the select test’s existing fixture hooks (for example around the
select component tests) to save the original values before patching and restore
them after each test, ensuring the global/prototype changes do not persist
beyond this file.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b16e457-a666-4f85-9fe9-2b08223dfe8b
📒 Files selected for processing (87)
src/vertex/app/changelog.mdsrc/vertex/components/shared/button/ReusableButton.test.tsxsrc/vertex/components/shared/card/CardWrapper.test.tsxsrc/vertex/components/shared/fileupload/ReusableFileUpload.test.tsxsrc/vertex/components/shared/inputfield/ReusableInputField.test.tsxsrc/vertex/components/shared/select/ReusableSelectInput.test.tsxsrc/vertex/components/shared/table/ReusableTable.test.tsxsrc/vertex/components/shared/table/ReusableTable.tsxsrc/vertex/components/shared/toast/ReusableToast.test.tsxsrc/vertex/components/ui/ErrorBoundary.test.tsxsrc/vertex/components/ui/accordion.test.tsxsrc/vertex/components/ui/alert-dialog.test.tsxsrc/vertex/components/ui/alert.test.tsxsrc/vertex/components/ui/avatar.test.tsxsrc/vertex/components/ui/badge.test.tsxsrc/vertex/components/ui/banner.test.tsxsrc/vertex/components/ui/button.test.tsxsrc/vertex/components/ui/calendar.test.tsxsrc/vertex/components/ui/card.test.tsxsrc/vertex/components/ui/chart.test.tsxsrc/vertex/components/ui/checkbox.test.tsxsrc/vertex/components/ui/collapsible.test.tsxsrc/vertex/components/ui/combobox.test.tsxsrc/vertex/components/ui/command.test.tsxsrc/vertex/components/ui/custom-dialog.test.tsxsrc/vertex/components/ui/date-picker.test.tsxsrc/vertex/components/ui/dialog.test.tsxsrc/vertex/components/ui/dropdown-menu.test.tsxsrc/vertex/components/ui/empty-state.test.tsxsrc/vertex/components/ui/forbidden-error.test.tsxsrc/vertex/components/ui/forbidden-page.test.tsxsrc/vertex/components/ui/form.test.tsxsrc/vertex/components/ui/hcaptcha-widget.test.tsxsrc/vertex/components/ui/hover-card.test.tsxsrc/vertex/components/ui/input.test.tsxsrc/vertex/components/ui/label.test.tsxsrc/vertex/components/ui/multi-select.test.tsxsrc/vertex/components/ui/pagination.test.tsxsrc/vertex/components/ui/permission-tooltip.test.tsxsrc/vertex/components/ui/popover.test.tsxsrc/vertex/components/ui/progress.test.tsxsrc/vertex/components/ui/scroll-area.test.tsxsrc/vertex/components/ui/select-field.test.tsxsrc/vertex/components/ui/select.test.tsxsrc/vertex/components/ui/separator.test.tsxsrc/vertex/components/ui/sheet.test.tsxsrc/vertex/components/ui/skeleton.test.tsxsrc/vertex/components/ui/sonner.test.tsxsrc/vertex/components/ui/stepper.test.tsxsrc/vertex/components/ui/switch.test.tsxsrc/vertex/components/ui/table.test.tsxsrc/vertex/components/ui/tabs.test.tsxsrc/vertex/components/ui/textarea.test.tsxsrc/vertex/components/ui/toast.test.tsxsrc/vertex/components/ui/toaster.test.tsxsrc/vertex/components/ui/tooltip.test.tsxsrc/vertex/core/config/proxyConfig.test.tssrc/vertex/core/config/vertex-config.test.tssrc/vertex/core/hooks/useBannerWithDelay.test.tsxsrc/vertex/core/hooks/useClipboard.test.tsxsrc/vertex/core/hooks/useCohorts.test.tsxsrc/vertex/core/hooks/useContextAwareRouting.test.tssrc/vertex/core/hooks/useContextAwareRouting.tssrc/vertex/core/hooks/useDevices.test.tsxsrc/vertex/core/hooks/useGroups.test.tsxsrc/vertex/core/hooks/usePermissions.test.tsxsrc/vertex/core/hooks/useRecentlyVisited.test.tsxsrc/vertex/core/hooks/useRoles.test.tsxsrc/vertex/core/hooks/useServerSideTableState.test.tsxsrc/vertex/core/hooks/useSites.test.tsxsrc/vertex/core/hooks/useUserContext.test.tsxsrc/vertex/core/hooks/useWindow.test.tsxsrc/vertex/core/services/network-service.test.tssrc/vertex/core/services/onboardingService.test.tssrc/vertex/core/utils/platform.test.tssrc/vertex/core/utils/proxyClient.test.tssrc/vertex/core/utils/secureApiProxyClient.test.tssrc/vertex/lib/api-routing.test.tssrc/vertex/lib/utils.test.tssrc/vertex/test/factories/apiResponseFactory.tssrc/vertex/test/factories/deviceFactory.tssrc/vertex/test/factories/organizationFactory.tssrc/vertex/test/factories/userFactory.tssrc/vertex/test/mocks/nextAuth.tssrc/vertex/test/mocks/queryClient.tssrc/vertex/test/utils/renderHookWithProviders.tsxsrc/vertex/test/utils/renderWithProviders.tsx
Extract repeated JSDOM/window mock setup into a new test utility (src/vertex/test/utils/domMocks.ts) exposing setupPointerEventMock and setupResizeObserverMock. Replace inline MockPointerEvent/ResizeObserver and HTMLElement overrides across multiple UI tests (chart, combobox, date-picker, dialog, dropdown-menu, multi-select, popover, select) with imports and calls to the new utilities. The helpers install mocks in beforeAll and restore originals in afterAll to keep tests isolated and reduce duplication.
Wrap render and assertions in a try/finally block so window error listener removal and console.error mock restoration always run. This prevents leaking global state and makes the ErrorBoundary test more robust if render or assertions throw.
|
New azure vertex changes available for preview here |
|
New azure vertex changes available for preview here |
Description
This pull request drastically expands our frontend test suite, introducing extensive unit test coverage for our custom React hooks, shared components, shadcn UI elements, and API service boundaries. These changes ensure our core rendering and network mechanisms are highly robust and validated offline without relying on staging backend services. (Note: The core Vitest infrastructure setup and CI/CD automation were addressed in a previous PR.)
Key Changes
1. UI & Component Testing
Dialog,Tooltip,Select,Button,Accordion, etc.), verifying user-event interactions, state changes, and accessibility-compliant rendering.ReusableTable,ReusableSelectInput,ReusableButton, andReusableFileUpload. Removed obsoletedefaultPropsinReusableTablefor strict mode compliance.2. API Boundary & Mock Infrastructure
apiResponseFactory.tsandnextAuth.tsmocks to simulate full HTTP/Auth environments natively, removing external mock adapter dependencies.createProxyHandler(App Router) andcreateSecureApiClient(Axios interceptors), asserting accurate token injections, network degradation error handling, and 401 session-clear behaviors.network-service.ts,onboardingService.ts, and core utilities.3. Redux & Hook Architecture
store.tsto cleanly usecombineReducers, allowing test suites to spin up isolated Redux trees and reset state reliably.Commits Included
d845948Update changelog.mddf6784aAdd tests for network, onboarding, proxy, platform1a27e89Update tests and remove ReusableTable defaultProps96acc8eAdd unit tests for UI componentsec983b1Add tests for shared and UI components7be62caAssert info severity in banner testb69c38aUse combineReducers to create root reducer8ce1defAdd hook tests and test utilitiesVerification
npm run testto execute the 250+ new unit tests. Ensure a 100% pass rate.Summary by CodeRabbit