Ensure edx-platform tests can run without fiddling with environment variables #648
Conversation
Many edx-platform unit tests need `EDXAPP_MONGO_TEST_HOST` to be set to the hostname of the Mongo container (for Tutor, that's "mongodb"). Previously, devs have needed to set this environment variable by hand before running those tests. Since there's no harm in setting this variable for the entire container, we just set it in env/dev/docker-compose.yml for lms and cms.
Also, explain some of the unavoidable complexity around the overlap of the LMS and CMS suites, and how to best run tests in all parts of the LMS/CMS Venn diagram. Dependent on an upstream fix, which makes it so devs don't need to fiddle with DJANGO_SETTINGS_MODULE: openedx/edx-platform#30298 Resolves https://github.com/overhangio/2u-tutor-adoption/issues/48
|
@regisb and @bradenmacdonald , here's my first stab at getting |
| pytest cms # Run tests for the entire CMS source tree. | ||
| pytest cms/djangoapps/coursegraph # Run tests in a single CMS app. | ||
| pytest --rootdir=cms openedx common # Run tests for both shared source trees. | ||
| pytest --rootdir=cms openedx/core/djangoapps/content/course_overviews # Run tests for a single shared app. |
There was a problem hiding this comment.
This contradicts the instruction from line 268. I'm confused: do we need to specify the rootdir when running shared app tests?
There was a problem hiding this comment.
@regisb Right, sorry, it's a little convoluted.
By default, ALL tests will use lms.envs.test (because of edx-platform's setup.cfg) EXCEPT the tests under cms/, which will use cms.envs.test (because of edx-platform's cms/pytest.ini).
This is generally reasonable, except that sometimes folks will want to run tests for the "shared" directories (openedx/ and common/) in a CMS context. The easiest way to make this happen is to tell pytest to look at the cms/ directory for a pytest.ini, hence --rootdir=cms.
As I think more about this, I really wish we could just have the behavior of:
- tests in lms/ always use
lms.envs.test, - tests in cms/ always use
cms.envs.test, - tests in shared directories use
lms.envs.testwhen in the LMS container, and usecms.envs.testwhen in the CMS container.
Orchestrating that will take some hair-pulling. For the conference, I think we'll just have to accept the status quo. The current instructions, while a little annoying, certainly work for the vast majority of edx-platform tests :)
I can't remember, unfortunately. Did you manage to run the unit tests without these statements? (fun fact: I opened 8d803fb in a different tab in my browser and started reviewing it, incorrectly believing it was actually your PR, and thinking wow this is horrible there is no way we are making these changes...) |
|
Closing for now per #648 (comment) |
Description
See the commit message for details.
Blocked by openedx/edx-platform#30298
Resolves https://github.com/overhangio/2u-tutor-adoption/issues/48
Considerations
This PR (and the linked edx-platform PR) would succeed at getting pytest to default to
[lms|cms].envs.tests. However, I haven't come up with an elegant way of getting pytest to default to[lms,cms].envs.tutor.test, which adds this change to the upstream test settings:In trying to find out why we need to change DOC_STORE_CONFIG connection credentials, git blame leads me to 8d803fb. @regisb , do you remember why you why need to change these credentials? Do you think it's possible we could work around this some other way, allowing us to use the upstream test settings?
Screenshot of changed docs section