test: add tests for MarksGradersController#randomly_assign method#7947
test: add tests for MarksGradersController#randomly_assign method#7947danielrafailov1 wants to merge 17 commits into
Conversation
Coverage Report for CI Build 26204813023Coverage increased (+0.03%) to 90.242%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
david-yz-liu
left a comment
There was a problem hiding this comment.
Nice work @danielrafailov1. I left a few comments, and also:
- Your PR is missing information in the template under "Proposed Changes". This is a key section in which you should describe what the purpose of the PR is and the changes it contains.
- In response to your question, yes I think it would be good to add another test to verify the behaviour when there are multiple students and graders specified.
| ### 🚨 Breaking changes | ||
|
|
||
| ### ✨ New features and improvements | ||
| - Added tests for `MarksGradersController` to achieve full test coverage for `randomly_assign` (#7947) |
There was a problem hiding this comment.
Add this entry under "Internal Changes", as we reserve the "New features and improvements" section for user-facing changes.
| context 'when students and graders are selected' do | ||
| it 'calls GradeEntryStudent `randomly_assign_tas` and returns a success response' do | ||
| student = create(:student) | ||
| grade_entry_student = grade_entry_form.grade_entry_students.find_or_create_by(role: student) |
There was a problem hiding this comment.
This code and the above line can be simplified, as we provide a factory for grade entry students directly. This means that you can use create(:grade_entry_student), you just need to also specify the associated grade_entry_form
There was a problem hiding this comment.
I think you may have missed that this comment was also referring specifically to Line 388. On GitHub, comments appear directly underneath the specific line that was selected when making the comment.
…ovements section to the Internal changes section
…domly_assign_tests
…domly_assign_tests
…_randomly_assign_tests Merged incoming changes from upstream master into this branch
…y_assign_tests' into marks_graders_controller_randomly_assign_tests merging with the remote branch
Proposed Changes
Implemented full test coverage for the randomly_assign method of the MarksGradersController class as per assigned task. Specifically, I took inspiration from the tests related to the unassign_all method to implement 5 seperate tests.
The first test ensures two things. Firstly that the randomly_assign_tas function gets called with the correct parameters (specifically two non-empty arrays of students and graders), and secondly that when all of the correct parameters are included in the body of the POST request, we get an ok response (200) response returned.
The second test ensures two things. Firstly that when we pass in an empty array of students to the request body we get a bad request returned. Secondly, we flash the user to indicate that they need to select a student.
The third test is the same as the second with the graders array being empty instead of the students array and the flash message being tailored to this.
The fourth test is the same as the second test with the only difference being that the students array is entirely omitted from the request body instead of just being empty.
The fifth test is the same as the third test with the only difference being that the graders array is entirely omitted from the request body instead of just being empty.
The sixth test is similar to the first test. It ensures that when all of the correct parameters are included in the body of the POST request, we get an ok response (200) response returned. However it differs from the first test in two ways. Firstly, we no longer mock the call to the randomly_assign_tas method. This is done so that we can actually ensure that the method did what it was supposed to do. Secondly, we ensure that the method did what it was supposed to do in the sense that every student should be assigned exactly one TA.
Screenshots of your changes (if applicable)
Associated documentation repository pull request (if applicable)
Type of Change
(Write an
Xor a brief description next to the type or types that best describe your changes.)Checklist
(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the
[ ]into a[x]in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)Before opening your pull request:
After opening your pull request:
Questions and Comments
(Include any questions or comments you have regarding your changes.)
Should I add another test to verify the order and number of students?