Unit tests for ajv helpers #430
Conversation
|
/review |
|
Preparing review... |
|
/review |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
/improve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // Mock implementation for initAjvSchemas that throws an error | ||
| const mockInitAjvSchemasWithError = () => { | ||
| const mockFn = mockRegistry.get('initGetAccountData3Req') | ||
| if (mockFn) { | ||
| mockFn() | ||
| } |
There was a problem hiding this comment.
Suggestion: Ensure that the mockInitAjvSchemasWithError function actually throws an error to properly test error handling in schema initialization. [possible issue, importance: 8]
| // Mock implementation for initAjvSchemas that throws an error | |
| const mockInitAjvSchemasWithError = () => { | |
| const mockFn = mockRegistry.get('initGetAccountData3Req') | |
| if (mockFn) { | |
| mockFn() | |
| } | |
| const mockInitAjvSchemasWithError = () => { | |
| const mockFn = mockRegistry.get('initGetAccountData3Req') | |
| if (mockFn) { | |
| mockFn() | |
| } | |
| throw new Error('Schema initialization error') | |
| } |
| const createMockVerifyFn = (isValid: boolean, errors: ErrorObject[] | null) => { | ||
| const mockFn = jest.fn().mockReturnValue(isValid) as jest.Mock & { errors: ErrorObject[] | null } | ||
| mockFn.errors = errors | ||
| return mockFn |
There was a problem hiding this comment.
Suggestion: Ensure that the createMockVerifyFn function correctly assigns the errors property to the mock function to avoid potential issues with error handling. [possible issue, importance: 7]
| const createMockVerifyFn = (isValid: boolean, errors: ErrorObject[] | null) => { | |
| const mockFn = jest.fn().mockReturnValue(isValid) as jest.Mock & { errors: ErrorObject[] | null } | |
| mockFn.errors = errors | |
| return mockFn | |
| const createMockVerifyFn = (isValid: boolean, errors: ErrorObject[] | null) => { | |
| const mockFn = jest.fn().mockReturnValue(isValid) as jest.Mock & { errors: ErrorObject[] | null } | |
| Object.defineProperty(mockFn, 'errors', { value: errors, writable: true }) | |
| return mockFn | |
| } |
|
|
||
| // Cleanup - Restore the original implementation | ||
| spy.mockRestore() | ||
| }) |
There was a problem hiding this comment.
This test mocks out initAjvSchemas implementation then calls it. it shouldnt not mock the implementation its testing.
Spy is fine since we could check how many times it was called but we shouldn't mock the implementation that we want to test.
|
|
||
| // Cleanup - Restore the original implementation | ||
| spy.mockRestore() | ||
| }) |
There was a problem hiding this comment.
same with this test. mocks out initAjvSchemas throwing an error then calls it expecting an error.
dont mock out the implementation
No description provided.