2u/add view tests to optimizer

[jesperhodge]

See #35887 .

[rayzhou-bit]

Should this be changed back?

[jesperhodge]

Copy of what I put in slack:

Something like ArticleSerializer(data=article) is for deserializing only. It's typically used for incoming requests and thus requires validation and that's why we have this is_valid situation in the code.
On the flipside, when you respond to a request, you serialize using ArticleSerializer(article). See the difference? This is a different parameter than data.
And then it doesn't expect is_valid and in fact other views also only have that for incoming data, not for serializing outgoing data.
I'll fix this here.

[rayzhou-bit]

remove print statement

[rayzhou-bit]

Is self.course set up through ModuleStoreTestCase here?

[jesperhodge]

Either that or AuthorizeStaffTestCase.

[jesperhodge]

Yes, it's in AuthorizeStaffTestCase.

As you can see here, I defined a whole bunch of objects in there:

    def setUp(self):
        super().setUp()
        self.course_key = self.get_course_key_string()
        self.other_course_key = self.get_other_course_key_string()
        self.course = self.create_course_from_course_key(CourseKey.from_string(self.course_key))
        self.other_course = self.create_course_from_course_key(CourseKey.from_string(self.other_course_key))
        self.password = 'password'
        self.student = UserFactory.create(username='student', password=self.password)
        self.global_staff = GlobalStaffFactory(
            username='global-staff', password=self.password
        )
        self.course_instructor = InstructorFactory(
            username='instructor',
            password=self.password,
            course_key=self.course.id,
        )
        self.other_course_instructor = InstructorFactory(
            username='other-course-instructor',
            password=self.password,
            course_key=self.other_course.id,
        )

All of which can and should be used by child classes.

Now the tests that are running with this that are automatically included in child classes are:

    def test_student(self, expect_status=status.HTTP_403_FORBIDDEN):

    def test_instructor_in_another_course(self, expect_status=status.HTTP_403_FORBIDDEN):

    def test_global_staff(self, expect_status=status.HTTP_200_OK):

    def test_course_instructor(self, expect_status=status.HTTP_200_OK):

So they cover some interesting cases like: if there's a course instructor, but he has access to a different course and not the current one, we make sure that he gets an Unauthorized 403.

But you could easily overwrite that expectation in your child class by just doing:

def test_instructor_in_another_course(self, expect_status=status.HTTP_200_OK):
    super().test_instructor_in_another_course(expect_status)

Because now you're calling the parent method with 200 as expect status instead, which changes the assertion it runs.

[rayzhou-bit]

I see. I noticed additional tests were running when I ran pytest that are coming from AuthorizeStaffTestCase. I'll be utilizing this in my tests

[rayzhou-bit]

lgtm!