End-to-end bok choy test for cohorted courseware. - #6540
Conversation
There was a problem hiding this comment.
Note to reviewers: these changes were taken from #6283, which has not yet merged to master.
e712ea0 to
3804bd2
Compare
There was a problem hiding this comment.
Note to reviewers: when I wrote this test, I followed the pattern of other tests in the file that put problems directly under subsections. I have changed it now to represent the structure that would be created by Studio (problems in a unit).
Because of that, I can't use course_nav.sequence_items, which only returns the display name of the first component on that page (which would be the unit display name). If I put each problem into its own unit it also doesn't work because all students will see each unit, even if it has no content for a particular student.
20850ff to
4c7ae01
Compare
3804bd2 to
fb92e96
Compare
|
@dan-f responded to your feedback. I also needed to rebase on your branch, so your comments were lost in the force push (they were on a specific commit). I renamed the method as you requested, but didn't do the list comprehension thingy as I find that syntax too difficult to remember/understand. |
There was a problem hiding this comment.
You could collapse the above three blocks by iterating over [(self.cohort_a_student_username, self.cohort_a_student_email), (self.cohort_b_student_username, self.cohort_b_student_email), (self.cohort_default_student_username, self.cohort_default_student_email)].
There was a problem hiding this comment.
I still have to define the variables, so the only thing it would impact is the call to StudioAutoAuthPage. Does not seem worth it to me, as it makes the code less readable.
|
Awesome! 👍 |
There was a problem hiding this comment.
I think this would be easier to read if you called it all_xblock_components_contain_strings. It wasn't clear to me in the usages that they all had to be true, so I had to come back to the doc string to check.
There was a problem hiding this comment.
Removed the method, added helper method with more helpful asserts in one of the test classes.
|
👍 Fantastic tests, as always. |
fb92e96 to
fa221ea
Compare
|
@andy-armstrong and @dan-f I responded to your feedback, squashed, and got a green build. Since I already got thumbs-ups, I will go ahead and merge this. If you look the commit over and have any issues, let me know and I will follow up. |
…o-end-test End-to-end bok choy test for cohorted courseware.
3 participants
@dan-f and @andy-armstrong this is ready for review. It has already passed once (https://jenkins.testeng.edx.org/job/edx-all-tests-auto-pr/21383/).
Note that there are some quality violations that I'm picking up from Dan's branch.