-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
test: run ./xmodule/ tests with CMS settings too #33534
Conversation
3bfea38
to
d409212
Compare
Just like ./common/ and ./openedx/, the unit tests in ./xmodule/ validate behavior in both LMS and CMS. In order to fully test ./xmodule/, we need to run its tests in both contexts. Also in this commit: * refactor: Rename the shards to be clear whether they're running under LMS or CMS. * docs: correct comments regarding conditions under which codejail's test_cant_do_something_forbidden is skipped. * test: update a unit test which was using the now-deleted library_sourced block to use library_content block instead.
d409212
to
8fe86a1
Compare
sourced_block = self.make_block("library_sourced", course, user_id=self.user.id) | ||
lc_block = self.make_block("library_content", course, user_id=self.user.id) |
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.
This change (library_sourced being deleted in favor of library_content) happened in the source code few weeks ago. The fact that this test case was still passing (under LMS settings) means that test wasn't working right!
When run under CMS settings, though, the test case did fail (like it should have).
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, pending those last LMS tests passing (they are still running.)
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
2U Release Notice: This PR has been deployed to the edX production environment. |
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
2U Release Notice: This PR has been deployed to the edX production environment. |
Currently, ./xmodule/ unit tests are only run with LMS settings. However, ./common/ and ./xmodule/ are run twice: once with LMS settings and once with CMS settings.
Just like ./common/ and ./openedx/, the unit tests in ./xmodule/ validate behavior in both LMS and CMS. So, order to fully test ./xmodule/, we should to run its tests with CMS settings too.
This will enable us to better validate certain LibraryContentBlocks behaviors being touched by #33263 which can't be expressed under LMS settings.
Also in this commit:
refactor: rename the shards to be clear whether they're running under LMS or CMS.
docs: correct comments regarding conditions under which codejail's test_cant_do_something_forbidden is skipped.
test: update a unit test which was using the now-deleted block to use library_content block instead.
Before and after merging, I will update branch protection settings to account for the shard changes.