Add component testing infrastructure for www app#146
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Co-authored-by: yamcodes <2014360+yamcodes@users.noreply.github.com>
Co-authored-by: yamcodes <2014360+yamcodes@users.noreply.github.com>
github-actions
bot
added
www
|
Notice we're using Vitest Projects in this monorepo setup @copilot |
Yes, exactly! The implementation follows the Vitest Projects pattern where each package/app has its own vitest config with a unique name ( |
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds extensive component tests and testing infrastructure for apps/www, updates Vitest configs and scripts, tweaks linting and TS types for tests, switches Vite React plugin to the SWC variant across multiple packages, adjusts .gitignore, and makes a minor test-only attribute addition to EditOnGithub. Removes one obsolete test and a commented snippet. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Note Free review on us!CodeRabbit is offering free reviews until Wed Oct 08 2025 to showcase some of the refinements we've made. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Updated pnpm version in package.json - Updated dependencies in pnpm-lock.yaml
- Changed project paths to use glob patterns for better scalability. - Enhanced coverage configuration with additional reporters and exclusions. - Updated app-specific Vitest config to use `defineProject` and modified setup file path. - Removed obsolete setup file for tests in the www app.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Replaced instances of @vitejs/plugin-react with @vitejs/plugin-react-swc across multiple package.json and config files. - Updated pnpm-lock.yaml to reflect the new plugin version and dependencies. - Removed deprecated Babel plugins related to React JSX transformation.
github-actions
bot
added
@arkenv/vite-plugin
commit: |
- Added new dependencies: @swc/plugin-styled-jsx, @testing-library/dom, styled-jsx, and vite-tsconfig-paths in package.json files. - Updated pnpm-lock.yaml to include new versions and resolutions for added dependencies. - Modified Vitest configuration to integrate vite-tsconfig-paths for better path resolution.
- Updated testing strategy in TESTING.md to emphasize component behavior and accessibility. - Added new tests for EditOnGithub and SailButton components, ensuring correct rendering and functionality. - Improved existing tests for CopyButton and StarUsButton, focusing on accessibility and user interactions. - Introduced new test files for VideoDemo and Heading components, covering various scenarios and edge cases. - Updated biome.jsonc to include linter rules for test files.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
- Add test:ui and test:coverage scripts - Configure coverage in .gitignore - Install @vitest/coverage-v8 and @vitest/ui
- Added restoration of document.getElementById mock in afterEach to prevent test pollution.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
examples/with-vite-react-ts/package.json (1)
22-22: Verify plugin version compatibility with Vite 7 and React 19.Similar to other files in this PR, the change to
@vitejs/plugin-react-swcv4.1.0 may represent a version downgrade. Ensure the plugin version is compatible with Vite ^7.1.5 and React ^19.0.0.apps/playgrounds/vite/package.json (1)
21-21: Verify plugin version compatibility with Vite 7 and React 19.Same concern as other files: ensure
@vitejs/plugin-react-swcv4.1.0 is compatible with Vite ^7.1.9 and React ^19.2.0.apps/www/components/page/logo.test.tsx (1)
6-8: Potential redundant cleanup call.Similar to other test files, this manual cleanup may be unnecessary if global cleanup is configured in the test setup.
apps/www/components/page/edit-on-github.test.tsx (1)
54-56: Potential redundant cleanup call.apps/www/components/page/video-demo.test.tsx (1)
18-20: Potential redundant cleanup call.
🧹 Nitpick comments (10)
apps/www/components/page/sail-button.test.tsx (2)
35-44: Strengthen animation verification.The test verifies that the link remains in the document after click, but doesn't validate the actual animation state. Consider checking for animation-related classes or state changes if the component exposes them.
If the component has observable animation states (e.g., CSS classes, data attributes), add assertions like:
await user.click(link); -// The button should still be rendered (animation state change) -expect(link).toBeInTheDocument(); +// Verify animation state is triggered +expect(link).toHaveClass("animating"); // or relevant animation class +expect(link).toBeInTheDocument();
82-86: Consider icon-specific verification.The test only verifies text content, not the actual Sailboat icon component. If icon rendering is critical, consider using a data-testid on the icon similar to the SquarePen icon approach in edit-on-github.tsx.
apps/www/components/page/copy-button.test.tsx (1)
48-52: Strengthen prop forwarding test.The test doesn't actually verify that additional props are forwarded. Consider passing a specific prop and asserting its presence.
Apply this diff to properly test prop forwarding:
it("forwards additional props", () => { - render(<CopyButton command="npm install arkenv" />); + render(<CopyButton command="npm install arkenv" data-testid="copy-btn" />); const button = screen.getByRole("button"); - expect(button).toBeInTheDocument(); + expect(button).toHaveAttribute("data-testid", "copy-btn"); });apps/www/hooks/use-is-mobile.test.ts (1)
63-90: Strengthen media query change test.The test doesn't actually verify that the hook responds to media query changes. Consider simulating a query change event and verifying the hook's response.
To properly test media query changes:
it("handles media query changes", () => { - // Mock window.innerWidth for desktop Object.defineProperty(window, "innerWidth", { writable: true, configurable: true, value: 1024, }); + let changeHandler: ((event: MediaQueryListEvent) => void) | null = null; mockMatchMedia.mockReturnValue({ matches: false, media: "(max-width: 768px)", onchange: null, addListener: vi.fn(), removeListener: vi.fn(), - addEventListener: vi.fn(), + addEventListener: vi.fn((_, handler) => { + changeHandler = handler as (event: MediaQueryListEvent) => void; + }), removeEventListener: vi.fn(), dispatchEvent: vi.fn(), }); const { result, unmount } = renderHook(() => useIsMobile()); + expect(result.current).toBe(false); - // The hook should render without errors and return a boolean - expect(typeof result.current).toBe("boolean"); + // Simulate media query change to mobile + if (changeHandler) { + changeHandler({ matches: true } as MediaQueryListEvent); + } + + // Verify hook updates to mobile state + expect(result.current).toBe(true); unmount(); - // Test passes if no errors are thrown - expect(true).toBe(true); });apps/www/components/page/star-us-button.test.tsx (1)
5-6: Enhance fetch mock in tests
Keep mockingglobal.fetchto avoid real API calls, but stub its implementation (e.g.mockResolvedValue({ ok: true, json: () => Promise.resolve({ stargazers_count: 42 }) })), then add assertions:
expect(global.fetch).toHaveBeenCalledWith(\https://api.github.com/repos/${owner}/${repo}\`);`expect(await screen.findByText('42')).toBeInTheDocument();apps/www/components/ui/button.test.tsx (2)
7-9: Potential redundant cleanup call.The manual
cleanup()call inafterEachmay be redundant ifapps/www/tests/setup.tsalready configures global cleanup. Per Testing Library's documentation, cleanup is automatically handled when using modern versions with proper setup.Verify if the global setup already handles cleanup:
#!/bin/bash # Check if global cleanup is configured in the test setup file cat apps/www/tests/setup.ts | grep -i cleanup
83-93: Consider verifying variant and size styling.While these tests confirm the component renders with variant/size props, they don't verify that the correct styling classes are applied. This could miss bugs where props are accepted but don't affect rendering.
If you want stronger assertions, consider checking for specific classes:
it("supports different variants through props", () => { render(<Button variant="outline">Outline</Button>); const button = screen.getByRole("button"); expect(button).toBeInTheDocument(); + expect(button).toHaveClass("variant-outline"); // or whatever class the variant applies });However, if you're following the "test behavior, not styling" philosophy from TESTING.md, the current approach is acceptable.
apps/www/components/page/video-demo.test.tsx (3)
14-15: Redundant mock clearing.Line 15's
mockWindowOpen.mockClear()is unnecessary because line 14'svi.clearAllMocks()already clears all mocks, includingmockWindowOpen.Apply this diff:
beforeEach(() => { vi.clearAllMocks(); - mockWindowOpen.mockClear(); });
30-47: Consider consolidating duplicate video attribute tests.Lines 30-47, 157-168, and 170-178 test similar or identical video attributes. This creates maintenance burden without adding value.
Consider consolidating these into a single comprehensive test or removing the duplicate assertions to keep tests DRY.
Also applies to: 157-168, 170-178
108-120: Redundant StackBlitz URL verification tests.Lines 108-120 and 122-135 test the exact same behavior (clicking opens the correct StackBlitz URL). The second test adds no additional coverage.
Consider removing one of these tests or differentiating them (e.g., test different file parameter values if that's parameterizable).
Also applies to: 122-135
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/with-vite-react-ts/package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.gitignore(1 hunks)TESTING.md(1 hunks)apps/playgrounds/vite/package.json(1 hunks)apps/playgrounds/vite/vite.config.ts(1 hunks)apps/www/components/page/copy-button.test.tsx(1 hunks)apps/www/components/page/edit-on-github.test.tsx(1 hunks)apps/www/components/page/edit-on-github.tsx(1 hunks)apps/www/components/page/logo.test.tsx(1 hunks)apps/www/components/page/sail-button.test.tsx(1 hunks)apps/www/components/page/star-us-button.test.tsx(1 hunks)apps/www/components/page/video-demo.test.tsx(1 hunks)apps/www/components/ui/button.test.tsx(1 hunks)apps/www/components/ui/heading.test.tsx(1 hunks)apps/www/hooks/use-is-mobile.test.ts(1 hunks)apps/www/lib/source.tsx(0 hunks)apps/www/lib/utils/cn.test.ts(0 hunks)apps/www/package.json(2 hunks)apps/www/tests/README.md(1 hunks)apps/www/tests/setup.ts(1 hunks)apps/www/tsconfig.json(1 hunks)apps/www/vitest.config.ts(1 hunks)biome.jsonc(1 hunks)examples/with-vite-react-ts/package.json(1 hunks)examples/with-vite-react-ts/vite.config.ts(1 hunks)package.json(2 hunks)packages/vite-plugin/package.json(1 hunks)vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/www/lib/source.tsx
- apps/www/lib/utils/cn.test.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-12T06:23:45.463Z
Learnt from: CR
PR: yamcodes/arkenv#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T06:23:45.463Z
Learning: Applies to **/*.test.ts : In tests, save and restore process.env in setup/teardown (beforeEach/afterEach) to avoid leakage between cases
Applied to files:
apps/www/tests/setup.ts
📚 Learning: 2025-09-12T06:23:45.463Z
Learnt from: CR
PR: yamcodes/arkenv#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T06:23:45.463Z
Learning: Applies to **/*.test.ts : Use Vitest's describe/it structure in tests
Applied to files:
apps/www/tests/setup.tsTESTING.md
⏰ 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: pkg-pr-new
🔇 Additional comments (29)
apps/www/tests/README.md (1)
1-8: LGTM!Clear and concise documentation of the test colocation strategy. The example effectively demonstrates the expected file structure.
.gitignore (1)
39-42: LGTM!Appropriate additions to ignore coverage artifacts generated by the new testing infrastructure and archive files.
apps/playgrounds/vite/package.json (1)
14-14: LGTM!Appropriate addition of the ArkEnv Vite plugin dependency to the playground app.
apps/playgrounds/vite/vite.config.ts (1)
2-2: LGTM!The import change to the SWC-based React plugin is consistent with the package.json update, and the usage pattern remains identical.
examples/with-vite-react-ts/vite.config.ts (1)
2-2: LGTM!Consistent with other Vite configuration updates in this PR.
vitest.config.ts (2)
5-5: LGTM!The switch from an explicit project list to glob patterns
["packages/*", "apps/*"]improves maintainability by automatically including new packages and apps without config updates.
6-30: LGTM!Comprehensive coverage configuration with sensible defaults:
- v8 provider is the modern Vitest standard
- Multiple reporter formats (text, json, html) for different consumption needs
- Extensive and appropriate exclude patterns covering build outputs, test files, config files, examples, type definitions, and static assets
packages/vite-plugin/package.json (1)
17-17: No compatibility issues found for @vitejs/plugin-react-swc v4.1.0. Peer dependencies include Vite ^7, and the plugin does not restrict React versions—compatible with React 19.package.json (1)
17-18: LGTM! Test infrastructure additions align with PR objectives.The new test:ui and test:coverage scripts, along with the corresponding Vitest dependencies, properly support the component testing infrastructure being introduced.
Also applies to: 24-25
apps/www/components/page/star-us-button.test.tsx (1)
14-19: LGTM! Comprehensive test coverage.The test suite provides excellent coverage of StarUsButton functionality:
- Verifies dual rendering (mobile/desktop)
- Validates link attributes (href, target, rel)
- Confirms custom className support
- Ensures proper accessibility attributes
Also applies to: 21-30, 32-37, 39-45, 47-57
apps/www/components/page/sail-button.test.tsx (1)
23-33: LGTM! Thorough click behavior testing.The test suite properly validates:
- Rendering and link attributes
- Different click behaviors (middle, ctrl, meta)
- Accessibility of button text
Also applies to: 46-56, 58-68, 70-80, 88-92
apps/www/components/page/copy-button.test.tsx (1)
22-27: LGTM! Good accessibility and behavior coverage.The tests properly verify:
- Accessibility attributes (aria-label)
- Icon rendering
- Command prop handling
- Button styling (hover classes)
Also applies to: 29-33, 35-39, 41-46
apps/www/hooks/use-is-mobile.test.ts (1)
17-38: LGTM! Solid mobile/desktop detection tests.The tests properly verify the hook's behavior for different screen sizes with correct matchMedia mocking.
Also applies to: 40-61
apps/www/components/ui/heading.test.tsx (2)
16-20: LGTM! Mock cleanup properly implemented.The use of
vi.restoreAllMocks()in the afterEach hook properly addresses the previous concern about test pollution. This ensures document.getElementById and other mocks are fully restored between tests.
22-27: LGTM! Comprehensive Heading component tests.The test suite provides excellent coverage:
- Rendering without/with id
- Custom heading levels (h1, h2, etc.)
- Anchor link functionality and scrolling behavior
- Hash updates
- Custom className and prop forwarding
- Accessibility attributes
Also applies to: 29-36, 38-46, 48-65, 67-80, 82-90, 92-100, 102-107
apps/www/components/page/edit-on-github.tsx (1)
32-32: LGTM! Appropriate test hook.The data-testid attribute enables reliable icon querying in tests without affecting production behavior.
apps/www/tests/setup.ts (1)
1-9: LGTM! Proper test setup configuration.The setup file correctly:
- Extends Vitest's expect with jest-dom matchers for enhanced DOM assertions
- Registers global cleanup after each test to prevent test pollution
This aligns with testing best practices and the learnings about proper test isolation. Based on learnings.
apps/www/tsconfig.json (1)
18-18: LGTM!Adding
@testing-library/jest-domto the types array enables TypeScript to recognize jest-dom matchers globally across all test files, which is the standard approach for this library.apps/www/package.json (2)
11-13: LGTM!The test scripts follow standard Vitest conventions and provide good coverage for different testing scenarios (basic run, UI mode, and coverage reporting).
48-66: LGTM!The testing dependencies are well-chosen and align with modern React testing practices. The SWC-based toolchain (lines 48, 59) provides faster transforms than Babel, and the Testing Library ecosystem (lines 50-53) is the industry standard for React component testing.
apps/www/components/ui/button.test.tsx (1)
11-103: Excellent test coverage!The test suite comprehensively covers the Button component's behavior, including accessibility (keyboard navigation, focus), interaction patterns (clicks, disabled state), composition (asChild), and prop forwarding. This aligns well with the testing philosophy documented in TESTING.md.
apps/www/components/page/logo.test.tsx (2)
17-25: Consider whether CSS class testing aligns with testing philosophy.According to TESTING.md lines 24-25, the testing philosophy states "What We Don't Test: Pure styling or CSS classes." These tests explicitly verify CSS classes like
font-bold,decoration-wavy, etc.If these classes are part of the component's guaranteed API (e.g., consumers rely on them for overrides or specificity), then testing them is reasonable. Otherwise, consider focusing on behavioral aspects instead.
Based on learnings
35-40: Good semantic element verification!Testing that the component renders as a
CODEelement is valuable, as it verifies the semantic HTML structure that affects accessibility and styling context.apps/www/components/page/edit-on-github.test.tsx (1)
1-233: Excellent test coverage with proper mocking strategy!This test suite exemplifies the "mock at component boundaries" principle from TESTING.md. The tests comprehensively cover:
- Path handling variations and edge cases (lines 86-152)
- Error scenarios with proper Sentry integration (lines 171-217)
- Accessibility attributes (lines 68-70, 95-96)
- Environment variable configuration (lines 99-125)
The mocking strategy isolates the component while still testing its integration with utilities like
getLinkTitleAndHref.apps/www/components/page/video-demo.test.tsx (1)
205-220: Excellent error handling test!Testing the graceful handling of popup blockers (when
window.openreturnsnull) is important for resilient user experience. This ensures the component doesn't throw errors in restrictive environments.TESTING.md (2)
13-36: Excellent testing philosophy documentation!The new Component Testing Philosophy section provides clear, actionable guidance that aligns with modern React testing best practices:
- Behavioral focus over implementation details (lines 15-22)
- Accessibility-first testing approach (line 32)
- Smart mocking strategy at component boundaries (line 33)
- Co-location principle for maintainability (line 35)
This guidance will help maintain consistent, high-quality tests across the codebase.
Based on learnings
3-3: Improved clarity in testing strategy.The rewording from generic "combines" to "focused testing that combines" (line 3) and clarifying examples' dual purposes (lines 7-9) makes the testing approach more concrete and understandable.
Also applies to: 7-11
apps/www/vitest.config.ts (2)
5-11: LGTM - Configuration correctly migrated to project-scoped setup!The changes properly address the path alias resolution:
defineProjectusage (line 5) correctly scopes this config to the www app within the monorepo's Vitest workspacetsconfigPathsplugin (line 10) handles the~/*path aliases by reading from tsconfig.json, replacing the manual alias configuration- React SWC plugin (lines 7-9) provides faster compilation with styled-jsx support
The previous concern about path alias resolution failures is resolved by using the
tsconfigPathsplugin, which properly applies aliases within the test environment.
13-15: Good test configuration!The test name "arkenv.js.org" clearly identifies this project in test output, and the setup file properly extends the test environment with jest-dom matchers and global cleanup.
- Remove redundant mock resets - Simplify test setup
| process.env.NEXT_PUBLIC_DOCS_CONTENT_PATH = "apps/www/content/docs/"; | ||
| process.env.NEXT_PUBLIC_GITHUB_URL = "https://github.com/yamcodes/arkenv"; | ||
| }); | ||
|
|
There was a problem hiding this comment.
Bug: Test Pollution from Uncleaned Environment Variables
The beforeEach hook sets process.env variables, but the afterEach hook doesn't clean them up. This can lead to test pollution, where environment variables from one test affect subsequent tests, potentially causing flakiness.
Note
We already had a component testing PR (#56) so feel free to reference it for the implementation.
This PR implements comprehensive component testing for the www Next.js application using Vitest and React Testing Library.
What's Added
Testing Infrastructure
Component Tests
Added comprehensive test coverage for key components:
Configuration Updates
vitest.config.tsto include www app in test projectspackage.json(test, test:ui, test:coverage)Test Results
Benefits
The testing infrastructure follows best practices and integrates seamlessly with the existing monorepo setup, providing a solid foundation for maintaining and expanding the www app's component library.
Fixes #52.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Summary by CodeRabbit