[#12048] Migrate tests for GetNotificationActionTest #13210
Conversation
| NotificationAttributes notification = typicalBundle.notifications.get("notification1"); | ||
| Notification testNotification = new Notification( | ||
| notification.getStartTime(), | ||
| notification.getEndTime(), | ||
| notification.getStyle(), | ||
| notification.getTargetUser(), | ||
| notification.getTitle(), | ||
| notification.getMessage()); | ||
| notification.setNotificationId(testNotification.getId().toString()); |
There was a problem hiding this comment.
Great observation to use typicalBundle. However, it uses src/test/resources/data/typicalDataBundle.json which is for datastore entities. After the migration is done, the file will become obsolete. I think it might be better to just create a Notification object in the test.
We may also consider adding a sql ver test data file @domoberzin ?
| NotificationData output = (NotificationData) jsonResult.getOutput(); | ||
| verifyNotificationEquals(expected, output); | ||
|
|
||
| when(mockLogic.getNotification(testNotification.getId())).thenCallRealMethod(); |
There was a problem hiding this comment.
Is the intended purpose of thenCallRealMethod() to reset the mock? Maybe can consider reset(mockLogic) because thenCallRealMethod() sets the real implementation to be called. But since Logic.getNotification should always be mocked, resetting is not strictly necessary.
| GetNotificationAction action = getAction(Const.ParamsNames.NOTIFICATION_ID, UUID.randomUUID().toString()); | ||
| EntityNotFoundException enfe = assertThrows(EntityNotFoundException.class, action::execute); |
There was a problem hiding this comment.
Minor style suggestion: Consider adding a blank line between the action and the assertions to improve readability.
| GetNotificationAction action = getAction(Const.ParamsNames.NOTIFICATION_ID, null, new String[] {}); | ||
| InvalidHttpParameterException ihpe = assertThrows(InvalidHttpParameterException.class, action::execute); |
Part of #12048
Outline of Solution
Added the unit tests for GetNotificationActionTest for PostgreSQL.
PS: This is not a duplicate of #13219 if anyone gets confused by the similar name