-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add test suite with GitHub Actions workflow and React version compatibility tests #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ompatibility tests
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a comprehensive automated testing infrastructure. It adds a GitHub Actions workflow for CI with matrix testing across multiple React and Node.js versions, expands Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Matrix (React/Node)
participant npm/Jest
participant Codecov
GitHub Actions->>Matrix (React/Node): Start jobs (test, lint) on push/PR
Matrix (React/Node)->>npm/Jest: Install dependencies, set up environment
npm/Jest->>npm/Jest: Install specific React version
npm/Jest->>npm/Jest: Run tests with coverage
npm/Jest->>Codecov: Upload coverage report (with React/Node flags)
GitHub Actions->>npm/Jest: Run lint job (Node 18)
npm/Jest->>npm/Jest: Build and type-check project
Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/__tests__/useAsyncMemo.test.ts (1)
1-342: Outstanding test suite with thorough edge case coverage!The test suite excellently covers all aspects of the
useAsyncMemohook including memoization behavior, dependency tracking, error handling, and cleanup. The tests for special values (null, 0, empty string) and StrictMode compatibility demonstrate attention to detail.However, the same React Testing Library compatibility issue affects this test suite.
The pipeline failure with "Cannot find module 'react-dom/client'" is the same compatibility issue identified in the
useAsyncEffekttest suite. The solution involves updating the React Testing Library version strategy to support the full React version matrix (16-19).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/workflows/test.yml(1 hunks)package.json(2 hunks)src/__tests__/useAsyncEffekt.test.ts(1 hunks)src/__tests__/useAsyncMemo.test.ts(1 hunks)src/setupTests.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Test
src/__tests__/useAsyncMemo.test.ts
[error] 2-2: Test suite failed to run: Cannot find module 'react-dom/client' from 'node_modules/@testing-library/react/dist/pure.js'. This caused the test to fail.
src/__tests__/useAsyncEffekt.test.ts
[error] 1-1: Test suite failed to run: Cannot find module 'react-dom/client' from 'node_modules/@testing-library/react/dist/pure.js'. This caused the test to fail.
🪛 actionlint (1.7.7)
.github/workflows/test.yml
42-42: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (4)
package.json (2)
15-26: Excellent testing infrastructure setup!The comprehensive Jest setup with multi-version React testing is well-designed. The scripts for testing across React versions 16-19 provide good compatibility coverage.
63-91: Jest configuration looks solid with good coverage thresholds.The Jest configuration is comprehensive with appropriate settings for TypeScript and React testing. The 80% coverage thresholds are reasonable for maintaining code quality.
src/setupTests.ts (1)
1-19: Clean and well-structured test setup!The setup correctly extends Jest matchers and thoughtfully suppresses specific React deprecation warnings to reduce test noise while preserving other error logging. The beforeAll/afterAll pattern ensures proper cleanup.
.github/workflows/test.yml (1)
13-21: Excellent matrix strategy with proper compatibility exclusions.The matrix testing across React versions 16-19 and Node versions 16,18,20 provides comprehensive coverage. The exclusion of React 19 with Node 16 shows good awareness of compatibility requirements.
…ng in useAsyncMemo and useAsyncEffekt
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/__tests__/useAsyncMemo.test.ts (2)
118-119: Replace setTimeout with proper async utilities.Using
setTimeoutfor waiting in tests can be flaky. Consider usingwaitForor similar testing utilities for more reliable timing.- // Wait a bit to ensure the error is handled - await new Promise((resolve) => setTimeout(resolve, 10)); + // Wait for the promise to be processed + await act(async () => { + await Promise.resolve(); + });
169-200: Consider more predictable test for rapid dependency changes.The test allows for either "value-2" or "value-3" due to race conditions, which might mask actual bugs. Consider using more controlled timing.
Consider using fake timers or more controlled promises to make the test behavior more predictable:
it("should handle rapid dependency changes", async () => { + jest.useFakeTimers(); const computeFn = jest .fn() - .mockResolvedValueOnce("value-1") - .mockResolvedValueOnce("value-2") - .mockResolvedValueOnce("value-3"); + .mockImplementation((value) => + new Promise(resolve => setTimeout(() => resolve(`value-${value}`), 100)) + ); // ... rest of test with controlled timing + jest.useRealTimers(); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
.coderabbit.yaml(1 hunks).github/workflows/test.yml(1 hunks)Dockerfile.test(1 hunks)package.json(2 hunks)run-matrix-tests.sh(1 hunks)src/__tests__/index.test.ts(1 hunks)src/__tests__/test-utils.ts(1 hunks)src/__tests__/useAsyncEffekt.test.ts(1 hunks)src/__tests__/useAsyncMemo.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .coderabbit.yaml
- src/tests/index.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tests/useAsyncEffekt.test.ts
- .github/workflows/test.yml
🔇 Additional comments (6)
src/__tests__/test-utils.ts (1)
10-31: The version compatibility logic is well-implemented.The conditional logic correctly handles the migration of
renderHookfrom@testing-library/react-hooksto@testing-library/reactin React 18+. The error handling with console.error and rethrowing is appropriate for debugging failed imports.src/__tests__/useAsyncMemo.test.ts (3)
6-13: Good test setup and cleanup practices.The beforeEach and afterEach hooks properly manage timer state, ensuring tests don't interfere with each other.
77-107: Excellent error handling test with proper console mocking.The test correctly mocks console.error to suppress expected error logs and properly restores it afterward. The logic for testing error scenarios while preserving the last successful value is well-implemented.
556-583: No changes required: test correctly asserts five factory callsThe
useAsyncMemohook always invokes the factory on each dependency change and only prevents stale state updates—it does not abort in-flight factory calls. Expecting all five calls (and settling on “value-5”) is correct.package.json (2)
15-26: Well-structured test scripts for multi-version compatibility.The test scripts provide good coverage across React versions with appropriate testing library versions for each React version.
65-94: Excellent Jest configuration with comprehensive coverage setup.The Jest configuration is well-structured with appropriate test environment, setup files, and coverage thresholds. The 80% coverage threshold is reasonable for a library.
Summary by CodeRabbit
New Features
Tests
useAsyncEffektanduseAsyncMemohooks, covering various scenarios and edge cases.Chores