Create tests for ErrorMessage, ErrorHandling and Milestone services #290

MadLamprey · 2024-03-11T09:08:38Z

Summary:

Fixes part of #87

Type of change:

🧪 Tests Update

Changes Made:

Added test cases for error-handling.service.ts, error-message.service.ts and milestone.service.ts

Proposed Commit Message:

Create tests for ErrorMessage, ErrorHandling and Milestone services

Currently, there are no tests written for ErrorMessage, ErrorHandling and Milestone services.

Let's create tests for ErrorMessage, ErrorHandling and Milestone services.

…create-tests

NereusWB922 · 2024-03-13T11:14:43Z

tests/services/error-message.service.spec.ts

+
+  describe('ErrorMessageService: invalidUrlMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.invalidUrlMessage()).toBe(
+        'URL is invalid, or repository does not exist, please indicate the repository you wish to view to continue.'
+      );
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchIssuesMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchIssuesMessage()).toBe('Failed to fetch issue.');
+    });
+  });
+
+  describe('ErrorMessageService: usesrsUnassignableMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.usersUnassignableMessage('test')).toBe(
+        'Cannot assign test to the issue. Please check if test is authorized.'
+      );
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchMilestoneMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchMilestoneMessage()).toBe('Failed to fetch milestones.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchLabelsMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchLabelsMessage()).toBe('Failed to fetch labels.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchUsersMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchUsersMessage()).toBe('Failed to fetch assignable users for repository');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchDataFileMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchDataFileMessage()).toBe('Failed to fetch data file.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchEventsMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchEventsMessage()).toBe('Failed to fetch issue events for repository');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchActivityEventsMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchActivityEventsMessage()).toBe('Failed to fetch activity events for repository');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchLatestRelease()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchLatestReleaseMessage()).toBe('Failed to fetch latest release.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchSettingsFileMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchSettingsFileMessage()).toBe('Failed to fetch settings file.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchAuthenticatedUsersMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchAuthenticatedUsersMessage()).toBe('Failed to fetch authenticated user.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToOpenInBrowserMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToOpenInBrowserMessage()).toBe('Unable to open this issue in Browser');
+    });
+  });
+
+  describe('ErrorMessageService: applicationVersionOutdatedMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.applicationVersionOutdatedMessage()).toBe('Please update to the latest version of WATcher.');
+    });
+  });
+
+  describe('ErrorMessageService: multipleDropdownOptionsErrorMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.multipleDropdownOptionsErrorMessage()).toBe('Supply either Dropdown option number or text, not both.');
+    });
+  });
+
+  describe('ErrorMessageService: noDropdownOptionsErrorMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.noDropdownOptionsErrorMessage()).toBe('No Dropdown identification parameters supplied.');
+    });
+  });


I'm not sure if there's any benefit to write test cases for the constant provider service. If test cases fail due to future changes in constant values, developers will likely just edit the test cases, resulting in duplicated changes. Also, I think that integration tests for services using these constants should suffice to catch any errors in those constant values.
@CATcher-org/2324s2 what do yall think?

Yeah I'm in agreement with you, don't think we need tests for this.

luminousleek

Thanks for this PR! Some small changes needed, and as per Nereus's comment I don't think we need tests for ErrorMessageService.

Also, please split this PR into one PR for each service you are writing tests for.

luminousleek · 2024-03-13T13:47:37Z

tests/services/error-message.service.spec.ts

+
+  describe('ErrorMessageService: invalidUrlMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.invalidUrlMessage()).toBe(
+        'URL is invalid, or repository does not exist, please indicate the repository you wish to view to continue.'
+      );
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchIssuesMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchIssuesMessage()).toBe('Failed to fetch issue.');
+    });
+  });
+
+  describe('ErrorMessageService: usesrsUnassignableMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.usersUnassignableMessage('test')).toBe(
+        'Cannot assign test to the issue. Please check if test is authorized.'
+      );
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchMilestoneMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchMilestoneMessage()).toBe('Failed to fetch milestones.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchLabelsMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchLabelsMessage()).toBe('Failed to fetch labels.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchUsersMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchUsersMessage()).toBe('Failed to fetch assignable users for repository');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchDataFileMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchDataFileMessage()).toBe('Failed to fetch data file.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchEventsMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchEventsMessage()).toBe('Failed to fetch issue events for repository');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchActivityEventsMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchActivityEventsMessage()).toBe('Failed to fetch activity events for repository');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchLatestRelease()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchLatestReleaseMessage()).toBe('Failed to fetch latest release.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchSettingsFileMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchSettingsFileMessage()).toBe('Failed to fetch settings file.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToFetchAuthenticatedUsersMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToFetchAuthenticatedUsersMessage()).toBe('Failed to fetch authenticated user.');
+    });
+  });
+
+  describe('ErrorMessageService: unableToOpenInBrowserMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.unableToOpenInBrowserMessage()).toBe('Unable to open this issue in Browser');
+    });
+  });
+
+  describe('ErrorMessageService: applicationVersionOutdatedMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.applicationVersionOutdatedMessage()).toBe('Please update to the latest version of WATcher.');
+    });
+  });
+
+  describe('ErrorMessageService: multipleDropdownOptionsErrorMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.multipleDropdownOptionsErrorMessage()).toBe('Supply either Dropdown option number or text, not both.');
+    });
+  });
+
+  describe('ErrorMessageService: noDropdownOptionsErrorMessage()', () => {
+    it('should return the correct message', () => {
+      expect(ErrorMessageService.noDropdownOptionsErrorMessage()).toBe('No Dropdown identification parameters supplied.');
+    });
+  });


Yeah I'm in agreement with you, don't think we need tests for this.

luminousleek · 2024-03-13T13:52:01Z

tests/services/error-handling.service.spec.ts

+      expect(mockSnackBar.openFromComponent).toHaveBeenCalledWith(FormErrorComponent, { data: HTTP_422_ERROR });
+    });
+
+    it('should use the GeneralMessageErrorComponent when handling other http errors', () => {


Might want to test another HttpError to test the default case.

luminousleek · 2024-03-13T13:55:54Z

tests/services/milestone.service.spec.ts

+      githubServiceSpy.fetchAllMilestones.and.returnValue(of(mockMilestones));
+      milestoneService.fetchMilestones().subscribe((response) => {
+        expect(githubServiceSpy.fetchAllMilestones).toHaveBeenCalled();
+        expect(response).toEqual(mockMilestones);


I don't think we need this line since you're already setting the return value ofr fetchAllMilestones as mockMilestones three lines above?

NereusWB922

Great work on adding new test cases! Just a minor suggestion to ensure proper handling of asynchronous behavior in the test.

NereusWB922 · 2024-03-13T15:34:40Z

tests/services/milestone.service.spec.ts

+    it('should fetch all milestones', async () => {
+      const mockMilestones = [{ title: 'Milestone 1' }, { title: 'Milestone 2' }];
+      githubServiceSpy.fetchAllMilestones.and.returnValue(of(mockMilestones));
+      milestoneService.fetchMilestones().subscribe((response) => {
+        expect(githubServiceSpy.fetchAllMilestones).toHaveBeenCalled();
+        expect(response).toEqual(mockMilestones);
+        expect(milestoneService.milestones.length).toBe(3);
+        expect(milestoneService.milestones[0].title).toBe('Milestone 1');
+        expect(milestoneService.hasNoMilestones).toBeFalse();
+      });


Consider using Jasmine's done() callback at the end of the assertions. This is to ensure that the test doesn't complete before the value is emitted, which would lead to the assertions not being executed by Jasmine.

The same issue applies to the test case below.

I looked into this, and received an Error Log.

If I am not wrong, async/await cannot be used with done. You try remove the async keyword.

MadLamprey · 2024-03-25T05:12:59Z

PR has been separated into #303 and #304.

Misra Aditya and others added 11 commits February 19, 2024 16:16

Test deployment

f7f2d32

Test deployment

dbfb109

Merge branch 'CATcher-org:main' into main

99460cb

Merge branch 'CATcher-org:main' into main

b0b9d68

Fix errors

76c511a

Write tests for error-handling and error-message.

9decaf7

Write tests for milestone service

3e8aeae

Merge branch 'CATcher-org:main' into main

700abeb

Merge branch 'main' of https://github.com/MadLamprey/WATcher into 87-…

b215aa9

…create-tests

Fix lint errors

6403eb1

Fix errors

7286347

NereusWB922 reviewed Mar 13, 2024

View reviewed changes

luminousleek requested changes Mar 13, 2024

View reviewed changes

NereusWB922 requested changes Mar 13, 2024

View reviewed changes

MadLamprey closed this Mar 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create tests for ErrorMessage, ErrorHandling and Milestone services #290

Create tests for ErrorMessage, ErrorHandling and Milestone services #290

MadLamprey commented Mar 11, 2024

NereusWB922 Mar 13, 2024

luminousleek Mar 13, 2024

luminousleek left a comment

luminousleek Mar 13, 2024

luminousleek Mar 13, 2024

luminousleek Mar 13, 2024

NereusWB922 left a comment

NereusWB922 Mar 13, 2024

MadLamprey Mar 15, 2024

NereusWB922 Mar 15, 2024

MadLamprey commented Mar 25, 2024

Create tests for ErrorMessage, ErrorHandling and Milestone services #290

Create tests for ErrorMessage, ErrorHandling and Milestone services #290

Conversation

MadLamprey commented Mar 11, 2024

Summary:

Type of change:

Changes Made:

Proposed Commit Message:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luminousleek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NereusWB922 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MadLamprey commented Mar 25, 2024