-
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 GetHasResponsesActionTest #13289
[#12048] Migrate tests for GetHasResponsesActionTest #13289
Conversation
Commit current changes before refactoring GetHasResponsesAction
Fix the path for execute method for migrated courses
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.
some nites, rest of the test lgtm!
src/test/java/teammates/sqlui/webapi/GetHasResponsesActionTest.java
Outdated
Show resolved
Hide resolved
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
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.
Other than the comments, the rest LGTM
} | ||
|
||
@Test | ||
void testAccessControl_nonInstructor_cannotAccessResponses() { |
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.
I think this test can be renamed to testAccessControl_nonInstructorWithInstructorEntityParams_cannotAccessResponses
for more clarity. We can also add another similar test for students with the name testAccessControl_nonStudentWithStudentEntityParams_cannotAccessResponses
.
|
||
verifyCannotAccess(params); | ||
} | ||
|
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.
I think we can add a test method with the name testAccessControl_invalidParams_throwsInvalidHttpParameterException
, with the following tests:
void testAccessControl_invalidParams_throwsInvalidHttpParameterException() {
loginAsStudent(typicalStudent.getGoogleId());
// Missing Entity Type in params
String[] params = new String[] {
Const.ParamsNames.COURSE_ID, typicalCourse.getId(),
};
verifyHttpParameterFailureAcl(params);
// Missing Course ID in params
String[] params1 = new String[] {
Const.ParamsNames.ENTITY_TYPE, Const.EntityType.STUDENT,
Const.ParamsNames.FEEDBACK_SESSION_NAME, typicalFeedbackSession.getName(),
};
verifyHttpParameterFailureAcl(params1);
logoutUser();
loginAsInstructor(typicalInstructor.getGoogleId());
// Missing course ID and feedback session id in params but with feedback session name in params
String[] params2 = new String[] {
Const.ParamsNames.FEEDBACK_SESSION_NAME, typicalFeedbackSession.getName(),
Const.ParamsNames.ENTITY_TYPE, Const.EntityType.INSTRUCTOR,
};
verifyHttpParameterFailureAcl(params2);
}
Part of #12048
Outline of Solution
Change GetHasResponsesActionTest.java to ensure compatibility with the PostgreSQL database following the database migration.