test: add integration tests for the grade api AP-1406
#302
Conversation
jignaciopm
changed the title
AP-1406
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| self.assertIn("earned_grade", response_content) | ||
| if extra_params["detailed"]: | ||
| self.assertIn("section_breakdown", response_content) | ||
| else: | ||
| self.assertNotIn("section_breakdown", response_content) | ||
|
|
||
| if extra_params["grading_policy"]: | ||
| self.assertIn("grading_policy", response_content) | ||
| self.assertIn("grader", response_content["grading_policy"]) | ||
| self.assertIn("grade_cutoffs", response_content["grading_policy"]) | ||
| else: | ||
| self.assertNotIn("grading_policy", response_content) |
There was a problem hiding this comment.
Should we also check the content of the response, or is checking the existence of the keys enough?
There was a problem hiding this comment.
It seems to me that it is enough to check that the expected keys come. Evaluating whether the correct content is coming seems more related to unit tests in my opinion 🤔 what do you think? @mariajgrimaldi
There was a problem hiding this comment.
Testing each API component's behavior is part of the unit test module, but here, we're testing that all those components work well together with the Open edX definitions available. So, we should include minimum checks for the response content to ensure the integration works as expected. Do you think that would work for this specific test suite?
| @@ -1,429 +0,0 @@ | |||
| Integration tests | |||
There was a problem hiding this comment.
If these integration tests were being executed somewhere (Jenkins?), dropping them would require a breaking change bump. I don't think it is, but we'll have to check either way.
There was a problem hiding this comment.
I agree with you, I didn't want to delete them either. How can we verify what you suggest? Because eliminating this module is part of the acceptance criteria for this task. @mariajgrimaldi 👀
There was a problem hiding this comment.
The job executing this test was called Saas-PROD-QA-eox-core, and according to an internal spreadsheet, it was marked as to be deleted. Do you have access to our old Jenkins installation? If you do, could you check if it was deleted? In any case, I don't think this would merit a breaking change since the last time the job was successfully executed was 3 years ago.
| @@ -7,7 +7,7 @@ jobs: | |||
| runs-on: ubuntu-latest | |||
| strategy: | |||
| matrix: | |||
| tutor_version: ['<18.0.0', '<19.0.0', 'nightly'] | |||
| tutor_version: ['<18.0.0', '<19.0.0'] | |||
There was a problem hiding this comment.
I think these changes should not reach main 👀
There was a problem hiding this comment.
Thank you, @jignaciopm! I've added some additional comments for you to review. Please let me know once you do :)
| "level_of_education": "p", | ||
| "city": "San Francisco", | ||
| "goals": "In ultrices sem nec turpis pretium, non consequat quam venenatis.", | ||
| }, |
There was a problem hiding this comment.
Why do we need this many new users?
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| self.assertIn("earned_grade", response_content) | ||
| if extra_params["detailed"]: | ||
| self.assertIn("section_breakdown", response_content) | ||
| else: | ||
| self.assertNotIn("section_breakdown", response_content) | ||
|
|
||
| if extra_params["grading_policy"]: | ||
| self.assertIn("grading_policy", response_content) | ||
| self.assertIn("grader", response_content["grading_policy"]) | ||
| self.assertIn("grade_cutoffs", response_content["grading_policy"]) | ||
| else: | ||
| self.assertNotIn("grading_policy", response_content) |
There was a problem hiding this comment.
Testing each API component's behavior is part of the unit test module, but here, we're testing that all those components work well together with the Open edX definitions available. So, we should include minimum checks for the response content to ensure the integration works as expected. Do you think that would work for this specific test suite?
|
|
||
| def test_get_grade_for_enrollment_not_found(self): | ||
| """ | ||
| Test grade info for a enrollment not found. |
There was a problem hiding this comment.
| Test grade info for a enrollment not found. | |
| Test grade info for a user not enrolled in the course. |
|
|
||
| def test_get_grade_for_course_not_found(self): | ||
| """ | ||
| Test grade info for a course not found. |
There was a problem hiding this comment.
| Test grade info for a course not found. | |
| Test grade info for an existing user in a course that does not exist. |
| response_data, [ | ||
| "No course found for course_id `course-not-exist`" | ||
| ], | ||
| ) |
There was a problem hiding this comment.
What would happen if the user does not exist?
Description
This PR adds integration tests for the Grade API and remove old module test integrations. These tests are executed in the job of Tutor Integration Tests
Testing instructions
Check the jobs of Tutor Integration Tests in the PR.
Jira Issue
Checklist for Merge