chore(auth-apis): Add api mocks to tests. #16867
Conversation
WalkthroughThe changes in this pull request primarily involve enhancements to the test suite for delegation access functionality. New imports for Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16867 +/- ##
==========================================
- Coverage 36.44% 36.43% -0.02%
==========================================
Files 6851 6852 +1
Lines 143572 143585 +13
Branches 40983 40977 -6
==========================================
- Hits 52328 52311 -17
- Misses 91244 91274 +30
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 12 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
![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
🧹 Outside diff range and nitpick comments (4)
apps/services/auth/public-api/test/mocks/companyRegistryClientService.mock.ts (1)
3-8: Consider enhancing the mock implementation.While the current implementation is functional, it could be improved to better support various test scenarios:
- The mock could support configurable responses for different test cases
- It might be beneficial to implement more methods from the original service
- Consider adding jest.fn() to track method calls during testing
Here's a suggested enhancement:
-export const CompanyRegistryClientServiceMock: Partial<CompanyRegistryClientService> = { - getCompany() { - return Promise.resolve(null) - }, -} +export class CompanyRegistryClientServiceMock implements Partial<CompanyRegistryClientService> { + getCompany = jest.fn().mockResolvedValue(null); + + // Allow setting custom responses for different test scenarios + setCompanyResponse(response: any) { + this.getCompany.mockResolvedValue(response); + } +}This enhancement would:
- Make the mock more flexible for different test scenarios
- Allow tracking of method calls using jest.spyOn()
- Maintain type safety with the original service interface
apps/services/auth/public-api/test/setup.ts (1)
Line range hint
1-240: Consider extracting mock providers into a shared configuration.The test setup is well-organized, but as more mock services are added, consider creating a shared mock provider configuration to improve maintainability. This could be a separate function or constant that groups all mock providers together.
Example approach:
const mockProviders = { NationalRegistryClientService: (user: IndividualDto) => createMockEinstaklingurApi(user), RskRelationshipsClient: RskProcuringClientMock, CompanyRegistryClientService: CompanyRegistryClientServiceMock, // ... other mocks }; // Then in setupWithAuth: .overrideProviders(Object.entries(mockProviders).map(([key, value]) => ({ provide: key, useValue: typeof value === 'function' ? value(nationalRegistryUser) : value, })))apps/services/auth/delegation-api/src/app/delegations/test/me-delegations.access-incoming.spec.ts (2)
Line range hint
46-69: Consider enhancing mock implementations for better test coverage.While the basic mocking setup is correct, consider these improvements:
- Add test cases for error scenarios
- Implement more realistic mock responses instead of just null values
- Consider testing edge cases with different response shapes
Example enhancement:
jest .spyOn(rskRelationshipsClientService, 'getIndividualRelationships') - .mockImplementation(async () => null) + .mockImplementation(async () => ({ + relationships: [], + metadata: { total: 0 } + })) jest .spyOn(companyRegistryClientService, 'getCompany') - .mockImplementation(async () => null) + .mockImplementation(async () => ({ + name: 'Test Company', + id: '1234567890', + status: 'active' + }))Also consider adding test cases for:
- API errors
- Network timeouts
- Invalid response formats
Line range hint
1-199: Well-structured test suite following NestJS best practices.The test suite demonstrates good practices:
- Proper use of TestApp and dependency injection
- Clear test organization with describe blocks
- Comprehensive CRUD operation coverage
- Good separation of concerns
Consider adding integration tests to verify the interaction between these mocked services in real-world scenarios.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
apps/services/auth/delegation-api/src/app/delegations/test/me-delegations.access-incoming.spec.ts(3 hunks)apps/services/auth/public-api/test/mocks/companyRegistryClientService.mock.ts(1 hunks)apps/services/auth/public-api/test/mocks/index.ts(1 hunks)apps/services/auth/public-api/test/setup.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/services/auth/delegation-api/src/app/delegations/test/me-delegations.access-incoming.spec.ts (2)
Pattern apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/services/auth/public-api/test/mocks/companyRegistryClientService.mock.ts (2)
Pattern apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/services/auth/public-api/test/mocks/index.ts (2)
Pattern apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/services/auth/public-api/test/setup.ts (2)
Pattern apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (5)
apps/services/auth/public-api/test/mocks/index.ts (1)
4-4: LGTM! The export follows established patterns.
The addition of CompanyRegistryClientService mock export follows the existing barrel file pattern and NestJS testing practices.
Let's verify the mock implementation exists:
✅ Verification successful
Mock implementation exists and follows the expected pattern
The verification confirms that:
- The mock file exists at the expected location
- It properly imports and implements the CompanyRegistryClientService interface
- The getCompany() method is implemented with a basic Promise.resolve(null) response
- The implementation uses a type-safe approach with Partial
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the CompanyRegistryClientService mock file exists and contains the expected mock implementation
# Check if the mock file exists and show its content
fd "companyRegistryClientService.mock.ts" --type f --exec cat {}
# Verify the mock is properly implemented with expected methods
ast-grep --pattern 'class $_Mock {
$$$
getCompany($$$) {
$$$
}
$$$
}'
Length of output: 394
apps/services/auth/public-api/test/mocks/companyRegistryClientService.mock.ts (1)
1-1: LGTM! Clean import statement.
The import is correctly sourced from the clients module, maintaining proper module boundaries.
apps/services/auth/public-api/test/setup.ts (2)
19-19: LGTM! Clean import organization.
The new imports for CompanyRegistryClientService and its mock are well-organized, following the established pattern of separating client services from their mocks.
Also applies to: 39-39
161-162: LGTM! Proper mock service integration.
The mock service is correctly integrated using NestJS's dependency injection system, which will help reduce test flakiness as intended.
Let's verify the mock implementation:
✅ Verification successful
Mock service implementation verified and properly structured
The mock service is correctly implemented and integrated:
- Mock is defined in a dedicated file
companyRegistryClientService.mock.ts - It implements
Partial<CompanyRegistryClientService>with required methods - Properly imported and used in the test setup
- Integration follows NestJS's dependency injection pattern with
.overrideProvider().useValue()
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the CompanyRegistryClientServiceMock implementation
# Check the mock implementation
rg -A 5 "CompanyRegistryClientServiceMock.*=.*{" ./apps/services/auth/public-api/test/mocks/
# Verify usage in tests
rg "CompanyRegistryClientService" ./apps/services/auth/public-api/test/ -g "*.spec.ts"
Length of output: 321
Script:
#!/bin/bash
# Let's fix the regex and try again with separate commands
# Check for mock implementation
rg "CompanyRegistryClientServiceMock" ./apps/services/auth/public-api/test/
# Look for the actual service usage in test files
rg "CompanyRegistryClientService" ./apps/services/auth/public-api/test/ -A 3
# Check for any mock files containing CompanyRegistry
fd "company.*mock" ./apps/services/auth/public-api/test/
Length of output: 2529
apps/services/auth/delegation-api/src/app/delegations/test/me-delegations.access-incoming.spec.ts (1)
12-12: LGTM! New client imports are properly organized.
The new imports for RskRelationshipsClient and CompanyRegistryClientService are correctly placed and follow the established import pattern.
Also applies to: 14-14
apps/services/auth/public-api/test/mocks/companyRegistryClientService.mock.ts
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
valurefugl
added
the
deprecated:automerge
Co-authored-by: Valur Einarsson <valure@live.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Add api mocks to tests.
Why
To prevent flakiness due to timeouts.
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
CompanyRegistryClientService, enhancing testing scenarios.Bug Fixes
Documentation
CompanyRegistryClientServiceMock, enhancing clarity and organization in the testing framework.