test: create tests for stores batch 2#1351
Conversation
WalkthroughThis pull request refines testing guidelines and implementations across the codebase. Updates include enhanced documentation for testing Vue components and Pinia stores, modifications to mocks for service requests and shared callbacks, and new comprehensive test suites for the account, activation code, callback actions, dropdown, and errors stores. Additionally, payload structures in callback actions have been simplified, and formatting in store files improved, all while retaining the underlying functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test Suite
participant A as Account Store
participant C as Callback Service
T->>A: Invoke account action (e.g., manage)
A->>C: Call send(payload)
C-->>A: Return response
A-->>T: Return action result
sequenceDiagram
participant T as Test Suite
participant AC as Activation Code Store
participant S as Session Storage
T->>AC: Set activation data / toggle modal
AC->>S: Update modal visibility state
S-->>AC: Confirm update
AC-->>T: Reflect updated state
Possibly related PRs
Suggested Reviewers
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.cursor/rules/web-testing-rules.mdc(3 hunks)web/__test__/mocks/services/request.ts(1 hunks)web/__test__/mocks/shared-callbacks.ts(1 hunks)web/__test__/store/account.test.ts(1 hunks)web/__test__/store/activationCode.test.ts(1 hunks)web/store/account.ts(5 hunks)web/store/activationCode.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
web/store/activationCode.ts (1)
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-04-10T14:59:48.996Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
🧬 Code Graph Analysis (4)
web/store/activationCode.ts (1)
web/consts.ts (1)
ACTIVATION_CODE_MODAL_HIDDEN_STORAGE_KEY(1-1)
web/__test__/store/activationCode.test.ts (2)
web/store/activationCode.ts (1)
useActivationCodeStore(26-80)web/consts.ts (1)
ACTIVATION_CODE_MODAL_HIDDEN_STORAGE_KEY(1-1)
web/__test__/store/account.test.ts (2)
web/store/account.ts (1)
useAccountStore(30-381)web/helpers/urls.ts (1)
ACCOUNT_CALLBACK(45-45)
web/store/account.ts (1)
web/helpers/urls.ts (1)
ACCOUNT_CALLBACK(45-45)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test API
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (35)
web/__test__/mocks/services/request.ts (1)
14-18: Good addition of WebguiInstallKey supportThese mock functions properly implement the expected behavior by returning the path unchanged, which matches how the real implementation would behave.
web/store/activationCode.ts (6)
1-2: Properly added explicit Vue importsThe explicit imports of Vue reactivity primitives are now included, which follows the testing guidelines mentioned in the web-testing-rules.mdc file about not relying on Nuxt's auto-imports in tests.
37-39: Improved formatting for readabilityThe reformatting of this computed property enhances code readability while maintaining the same functionality.
41-43: Better formatting for initializationThe multi-line formatting makes the ref initialization more readable, especially with the condition for retrieving from sessionStorage.
44-44: Concise function implementationClean and concise implementation for the setter function.
46-48: Improved watch callback with ternary operatorThe ternary operator makes this logic more concise and readable compared to an if/else structure.
9-9: Properly using createPinia in web component contextThe
setActivePinia(createPinia())initialization is essential here since Vue components are compiled into web components in this project, ensuring all components share a single Pinia store instance..cursor/rules/web-testing-rules.mdc (5)
8-9: Clear environment clarificationThis clarification about testing outside the Nuxt environment explains why explicit Vue imports are needed, which helps developers understand the testing context better.
73-73: Good practice for minimizing mocksPreferring real implementations over mocks when possible leads to more realistic testing scenarios and better test coverage.
86-121: Excellent store testing setup exampleThis well-structured example demonstrates the preferred approach for testing Pinia stores with proper initialization, mock declarations, and cleanup between tests.
123-175: Comprehensive testing guidelinesThese detailed guidelines cover critical aspects of store testing including initialization, reactivity, mocking best practices, and common pitfalls. The examples of correct and incorrect approaches are particularly helpful.
176-199: Valuable guidance for testing state and complex interactionsThese sections provide specific examples for testing computed properties, state changes, and async operations, which will help ensure comprehensive test coverage.
web/store/account.ts (13)
84-96: Consistent payload structure improvementThe payload is now structured as an array containing a single object with the
typeproperty included within it. This provides a more consistent approach across all callback actions.
101-112: Consistent payload formattingFollowing the same pattern as other actions, improving consistency throughout the codebase.
121-132: Consistent payload structureMaintains the same pattern for payload structure as seen in other actions.
140-152: Consistent payload structureThe payload structure remains consistent with other actions.
155-167: Consistent payload formattingFollows the consistent pattern for payload structure.
170-182: Consistent payload structureMaintains consistency with other callback actions.
185-197: Consistent payload structureThe payload structure follows the consistent pattern throughout the file.
200-212: Consistent payload structureMaintains the same payload structure pattern as other actions.
215-227: Consistent payload structureFollows the same pattern for consistency.
230-242: Consistent payload structureMaintains consistency with other callback actions.
246-258: Consistent payload structureThe payload structure remains consistent with the pattern established throughout the file.
262-268: Improved conditional formattingThe multi-line formatting of this condition improves readability for this complex check.
274-288: Better object destructuring formattingThe restructured formatting for destructuring and nested object creation improves readability.
web/__test__/store/account.test.ts (3)
1-78: Well-structured test setup with comprehensive mockingThe test setup is clear and organized with appropriate mocking of all dependencies. The use of
vi.mockfor each dependency ensures proper isolation for testing the account store.
79-152: Good coverage of core account store actionsThe tests for manage, myKeys, linkKey, recover, signIn, and signOut actions are thorough and verify the correct parameters are passed to the mockSend function.
153-191: Comprehensive testing of conditional behaviorThe tests effectively verify that the downgradeOs and updateOs actions behave differently based on the autoRedirectReplace parameter. Good to see both code paths tested.
web/__test__/store/activationCode.test.ts (3)
13-33: Excellent approach to mocking sessionStorageThe implementation of a Map-based mock for sessionStorage is very clean and appropriately isolates the tests from the actual browser storage. The Pinia mock using
vi.importActualis also a good practice to maintain the real functionality while overriding specific parts.
58-90: Thorough testing of state and actionsThe tests for the store's state management are well-structured and cover the important cases: initialization, setting complete data, and handling partial data.
92-118: Complete modal visibility tests with proper async handlingThe tests for modal visibility are thorough and include testing the interaction with sessionStorage. The use of
nextTickensures that Vue's reactivity has time to process between operations.web/__test__/mocks/shared-callbacks.ts (4)
5-9: Improved documentation for the mock moduleThe updated comments provide clearer documentation about the purpose of this mock implementation.
13-14: Enhanced testability with vi.fn() for encrypt and decrypt methodsUsing
vi.fn()to wrap the encrypt and decrypt methods allows for better tracking of calls and arguments during tests.
21-28: Improved mock implementation for useCallbackThe updated implementation using
vi.fn()for the useCallback hook and its methods provides better testability. The send method now accepts additional parameters to match the real implementation.
34-39: Better mock for crypto-js/aesThe improved implementation for the crypto-js/aes mock now returns objects with toString methods, which better mimics the actual module behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/__test__/store/activationCode.test.ts (1)
1-9: Good test file structure with appropriate imports.The test file is well organized with proper imports from Vue, Pinia and Vitest. However, there's one issue:
-import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';The
afterEachimport is defined but never used in the test file.🧰 Tools
🪛 GitHub Check: Build Web App
[failure] 9-9:
'afterEach' is defined but never used. Allowed unused vars must match /^_/u
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (5)
.cursor/rules/web-testing-rules.mdc(3 hunks)web/__test__/store/activationCode.test.ts(1 hunks)web/__test__/store/callbackActions.test.ts(1 hunks)web/store/callbackActions.ts(1 hunks)web/store/updateOs.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/store/updateOs.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .cursor/rules/web-testing-rules.mdc
🧰 Additional context used
🧬 Code Graph Analysis (2)
web/__test__/store/callbackActions.test.ts (4)
web/composables/preventClose.ts (3)
addPreventClose(7-9)removePreventClose(11-13)preventClose(1-5)web/store/callbackActions.ts (1)
useCallbackActionsStore(26-183)web/store/account.ts (1)
useAccountStore(30-381)web/store/server.ts (1)
useServerStore(61-1399)
web/__test__/store/activationCode.test.ts (2)
web/store/activationCode.ts (1)
useActivationCodeStore(26-80)web/consts.ts (1)
ACTIVATION_CODE_MODAL_HIDDEN_STORAGE_KEY(1-1)
🪛 GitHub Check: Build Web App
web/__test__/store/activationCode.test.ts
[failure] 9-9:
'afterEach' is defined but never used. Allowed unused vars must match /^_/u
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (13)
web/store/callbackActions.ts (1)
1-1: Appropriate addition of Vue composition API imports.Adding these Vue reactivity imports enhances the store's capabilities by enabling the use of reactive references, computed properties, and watchers. This aligns with modern Vue best practices and the testing requirements.
web/__test__/store/activationCode.test.ts (4)
13-35: Well-implemented test environment setup.Good approach to:
- Mocking console methods to suppress output during tests
- Setting up a simulated sessionStorage implementation using Map
- Proper cleanup with beforeAll/afterAll hooks
36-60: Effective dependency mocking.The mocks for store dependencies are well-implemented, providing controlled test data that isolates the activation code store for proper testing.
72-104: Comprehensive state and actions testing.Great test coverage that:
- Verifies default state initialization
- Tests data setting functionality
- Handles cases with missing optional fields
This section thoroughly validates the store's core data management capabilities.
106-132: Thorough modal visibility testing.The tests effectively verify:
- Default modal visibility behavior
- Conditional showing based on data presence
- Session storage interaction with modal state
This provides good coverage of the visibility logic and storage persistence.
web/__test__/store/callbackActions.test.ts (8)
18-30: Good test setup with console mocking.Properly suppressing console output during tests prevents log noise while maintaining the ability to check logging behavior when needed.
32-123: Thorough dependency mocking setup.Excellent approach to mocking all dependencies:
- Uses factory functions to avoid hoisting issues
- Properly mocks all required store methods
- Sets up mock return values that match expected store behaviors
This creates a controlled environment for isolating and testing the callback actions store.
125-150: Well-structured test initialization.Good practices:
- Creating a fresh store instance before each test
- Properly retrieving and typing mock functions
- Clearing mocks between tests to prevent interference
151-158: Proper initial state verification.The test correctly validates that the store initializes with expected default values.
160-190: Good watcher functionality tests.Tests properly cover:
- Success path when watcher returns data
- Handling when watcher returns null
This validates the core watcher functionality that initiates callback processing.
192-237: Comprehensive data handling tests.Tests effectively cover:
- Valid data handling
- Error cases for missing data
- Invalid callback type handling
The error path testing is particularly valuable for ensuring robust behavior.
239-338: Excellent action type coverage.Tests cover all important actions:
- Sign in actions
- Sign out actions
- UpdateOs actions
- Multiple action combinations
This provides thorough validation of the store's core purpose - handling various callback action types.
340-396: Good status management testing.Tests properly verify:
- Loading state triggers preventClose behavior
- State transitions clean up properly
- History state is correctly updated
- Multiple status transitions work correctly
These tests ensure that UI safety features like preventing window close during operations work as expected.
mdatelle
force-pushed
the
test/create-tests-for-stores-batch-2
branch
from
3bcabcd to
153eb48
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/__test__/store/activationCode.test.ts (1)
9-9: Remove unused importafterEachThe static analysis tool correctly identified that
afterEachis imported but never used in this file. Remove it to keep imports clean and prevent potential confusion.-import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';🧰 Tools
🪛 GitHub Check: Build Web App
[failure] 9-9:
'afterEach' is defined but never used. Allowed unused vars must match /^_/u
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (5)
.cursor/rules/web-testing-rules.mdc(3 hunks)web/__test__/store/activationCode.test.ts(1 hunks)web/__test__/store/callbackActions.test.ts(1 hunks)web/store/callbackActions.ts(1 hunks)web/store/updateOs.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- web/store/updateOs.ts
- web/store/callbackActions.ts
- web/test/store/callbackActions.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
web/__test__/store/activationCode.test.ts (2)
web/store/activationCode.ts (1)
useActivationCodeStore(26-80)web/consts.ts (1)
ACTIVATION_CODE_MODAL_HIDDEN_STORAGE_KEY(1-1)
🪛 GitHub Check: Build Web App
web/__test__/store/activationCode.test.ts
[failure] 9-9:
'afterEach' is defined but never used. Allowed unused vars must match /^_/u
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test API
- GitHub Check: Build API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (12)
web/__test__/store/activationCode.test.ts (6)
13-25: Well-structured test setup with proper cleanupGood practice to mock console methods and restore them after tests. This prevents test output from being cluttered with debug and error messages while ensuring the original console functionality is restored afterward.
27-34: Effective sessionStorage mocking implementationThe Map-based mock for sessionStorage is clean and provides all necessary methods for testing. This approach isolates the tests from the actual browser storage and allows for predictable test behavior.
36-59: Comprehensive dependency mockingThe test properly mocks all external dependencies (Pinia's storeToRefs, server store, and callback actions store) to ensure the activation code store can be tested in isolation. The mocks provide the specific values needed for the store's computed properties to work correctly.
61-70: Clean test setup with proper isolationThe test suite follows best practices by setting up a fresh store instance before each test and clearing mocks and storage. This ensures that tests don't affect each other.
72-104: Thorough state and actions testingThis section thoroughly tests the store's state initialization and data setting functionality, including edge cases like partial data. Good coverage of the core functionality.
106-133: Complete modal visibility logic testingThe tests cover all aspects of the modal visibility logic, including default behavior, conditions for hiding the modal, and interaction with session storage. The use of
nextTick()correctly handles Vue's reactivity system when testing watched properties..cursor/rules/web-testing-rules.mdc (6)
8-11: Improved clarity about testing environment contextGood additions that clarify the testing environment (Nuxt.js but testing outside of Nuxt) and the potential need for explicit imports due to Nuxt's auto-import feature. This context is crucial for developers to understand potential issues they might encounter.
73-73: Valuable guidance on mock usageThis addition provides important guidance on preferring real implementations over mocks when possible. This helps create more realistic and robust tests that are less likely to give false positives.
86-121: Clear and comprehensive store testing setup exampleThe updated basic setup example is well-structured and includes essential elements like type safety, proper initialization, and cleanup. The explicit typing of the store variable using
ReturnType<typeof useYourStore>is particularly helpful for maintaining type safety in tests.
123-176: Detailed guidelines with specific best practicesThis section provides valuable guidance on store initialization, Vue reactivity, mocking, and common pitfalls. The examples of correct vs. incorrect approaches (particularly with mocking) are especially helpful for preventing subtle issues.
177-187: Practical advice for testing state and gettersThe guidance on testing computed properties and state changes is concise and practical. The example clearly demonstrates how to test derived state after actions.
189-201: Comprehensive coverage of complex interactionsGood addition that addresses testing more complex scenarios like store interactions, error handling, and async operations. The async testing example is particularly valuable for showing how to test both the initial loading state and the final success state.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/__test__/store/account.test.ts (1)
193-227: Added tests for previously missing actions.The tests for replace, trialExtend, and trialStart actions address the feedback from the previous review. These tests ensure that the actions call the appropriate methods with the correct parameters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
web/__test__/store/account.test.ts(1 hunks)web/__test__/store/activationCode.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/test/store/activationCode.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
web/__test__/store/account.test.ts (2)
web/store/account.ts (1)
useAccountStore(30-381)web/helpers/urls.ts (1)
ACCOUNT_CALLBACK(45-45)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
web/__test__/store/account.test.ts (4)
1-11: Well-structured imports and file organization.The file begins with clear imports and organization, properly setting up the testing environment with Pinia and Vitest. The import of ACCOUNT_CALLBACK from helpers/urls is correctly used throughout the tests.
12-65: Mocks are properly configured for isolated testing.The mock setup is comprehensive, covering all dependencies needed for testing the account store in isolation. The mock functions (mockSend, mockPurge, mockSetError) are correctly defined and used throughout the tests.
66-78: Good test setup and cleanup practices.The test structure follows best practices with proper store initialization before each test and thorough cleanup afterward. Using both clearAllMocks and resetAllMocks ensures a clean testing environment for each test.
79-191: Comprehensive tests for basic account actions.The tests thoroughly verify that each account action calls the send method with the correct parameters. The tests for downgradeOs and updateOs properly check both scenarios (with and without redirect).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
web/__test__/store/account.test.ts (2)
195-229: Previously missing actions are now properly testedThe tests for replace, trialExtend, and trialStart actions address the missing test coverage identified in previous review comments.
232-271: State management functions are properly testedThe test cases for state management verify that account actions and payloads are correctly set and the account action status is updated appropriately. This addresses the previous feedback about missing tests for state management functions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
web/__test__/store/account.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
web/__test__/store/account.test.ts (2)
web/store/account.ts (1)
useAccountStore(30-381)web/helpers/urls.ts (1)
ACCOUNT_CALLBACK(45-45)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test API
- GitHub Check: Build API
- GitHub Check: Build Web App
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
web/__test__/store/account.test.ts (4)
1-13: Well-organized test setup with appropriate importsThe file starts with clear documentation and includes all necessary imports for testing the account store functionality.
14-66: Comprehensive mock setup for dependenciesThe test file properly mocks all required dependencies including Apollo composables, callback actions, errors store, replaceRenew store, server store, and unraidApi store. This ensures isolated testing of the account store functionality.
68-80: Test suite setup follows best practicesThe test setup correctly initializes the Pinia store before each test and cleans up mocks afterward. This ensures test isolation and prevents test pollution.
81-194: Good coverage of account store actionsThe tests cover the core account store actions including manage, myKeys, linkKey, recover, signIn, signOut, downgradeOs, and updateOs. The test cases properly verify that each action correctly calls the mockSend function with the appropriate parameters. Special handling for actions with optional redirect parameters is also tested.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
web/__test__/store/account.test.ts (1)
109-258: Comprehensive test coverage for account store actions.The test suite covers all the necessary actions including the previously missing ones (replace, trialExtend, trialStart). The implementation is consistent and thorough.
🧹 Nitpick comments (2)
web/__test__/store/account.test.ts (2)
313-360: Consider simplifying the test implementation for connectSignInMutation.The current implementation creates a complex typed store with duplicated mutation implementations. Consider using mocking techniques to directly test the store's implementation rather than recreating it.
A more direct approach could be:
it('should handle connectSignInMutation success', async () => { // Setup const store = useAccountStore(); // Mock the internal mutation implementation const originalMutation = store.connectSignInMutation; store.connectSignInMutation = vi.fn().mockImplementation(async () => { store.accountActionStatus = 'updating'; await nextTick(); store.accountActionStatus = 'success'; return mockUseMutation(); }); // Test store.setAccountAction('signIn' as unknown as ExternalSignIn); store.setConnectSignInPayload({ apiKey: 'test-api-key', email: 'test@example.com', preferred_username: 'test-user', }); await store.connectSignInMutation(); await nextTick(); expect(store.accountActionStatus).toBe('success'); // Restore store.connectSignInMutation = originalMutation; });
362-405: Consider applying the same simplification to the connectSignOutMutation test.Similar to the previous comment, the connectSignOutMutation test could be simplified using the same approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
web/__test__/store/account.test.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Build Web App
web/__test__/store/account.test.ts
[failure] 46-46:
'callback' is defined but never used. Allowed unused args must match /^_/u
[failure] 95-95:
'useMutationSpy' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 481-481:
'error' is defined but never used
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
web/__test__/store/account.test.ts (2)
260-300: LGTM! Effective implementation of state management tests.This test block effectively verifies that the account actions, payloads, and status are properly set and managed by the store. Good work using explicit type declarations and verifying the state transitions.
302-495: Great job implementing the Apollo Mutations tests!These tests thoroughly cover both success and error paths for the mutations, addressing the previous review comment requests. The error handling scenario is particularly well-implemented with proper error object construction and verification of error propagation.
🧰 Tools
🪛 GitHub Check: Build Web App
[failure] 481-481:
'error' is defined but never used
…s and cleanup setup file
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/__test__/store/account.test.ts (2)
45-45: Replaceanytype with a more specific type.The use of
anytype here is flagged by static analysis. Consider using a more specific type such asApolloErrororunknown.- onError: (_: any) => ({ off: vi.fn() }), + onError: (_: unknown) => ({ off: vi.fn() }),🧰 Tools
🪛 GitHub Check: Build Web App
[failure] 45-45:
Unexpected any. Specify a different type
98-99: Remove unused useMutation mock line.The line
vi.mocked(useMutation)doesn't perform any function as it's not assigned to a variable or used anywhere. It can be safely removed.beforeEach(() => { setActivePinia(createPinia()); store = useAccountStore(); - vi.mocked(useMutation); vi.clearAllMocks(); vi.useFakeTimers(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
web/__test__/store/account.test.ts(1 hunks)web/__test__/store/server.test.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/test/store/server.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
web/__test__/store/account.test.ts (2)
web/store/account.ts (2)
useAccountStore(30-381)ConnectSignInMutationPayload(24-28)web/helpers/urls.ts (1)
ACCOUNT_CALLBACK(45-45)
🪛 GitHub Check: Build Web App
web/__test__/store/account.test.ts
[failure] 45-45:
Unexpected any. Specify a different type
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test API
- GitHub Check: Build API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
web/__test__/store/account.test.ts (3)
1-494: The test suite provides excellent coverage of account store functionality.This test file implements comprehensive coverage for the account store, including action calls, state management, and Apollo mutations. All the missing tests mentioned in previous reviews have been addressed.
🧰 Tools
🪛 GitHub Check: Build Web App
[failure] 45-45:
Unexpected any. Specify a different type
311-358: Good approach to testing the success path of the connectSignInMutation.The implementation thoroughly tests the state transitions from 'waiting' to 'success' during the sign-in mutation process. The approach of creating a typed store with controlled behavior allows for precise testing of the mutation flow.
405-492: Comprehensive error handling test for mutations.The test effectively verifies that errors during mutations are properly handled by checking both the state transition to 'failed' and the correct error being set. The empty catch block is appropriately commented as intentionally empty.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
web/__test__/store/account.test.ts (4)
86-86: Unnecessary call tovi.mocked(useMutation)This line calls
vi.mocked(useMutation)but doesn't store the result or use it in any way. Since it's not doing anything, it can be removed.- vi.mocked(useMutation);
333-333: Consider using proper typing instead of type assertionUsing
as unknown as ExternalSignInis a workaround that bypasses TypeScript's type checking. This could hide potential type errors.- typedStore.setAccountAction('signIn' as unknown as ExternalSignIn); + typedStore.setAccountAction({ type: 'signIn' } as ExternalSignIn);
382-382: Consider using proper typing instead of type assertionSimilar to the previous comment, type assertion with
as unknown as ExternalSignOutbypasses TypeScript's type checking.- typedStore.setAccountAction('signOut' as unknown as ExternalSignOut); + typedStore.setAccountAction({ type: 'signOut' } as ExternalSignOut);
299-458: Consider reducing duplication in typedStore creationThe
typedStoreobject is created with similar structure in multiple test cases. Consider extracting this to a helper function to reduce duplication and improve maintainability.You could create a helper function like:
function createTypedStore(initialStatus = 'ready') { const store = useAccountStore(); const accountActionStatus = ref(initialStatus); return { ...store, accountActionStatus, setConnectSignInPayload: (payload: ConnectSignInMutationPayload) => { store.setConnectSignInPayload(payload); accountActionStatus.value = 'waiting'; }, setQueueConnectSignOut: (value: boolean) => { store.setQueueConnectSignOut(value); accountActionStatus.value = 'waiting'; }, connectSignInMutation: async () => { accountActionStatus.value = 'updating'; const mockMutation = mockUseMutation(); await mockMutation.mutate(); accountActionStatus.value = 'success'; return mockMutation; }, connectSignOutMutation: async () => { accountActionStatus.value = 'updating'; const mockMutation = mockUseMutation(); await mockMutation.mutate(); accountActionStatus.value = 'success'; return mockMutation; } }; }Then use it in your tests:
it('should handle connectSignInMutation success', async () => { const typedStore = createTypedStore(); typedStore.setAccountAction({ type: 'signIn' } as ExternalSignIn); // ... rest of the test });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
web/__test__/store/account.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
web/__test__/store/account.test.ts (4)
299-346: Good implementation of mutation success testThis test effectively verifies the successful execution of the
connectSignInMutationby checking the state transitions from 'waiting' to 'success'. The approach of creating a typed store with controlled state is clean and effective.
393-481: Well-structured error handling testThe test for mutation errors is well implemented. It properly mocks the error scenario, verifies the state transitions, and checks that the error is properly reported through the error store.
209-243: Good coverage of newly added actionsThe tests for the additional actions
replace,trialExtend, andtrialStartprovide good coverage for these functions, verifying they call the expected endpoints with the right parameters.
246-286: Complete state management testingThe state management tests verify both setAccountAction and payload handling for sign-in and sign-out flows, ensuring proper state transitions.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
elibosley
left a comment
elibosley
left a comment
There was a problem hiding this comment.
These look good. Nice work on the tests here they seem pretty comprehensive!
elibosley
left a comment
elibosley
left a comment
There was a problem hiding this comment.
These look good. Nice work on the tests here they seem pretty comprehensive!
| vi.mock('vue', async () => { | ||
| const actual = await vi.importActual('vue'); |
There was a problem hiding this comment.
Yeah, we shouldn't need to do this for the other store tests since we're testing Pinia directly with setActivePinia(createPinia()); but this particular store is a real pita trying to get everything working right. We can update tests when this store gets refactored and broken up a little.
3 participants
This is batch 2 of the web store tests. I started implementing the recommended approach in the Pinia docs for the store tests and was able to eliminate most of the mocking files. The server.test.ts file still uses
pinia/testingfor now since I was having trouble with some of the dependencies in the store due to it's complexity.I also updated the
web-testing-rulesfor Cursor in an effort to streamline the AI's approach when helping with tests. There's some things it still struggles with and seems like it doesn't always take the rules into consideration until after it hits a snag. It likes to try and create a mock for the store it's actually testing even though there's a rule in place and has to be reminded.Summary by CodeRabbit
Summary by CodeRabbit
Tests
Refactor