Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request adds a comprehensive test suite for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CopyButton
participant Clipboard
participant ErrorHandler as Sentry/Toast
User->>CopyButton: Clicks button
CopyButton->>Clipboard: writeText("npm install")
alt Copy succeeds
Clipboard-->>CopyButton: Success
else Copy fails
Clipboard-->>CopyButton: Throws error
CopyButton->>ErrorHandler: Capture error and notify
end
Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/docs/vitest.config.ts (1)
14-25: Consider enhancing coverage configuration.The coverage configuration is good, but consider adding these commonly excluded patterns:
- Test files:
**/*.test.*,**/*.spec.*- Story files:
**/*.stories.*- Mock files:
**/__mocks__/**coverage: { provider: 'v8', reporter: ['text', 'html'], exclude: [ 'node_modules/**', '.next/**', 'coverage/**', '**/*.d.ts', '**/*.config.*', '**/index.ts', + '**/*.test.*', + '**/*.spec.*', + '**/*.stories.*', + '**/__mocks__/**', ], },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (5)
apps/docs/components/page/copy-button.test.tsx(1 hunks)apps/docs/package.json(2 hunks)apps/docs/test/setup.d.ts(1 hunks)apps/docs/vitest.config.ts(1 hunks)apps/docs/vitest.setup.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Check: test (macos-latest)
apps/docs/components/page/copy-button.test.tsx
[failure] 7-7: TypeError: vi.mock is not a function. (In 'vi.mock("@sentry/nextjs"
}))', 'vi.mock' is undefined)
at /Users/runner/work/ark.env/ark.env/apps/docs/components/page/copy-button.test.tsx:7:4
🪛 GitHub Check: test (ubuntu-latest)
apps/docs/components/page/copy-button.test.tsx
[failure] 7-7: TypeError: vi.mock is not a function. (In 'vi.mock("@sentry/nextjs"
}))', 'vi.mock' is undefined)
at /home/runner/work/ark.env/ark.env/apps/docs/components/page/copy-button.test.tsx:7:4
🪛 GitHub Actions: tests
apps/docs/components/page/copy-button.test.tsx
[error] 1-1: Unhandled error between tests: 'vi.mock' is undefined.
🪛 Biome (1.9.4)
apps/docs/test/setup.d.ts
[error] 7-7: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🔇 Additional comments (4)
apps/docs/vitest.setup.ts (1)
1-11: LGTM! Well-structured test setup.The setup follows testing best practices by:
- Importing necessary testing libraries
- Extending expect with Testing Library matchers
- Registering cleanup to prevent test pollution
apps/docs/test/setup.d.ts (1)
1-9: LGTM! Type declarations are correctly set up.The
anytype usage in the interface is acceptable here as:
- It's part of the standard type definition pattern for test matchers
- It's constrained by the TestingLibraryMatchers generic type
🧰 Tools
🪛 Biome (1.9.4)
[error] 7-7: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
apps/docs/package.json (2)
10-13: New Testing Scripts Added
The addition of the"test": "vitest","test:ui": "vitest --ui", and"coverage": "vitest run --coverage"scripts directly supports the component testing objectives of this PR. These scripts provide a clear pathway for running tests, viewing test UI, and generating coverage reports with Vitest.
44-53: New DevDependencies for Enhanced Testing Infrastructure
The newly added devDependencies (including"@testing-library/react","@testing-library/user-event","@vitejs/plugin-react","@vitest/coverage-v8","@vitest/ui","jsdom","vitest","@testing-library/jest-dom", and"@types/testing-library__jest-dom") robustly expand the testing capabilities for the project. This is in line with the PR’s objective of adding component testing and ensures a more integrated and maintainable testing environment. Additionally, the inclusion of"postcss": "^8.5.1"is appropriate if CSS processing is needed in the build or test setups.
| vi.mock('@sentry/nextjs', () => ({ | ||
| captureException: vi.fn(), | ||
| })) | ||
|
|
||
| // Mock the useToast hook | ||
| vi.mock('~/hooks/use-toast', () => ({ | ||
| useToast: () => ({ | ||
| toast: vi.fn(), | ||
| }), | ||
| })) |
There was a problem hiding this comment.
Fix vi.mock setup to resolve pipeline failures.
The pipeline failures indicate that vi.mock is undefined. This is likely due to incorrect Vitest setup. Move the mock setup to a separate file and import it in the test.
Create a new file __mocks__/setup.ts:
import { vi } from 'vitest'
// Mock Sentry
vi.mock('@sentry/nextjs', () => ({
captureException: vi.fn(),
}))
// Mock the useToast hook
vi.mock('~/hooks/use-toast', () => ({
useToast: () => ({
toast: vi.fn(),
}),
}))
export {}Then update the test file:
-// Mock Sentry
-vi.mock('@sentry/nextjs', () => ({
- captureException: vi.fn(),
-}))
-
-// Mock the useToast hook
-vi.mock('~/hooks/use-toast', () => ({
- useToast: () => ({
- toast: vi.fn(),
- }),
-}))
+import './__mocks__/setup'🧰 Tools
🪛 GitHub Check: test (macos-latest)
[failure] 7-7: TypeError: vi.mock is not a function. (In 'vi.mock("@sentry/nextjs"
}))', 'vi.mock' is undefined)
at /Users/runner/work/ark.env/ark.env/apps/docs/components/page/copy-button.test.tsx:7:4
🪛 GitHub Check: test (ubuntu-latest)
[failure] 7-7: TypeError: vi.mock is not a function. (In 'vi.mock("@sentry/nextjs"
}))', 'vi.mock' is undefined)
at /home/runner/work/ark.env/ark.env/apps/docs/components/page/copy-button.test.tsx:7:4
| it('shows copied state when clicked', async () => { | ||
| const user = userEvent.setup() | ||
| const mockWriteText = vi.fn() | ||
|
|
||
| // Mock the clipboard API | ||
| const clipboardSpy = vi.spyOn(navigator.clipboard, 'writeText') | ||
| clipboardSpy.mockImplementation(mockWriteText) | ||
|
|
||
| render(<CopyButton command="npm install" />) | ||
| const button = screen.getByRole('button') | ||
|
|
||
| await user.click(button) | ||
|
|
||
| expect(mockWriteText).toHaveBeenCalledWith('npm install') | ||
| expect(screen.getByLabelText('Copy command')).toBeInTheDocument() | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add test cases for toast notifications.
The test suite should verify toast notifications for both successful and failed copy operations.
Add these test cases:
it('shows success toast when copy succeeds', async () => {
const user = userEvent.setup()
const mockToast = vi.fn()
vi.mocked(useToast).mockReturnValue({ toast: mockToast })
render(<CopyButton command="npm install" />)
await user.click(screen.getByRole('button'))
expect(mockToast).toHaveBeenCalledWith({
title: 'Copied to clipboard',
description: 'The command has been copied to your clipboard'
})
})
it('shows error toast when copy fails', async () => {
const user = userEvent.setup()
const mockToast = vi.fn()
vi.mocked(useToast).mockReturnValue({ toast: mockToast })
// Mock clipboard error
vi.spyOn(navigator.clipboard, 'writeText')
.mockRejectedValue(new Error('Clipboard error'))
render(<CopyButton command="npm install" />)
await user.click(screen.getByRole('button'))
expect(mockToast).toHaveBeenCalledWith({
title: 'Failed to copy',
description: 'Please try again or copy manually',
variant: 'destructive'
})
})
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/docs/components/page/copy-button.test.tsx (2)
6-16:⚠️ Potential issueFix vi.mock setup to resolve pipeline failures.
The pipeline failures indicate that
vi.mockis undefined. This is likely due to incorrect Vitest setup.Move the mock setup to a separate file and import it in the test:
// __mocks__/setup.ts import { vi } from 'vitest' // Mock Sentry vi.mock('@sentry/nextjs', () => ({ captureException: vi.fn(), })) // Mock the useToast hook vi.mock('~/hooks/use-toast', () => ({ useToast: () => ({ toast: vi.fn(), }), })) export {}Then update the test file:
-// Mock Sentry -vi.mock("@sentry/nextjs", () => ({ - captureException: vi.fn(), -})); - -// Mock the useToast hook -vi.mock("~/hooks/use-toast", () => ({ - useToast: () => ({ - toast: vi.fn(), - }), -})); +import './__mocks__/setup'🧰 Tools
🪛 GitHub Check: test (macos-latest)
[failure] 7-7: TypeError: vi.mock is not a function. (In 'vi.mock("@sentry/nextjs"
}))', 'vi.mock' is undefined)
at /Users/runner/work/ark.env/ark.env/apps/docs/components/page/copy-button.test.tsx:7:4🪛 GitHub Check: test (ubuntu-latest)
[failure] 7-7: TypeError: vi.mock is not a function. (In 'vi.mock("@sentry/nextjs"
}))', 'vi.mock' is undefined)
at /home/runner/work/ark.env/ark.env/apps/docs/components/page/copy-button.test.tsx:7:4
23-27: 🛠️ Refactor suggestionAdd test cases for toast notifications.
The test suite should verify toast notifications for both successful and failed copy operations.
Add these test cases:
it('shows success toast when copy succeeds', async () => { const user = userEvent.setup() const mockToast = vi.fn() vi.mocked(useToast).mockReturnValue({ toast: mockToast }) render(<CopyButton command="npm install" />) await user.click(screen.getByRole('button')) expect(mockToast).toHaveBeenCalledWith({ title: 'Copied to clipboard', description: 'The command has been copied to your clipboard' }) }) it('shows error toast when copy fails', async () => { const user = userEvent.setup() const mockToast = vi.fn() vi.mocked(useToast).mockReturnValue({ toast: mockToast }) // Mock clipboard error vi.spyOn(navigator.clipboard, 'writeText') .mockRejectedValue(new Error('Clipboard error')) render(<CopyButton command="npm install" />) await user.click(screen.getByRole('button')) expect(mockToast).toHaveBeenCalledWith({ title: 'Failed to copy', description: 'Please try again or copy manually', variant: 'destructive' }) })Also applies to: 29-44, 46-60
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/tests.yml(1 hunks)apps/docs/components/page/copy-button.test.tsx(1 hunks)apps/docs/package.json(2 hunks)apps/docs/test/setup.d.ts(1 hunks)apps/docs/vitest.config.ts(1 hunks)apps/docs/vitest.setup.ts(1 hunks)package.json(1 hunks)turbo.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/docs/vitest.setup.ts
- apps/docs/test/setup.d.ts
- apps/docs/vitest.config.ts
🧰 Additional context used
🪛 GitHub Check: test (macos-latest)
apps/docs/components/page/copy-button.test.tsx
[failure] 7-7: TypeError: vi.mock is not a function. (In 'vi.mock("@sentry/nextjs"
}))', 'vi.mock' is undefined)
at /Users/runner/work/ark.env/ark.env/apps/docs/components/page/copy-button.test.tsx:7:4
🪛 GitHub Check: test (ubuntu-latest)
apps/docs/components/page/copy-button.test.tsx
[failure] 7-7: TypeError: vi.mock is not a function. (In 'vi.mock("@sentry/nextjs"
}))', 'vi.mock' is undefined)
at /home/runner/work/ark.env/ark.env/apps/docs/components/page/copy-button.test.tsx:7:4
🪛 GitHub Actions: tests
apps/docs/components/page/copy-button.test.tsx
[error] 1-1: Unhandled error between tests: 'vi.mock' is undefined.
🔇 Additional comments (5)
turbo.json (1)
14-19: LGTM! Well-structured Turborepo task configuration.The new tasks are correctly configured:
test:oncefor one-time test runstestwith cache disabled and persistence enabled for watch modecoveragefor test coverage reportingpackage.json (1)
16-18: LGTM! Well-structured npm scripts for testing.The new scripts correctly utilize Turborepo for test execution:
testfor running tests in watch modetest:oncefor single test runscoveragefor test coverage reporting.github/workflows/tests.yml (1)
54-55: LGTM! Good use of Turborepo caching.The caching setup will help optimize the build process.
apps/docs/package.json (2)
10-14: New Testing Scripts for Vitest IntegrationThe addition of the "test", "test:ui", "test:once", and "coverage" scripts using Vitest offers a streamlined approach for running tests and generating coverage reports. Please ensure that these commands are documented in the project README to assist team members in using them effectively.
45-54: Expanded DevDependencies to Support Enhanced TestingThe newly added devDependencies—including Vitest, Testing Library packages, and associated plugins—correctly extend the application's testing infrastructure to support component tests such as those for the
CopyButton. Verify that all version specifications are in line with your project's compatibility requirements and that any configuration changes associated with these libraries are well-documented.
| - name: Run test | ||
| run: bun test |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update test command to use npm scripts.
The test command should use the newly added npm scripts instead of directly using bun.
- - name: Run test
- run: bun test
+ - name: Run test
+ run: bun run test:once📝 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.
| - name: Run test | |
| run: bun test | |
| - name: Run test | |
| run: bun run test:once |
|
Closing in favor of #146. |
> [!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 - **Vitest configuration** with React and jsdom support for component testing - **Test setup** with @testing-library/jest-dom matchers for enhanced assertions - **Testing dependencies** including @testing-library/react, jsdom, and @vitejs/plugin-react - **Integration** with existing monorepo test setup via updated root vitest config ### Component Tests Added comprehensive test coverage for key components: - **Button component** (10 tests) - Tests all variants, sizes, disabled states, custom classes, and asChild behavior - **Card component** (3 tests) - Tests content rendering, styling application, and prop forwarding - **CopyButton component** (3 tests) - Tests rendering, styling, and accessibility features - **Logo component** (4 tests) - Tests text rendering, default styling, custom classes, and HTML semantics - **StarUsButton component** (4 tests) - Tests responsive behavior, link attributes, and custom styling ### Configuration Updates - Updated root `vitest.config.ts` to include www app in test projects - Added test scripts to www app `package.json` (test, test:ui, test:coverage) - Configured proper test cleanup and mocking support ## Test Results - **24 new component tests** all passing - **Total test suite**: 76 tests (52 existing + 24 new) - All tests run successfully across packages and apps - Maintains existing test performance and reliability ## Benefits - Enables confident refactoring of React components - Catches regressions early in development - Provides foundation for testing complex UI interactions - Follows established testing patterns from the repository - Supports TypeScript with full type checking 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. <!-- START COPILOT CODING AGENT TIPS --> --- 💡 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](https://gh.io/copilot-coding-agent-tips) in the docs. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Tests - Added comprehensive unit/integration tests for multiple UI components and hooks, plus shared test setup and project-wide Vitest config with coverage. - Documentation - Revamped testing philosophy and structure in TESTING.md; added README note on test colocation. - Chores - Switched Vite React plugin to SWC variant across projects; added test scripts, UI runner, coverage tooling, and typings. - Updated Vitest configs (projects, setup files, coverage, path resolution). - Ignored coverage directory and *.tar.wim; adjusted lint rules for test files. - Minor cleanup and commented code removal. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: yamcodes <2014360+yamcodes@users.noreply.github.com> Co-authored-by: Yam C Borodetsky <yam@yam.codes> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 participant
Summary by CodeRabbit