-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[#12048] Migrate tests for UpdateFeedbackResponseCommentActionTest #13251
base: master
Are you sure you want to change the base?
[#12048] Migrate tests for UpdateFeedbackResponseCommentActionTest #13251
Conversation
Test execute method with some positive and negative test cases Test checkSpecificAccessControl method with some positive and negative test cases
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.
Overall, really comprehensive, great work!
JsonResult r = getJsonResult(action); | ||
FeedbackResponseCommentData response = (FeedbackResponseCommentData) r.getOutput(); | ||
|
||
assertEquals(updatedComment.getCommentText(), response.getCommentText()); |
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.
abit of a nit: this particular sequence of assertions seems quite repetitive. Perhaps it could be better to extract out into a general method to compare the response/comments/other parameters?
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.
@BunnyHoppp Is it possible to refactor this?
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.
apart from the above comment, lgtm! really well done migrating this huge test 👍
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! Thanks for the hard work
Part of #12048
Outline of Solution
Change UpdateFeedbackResponseCommentActionTest.java to ensure compatibility with the PostgreSQL database following the database migration.