Skip to content
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

Fix lms & cms container environments to run tests without modification #23

Open
Tracked by #202
kdmccormick opened this issue Feb 15, 2022 · 9 comments
Open
Tracked by #202
Labels
enhancement Relates to new features or improvements to existing features help wanted Ready to be picked up by anyone in the community tutor Requires a change to Tutor

Comments

@kdmccormick
Copy link
Member

kdmccormick commented Feb 15, 2022

Context

Here are the instructions on how to run tests in Tutor: https://docs.tutor.overhang.io/dev.html#running-edx-platform-unit-tests

Notice that:

  1. the lms container must be used, even for cms tests
  2. one needs to fiddle with a few different environment variables in order to run pytest on each root folder (lms, cms, common, openedx, xmodule).

Both of these limitations are awkward and potential stumbling blocks for developers.

Note on lms vs cms tests

edx-platform runs in one of two modes: LMS (the learning/instruction interface & APIs) and CMS (Studio and authoring APIs). Some code is specific to LMS or CMS, and some code is shared between the two. Both LMS and CMS have their own Django settings files.

In the edx-platform source tree...

  • The tests under ./lms/ are LMS-specific and should only be run with LMS settings.
  • The tests under ./cms/ are CMS-specific and should only be run with CMS settings.
  • The tests under ./openedx/, ./common/, and ./xmodule/ are "shared". That, the code could be used by both LMS and CMS, so we want to be able to run unit tests both using LMS settings and CMS settings, and the tests should pass under both contexts.
    • Exceptions: Some unit tests in these directories are LMS-specific or CMS-specific. Those tests are marked with decorators @skip_unless_lms and @skip_unless_cms, respectively.

edx-platform's PR checks ensure that the above is always true on the master branch.

Acceptance

Tweak edx-platform's test setup and/or Tutor's cms+lms dev container environments such that, without having to change any environment variables:

  • One can run all LMS-specific tests and shared tests in the lms container using LMS Django settings.
  • One can run all CMS-specific tests and shared tests in the cms container using CMS Django settings.

In other words, these should just work:

tutor dev run lms pytest lms/ xmodule/ common/ openedx/
tutor dev run cms pytest cms/ xmodule/ common/ openedx/

Notes

@kdmccormick
Copy link
Member Author

@bradenmacdonald can you just leave a comment here? GH won't let me make you an assignee until you do.

@bradenmacdonald
Copy link

@kdmccormick Sure.

@kdmccormick
Copy link
Member Author

@bradenmacdonald I'm going to start poking at this as I have time this week, and I'll post here if I learn anything. I don't know if your situation makes you more or less likely to work on this 😛 but if you do, let me know what you find!

@bradenmacdonald
Copy link

Ok sounds good @kdmccormick. Please let me know what you end up finding either way :)

kdmccormick referenced this issue in kdmccormick/tutor Apr 21, 2022
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
@kdmccormick
Copy link
Member Author

kdmccormick commented Apr 21, 2022

kdmccormick referenced this issue in kdmccormick/tutor Apr 21, 2022
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
@kdmccormick
Copy link
Member Author

The two PRs above are ready for feedback.

@kdmccormick
Copy link
Member Author

Going to come back to this issue after the conference. See overhangio/tutor#648 (comment)

@kdmccormick kdmccormick removed their assignment Oct 12, 2022
@kdmccormick
Copy link
Member Author

unfortunately I haven't been able to dig into this more, so I'm unassigning myself for now.

@bradenmacdonald were you still hoping to look into this, or should I unassign you?

@bradenmacdonald
Copy link

@kdmccormick I don't think I'll have time anytime soon unfortunately.

@kdmccormick kdmccormick changed the title Fix edx-platform so that it doesn't need special environment vars to run tests Fix lms & cms container environments to run tests without modification Nov 17, 2022
@kdmccormick kdmccormick added the enhancement Relates to new features or improvements to existing features label Jun 2, 2023
@kdmccormick kdmccormick added the help wanted Ready to be picked up by anyone in the community label Oct 18, 2023
@kdmccormick kdmccormick transferred this issue from openedx-unsupported/wg-developer-experience Mar 28, 2024
@kdmccormick kdmccormick added the tutor Requires a change to Tutor label Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Relates to new features or improvements to existing features help wanted Ready to be picked up by anyone in the community tutor Requires a change to Tutor
Projects
No open projects
Status: Refined
Status: No status
Development

No branches or pull requests

2 participants