I recently started a development task on edx-platform, and it’s been a while since the last time I submitted a PR that was not a simple bug fix on edx-platform. So I find myself running the unit tests locally – as opposed to just waiting for Jenkins to run them for me.
Unless I’m mistaken, the official recommendation has always been to run the tests with paver test_* commands. But these days the documentation is a bit fuzzy on the topic. I could not find an up-to-date recommendation: https://edx.readthedocs.io/projects/edx-developer-guide/en/latest/testing/index.html
Anyway, I tried to run the tests with paver, but this command was clearly not designed for humans (as opposed to CI). I found the following issues:
- It takes a very long time to run, as it performs many test bootstrapping tasks, such as checking Python prerequirements, cleaning xml reports, etc.
- Failed test results and stacktraces are not printed in the console, but instead stored in xml reports.
- Running unit tests on the current master branches causes many deprecation warnings to be printed in the console. Here is an excerpt:
lms/djangoapps/lti_provider/users.py:18
/edx/app/edxapp/edx-platform/lms/djangoapps/lti_provider/users.py:18: SysPathHackWarning: Importing lti_provider.models instead of lms.djangoapps.lti_provid
er.models is deprecated
from lti_provider.models import LtiUser
../venvs/edxapp/lib/python3.5/site-packages/sorl/thumbnail/conf/__init__.py:16
/edx/app/edxapp/venvs/edxapp/lib/python3.5/site-packages/sorl/thumbnail/conf/__init__.py:16: RemovedInDjango30Warning: The DEFAULT_CONTENT_TYPE setting is d
eprecated.
setattr(self, attr, getattr(obj, attr))
../venvs/edxapp/lib/python3.5/site-packages/sorl/thumbnail/conf/__init__.py:16
/edx/app/edxapp/venvs/edxapp/lib/python3.5/site-packages/sorl/thumbnail/conf/__init__.py:16: RemovedInDjango31Warning: The FILE_CHARSET setting is deprecate
d. Starting with Django 3.1, all files read from disk must be UTF-8 encoded.
setattr(self, attr, getattr(obj, attr))
lms/djangoapps/bulk_email/forms.py:11
/edx/app/edxapp/edx-platform/lms/djangoapps/bulk_email/forms.py:11: SysPathHackWarning: Importing bulk_email.models instead of lms.djangoapps.bulk_email.mod
els is deprecated
from bulk_email.models import COURSE_EMAIL_MESSAGE_BODY_TAG, CourseAuthorization, CourseEmailTemplate
../venvs/edxapp/lib/python3.5/site-packages/django/test/testcases.py:887
/edx/app/edxapp/venvs/edxapp/lib/python3.5/site-packages/django/test/testcases.py:887: RemovedInDjango31Warning: `TestCase.multi_db` is deprecated. Database
s available during this test can be defined using lms.djangoapps.dashboard.tests.test_sysadmin.SysadminBaseTestCase.databases.
warnings.warn(msg, RemovedInDjango31Warning)
...
There are so many of those deprecation warnings that the test output extends past my console history.
I managed to address issues 1 by running:
NO_PREREQ_INSTALL=1 paver test_system --fail-fast --fasttest --skip-clean --system=lms
But clearly this is not a practical command for humans to remember. Also, it does not resolve issue 2.
The, I realized that paver commands are equivalent to simple pytest commands. So instead, I now run:
pytest --ds=lms.envs.test --exitfirst openedx/core/
This simple enough command addresses issues 1 and 2. Of course, specific development efforts need to be made to resolve issue 3.
The PythonTestSuite in edx-platform was created in 2014, and was little modified since then. It probably had its merits then, but I wonder whether we should now avoid recommending to use it, and move to a pytest-based set of commands?
I’m curious to hear how the rest of the community runs unit tests in edx-platform; maybe I’m doing this the wrong way?
of realizing the paver strange behavior and I agree with the intention to move toward the pytest-based way.