-
-
Notifications
You must be signed in to change notification settings - Fork 93
add test for events routers #1956
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
add test for events routers #1956
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test module works great. It would just be nice to have some more comments to explain how everything works and then remove a few unneeded items.
it('should call EventController.create and return the created event', async (done) => { | ||
EventController.create.mockImplementationOnce((req, res) => res.status(201).send(mockEvent)); | ||
|
||
const newEventData = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't really need this object. We could just reuse. mockEvent
it('should return 500 if an error occurs when fetching next event by project', async (done) => { | ||
const mockError = new Error('Simulated database error for next event by project'); | ||
|
||
Event.find.mockImplementationOnce(() => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please write some comments for this and other sections
|
||
const response = await request.get(`/api/events/nexteventbyproject/${mockProjectId}`); | ||
|
||
expect(Event.find).toHaveBeenCalledWith({ project: mockProjectId }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need these conditions that are tested in the last test case.
Update - @angela-lee1 will be updating this PR over the course of this week |
Terribly sorry about the delay. I'll get it completed by tonight or tomorrow night. No blockers. |
![]() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Fixes #1888
What changes did you make and why did you make them ?