test: Added unit tests for the cc merchant info #3372
Conversation
WalkthroughSuperstar, listen up! 💥 This pull request is all about leveling up our test game for expense-related components. We've added new test cases for Changes
Sequence DiagramsequenceDiagram
participant User
participant Page
participant PopoverController
participant MerchantInfoPopover
User->>Page: Trigger Merchant Info
Page->>PopoverController: Create Popover
PopoverController-->>MerchantInfoPopover: Instantiate
MerchantInfoPopover->>User: Display Merchant Details
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 🪧 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: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/app/fyle/add-edit-expense/add-edit-expense-6.spec.ts(2 hunks)src/app/fyle/view-expense/view-expense.page.spec.ts(2 hunks)src/app/shared/components/cc-expense-merchant-info-popover/cc-expense-merchant-info-popover.component.spec.ts(1 hunks)
| it('should close the popover when clicked on close button', () => { | ||
| const closeBtn = getElementBySelector(fixture, '[data-testid="close-btn"') as HTMLButtonElement; | ||
| closeBtn.click(); | ||
|
|
||
| fixture.detectChanges(); | ||
|
|
||
| expect(popoverController.dismiss).toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Superstar says: Add more test cases for close button interactions!
The close button test is good, but we need more test cases for:
- Close button keyboard interactions (Enter/Space key)
- Close button accessibility attributes
- Error handling when dismiss fails
| describe('template', () => { | ||
| it('should display the correct title', () => { | ||
| fixture.detectChanges(); | ||
| const title = getElementBySelector(fixture, '[data-testid="title"'); | ||
| expect(title.textContent).toEqual('Merchant'); | ||
| }); | ||
|
|
||
| it('should display the correct content', () => { | ||
| fixture.detectChanges(); | ||
| const content = getElementBySelector(fixture, '[data-testid="content"'); | ||
| expect(content.textContent).toEqual('This merchant name comes from the transaction.'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mind it! Template tests need more power!
The template tests verify basic content, but we should also test:
- Content formatting/styling
- Localization if applicable
- Dynamic content updates
| describe('CCExpenseMerchantInfoComponent', () => { | ||
| let component: CCExpenseMerchantInfoPopoverComponent; | ||
| let popoverController: jasmine.SpyObj<PopoverController>; | ||
| let fixture: ComponentFixture<CCExpenseMerchantInfoPopoverComponent>; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mind it! The test suite name doesn't match the component name.
The test suite is named 'CCExpenseMerchantInfoComponent' but the component being tested is 'CCExpenseMerchantInfoPopoverComponent'. Consistency is the style of the superstar!
-describe('CCExpenseMerchantInfoComponent', () => {
+describe('CCExpenseMerchantInfoPopoverComponent', () => {📝 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.
| describe('CCExpenseMerchantInfoComponent', () => { | |
| let component: CCExpenseMerchantInfoPopoverComponent; | |
| let popoverController: jasmine.SpyObj<PopoverController>; | |
| let fixture: ComponentFixture<CCExpenseMerchantInfoPopoverComponent>; | |
| describe('CCExpenseMerchantInfoPopoverComponent', () => { | |
| let component: CCExpenseMerchantInfoPopoverComponent; | |
| let popoverController: jasmine.SpyObj<PopoverController>; | |
| let fixture: ComponentFixture<CCExpenseMerchantInfoPopoverComponent>; | |
| it('openCCExpenseMerchantInfoModal(): should open the transaction status info modal', fakeAsync(() => { | ||
| const popoverSpy = jasmine.createSpyObj('HTMLIonPopoverElement', ['present']); | ||
| popoverController.create.and.resolveTo(popoverSpy); | ||
|
|
||
| component.openCCExpenseMerchantInfoModal(); | ||
|
|
||
| tick(); | ||
|
|
||
| expect(popoverController.create).toHaveBeenCalledOnceWith({ | ||
| component: CCExpenseMerchantInfoPopoverComponent, | ||
| cssClass: 'fy-dialog-popover', | ||
| }); | ||
| expect(popoverSpy.present).toHaveBeenCalledTimes(1); | ||
| })); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Style! The new test case looks good but needs more power!
The test verifies basic modal creation, but we should add test cases for:
- Error handling when modal creation fails
- Modal dismissal
- Data passing to the modal
Example test cases to add:
it('should handle modal creation failure', fakeAsync(() => {
const error = new Error('Modal creation failed');
popoverController.create.and.rejectWith(error);
component.openCCExpenseMerchantInfoModal();
tick();
expect(console.error).toHaveBeenCalledWith('Failed to open merchant info modal:', error);
}));
it('should handle modal dismissal', fakeAsync(() => {
const popoverSpy = jasmine.createSpyObj('HTMLIonPopoverElement', ['present', 'onDidDismiss']);
popoverController.create.and.resolveTo(popoverSpy);
popoverSpy.onDidDismiss.and.resolveTo({ data: { dismissed: true } });
component.openCCExpenseMerchantInfoModal();
tick();
expect(popoverSpy.onDidDismiss).toHaveBeenCalled();
}));
There was a problem hiding this comment.
@devendrafyle, should we add a test case for modal dismiss alone?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@Dimple16 modal dismiss is not used here, it's the CCExpenseMerchantInfoPopoverComponent property and we have added the test case for it there.
| it('openCCExpenseMerchantInfoModal(): should open the transaction status info modal', fakeAsync(() => { | ||
| const popoverSpy = jasmine.createSpyObj('HTMLIonPopoverElement', ['present']); | ||
| popoverController.create.and.resolveTo(popoverSpy); | ||
|
|
||
| component.openCCExpenseMerchantInfoModal(); | ||
|
|
||
| tick(); | ||
|
|
||
| expect(popoverController.create).toHaveBeenCalledOnceWith({ | ||
| component: CCExpenseMerchantInfoPopoverComponent, | ||
| cssClass: 'fy-dialog-popover', | ||
| }); | ||
| expect(popoverSpy.present).toHaveBeenCalledTimes(1); | ||
| })); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mind it! This test case needs the same power as its brother!
Similar to ViewExpensePage, this test should include additional scenarios:
- Error handling
- Modal dismissal
- Data passing
Example test cases to add:
it('should handle modal creation failure gracefully', fakeAsync(() => {
const error = new Error('Modal creation failed');
popoverController.create.and.rejectWith(error);
component.openCCExpenseMerchantInfoModal();
tick();
expect(console.error).toHaveBeenCalledWith('Failed to open merchant info modal:', error);
}));
it('should handle modal dismissal properly', fakeAsync(() => {
const popoverSpy = jasmine.createSpyObj('HTMLIonPopoverElement', ['present', 'onDidDismiss']);
popoverController.create.and.resolveTo(popoverSpy);
popoverSpy.onDidDismiss.and.resolveTo({ data: { dismissed: true } });
component.openCCExpenseMerchantInfoModal();
tick();
expect(popoverSpy.onDidDismiss).toHaveBeenCalled();
}));…to FYLE-86cxbrpvu
|
| it('openCCExpenseMerchantInfoModal(): should open the transaction status info modal', fakeAsync(() => { | ||
| const popoverSpy = jasmine.createSpyObj('HTMLIonPopoverElement', ['present']); | ||
| popoverController.create.and.resolveTo(popoverSpy); | ||
|
|
||
| component.openCCExpenseMerchantInfoModal(); | ||
|
|
||
| tick(); | ||
|
|
||
| expect(popoverController.create).toHaveBeenCalledOnceWith({ | ||
| component: CCExpenseMerchantInfoPopoverComponent, | ||
| cssClass: 'fy-dialog-popover', | ||
| }); | ||
| expect(popoverSpy.present).toHaveBeenCalledTimes(1); | ||
| })); |
There was a problem hiding this comment.
@devendrafyle, should we add a test case for modal dismiss alone?
Clickup
https://app.clickup.com/t/86cxbrpvu
Summary by CodeRabbit
AddEditExpensePageandViewExpensePagecomponents to verify the functionality of opening a modal for credit card expense merchant information.CCExpenseMerchantInfoPopoverComponentwith additional assertions and improved mocking practices.