-
-
Notifications
You must be signed in to change notification settings - Fork 218
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
Replace nose with pytest #34778
Replace nose with pytest #34778
Changes from 4 commits
77ca2d2
aba41f6
ec95345
18b98e9
4d44b95
40b26c2
3d22e5c
e56d177
2efd892
66663ce
8372914
c3b3c75
1ae6ea1
a834219
aa24be1
9613cdf
3d5c1a1
a513405
0ca69fd
069391e
9b8137f
912e55c
421f770
9b523cd
e305b74
68219e5
5ae9b5e
c58537c
214546f
2253349
e7650e0
509dddc
05d1a10
e33e25f
cd7c0a8
946173a
ca33bd1
7ebdc62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
[pytest] | ||
minversion = 8.1 | ||
|
||
addopts = | ||
--strict-markers | ||
-pcorehq.tests.pytest_hooks | ||
# HQ has its own (incompatible) warnings system | ||
-pno:warnings | ||
|
||
empty_parameter_set_mark = xfail | ||
xfail_strict = true | ||
|
||
norecursedirs = | ||
docker | ||
node_modules | ||
staticfiles | ||
|
||
pythonpath = | ||
. | ||
|
||
required_plugins = pytest-django | ||
DJANGO_SETTINGS_MODULE = testsettings |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import sys | ||
|
||
from .tools import nottest as nottest_tool | ||
|
||
|
||
def create_nose_virtual_package(): | ||
sys.modules['nose.tools'] = VirtualNose.tools | ||
|
||
|
||
class VirtualNose: | ||
"""Legacy namespace for tests written before pytest""" | ||
class tools: | ||
nottest = nottest_tool |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +0,0 @@ | ||
import corehq.tests.pytest_compat # noqa: F401 | ||
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
import os | ||
|
||
import pytest | ||
|
||
|
||
@pytest.hookimpl(tryfirst=True) | ||
def pytest_load_initial_conftests(): | ||
assert not hasattr(pytest_load_initial_conftests, 'loaded'), "Already loaded" | ||
pytest_load_initial_conftests.loaded = True | ||
os.environ.setdefault('CCHQ_TESTING', '1') | ||
|
||
from manage import init_hq_python_path, run_patches | ||
init_hq_python_path() | ||
run_patches() | ||
|
||
from corehq.warnings import configure_warnings | ||
configure_warnings(is_testing=True) | ||
|
||
from .nosecompat import create_nose_virtual_package | ||
create_nose_virtual_package() | ||
|
||
|
||
def pytest_pycollect_makeitem(collector, name, obj): | ||
"""Fail on common mistake that results in masked tests""" | ||
if ( | ||
"Test" in name | ||
and not isinstance(obj, type) | ||
and isinstance(wrapped := _get_wrapped(obj), type) | ||
and any(n.startswith("test_") for n in dir(wrapped)) | ||
): | ||
return pytest.fail( | ||
f"{obj.__module__}.{name} appears to be a test class that has " | ||
"been wrapped with a decorator that masks its tests." | ||
) | ||
return None | ||
|
||
|
||
def _get_wrapped(obj): | ||
while hasattr(obj, "__wrapped__"): | ||
obj = obj.__wrapped__ | ||
return obj |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,6 @@ | ||
-r requirements.in | ||
|
||
django-nose @ https://github.com/dimagi/django-nose/raw/fast-first-1.4.6.1/releases/django_nose-1.4.6.1-py2.py3-none-any.whl | ||
fakecouch | ||
nose | ||
nose-exclude | ||
pip-tools>=6.8.0 | ||
testil | ||
requests-mock | ||
|
@@ -12,5 +9,9 @@ Faker | |
flaky | ||
freezegun | ||
radon>=5.0 # v5 no longer depends on flake8-polyfill | ||
# pytest is pinned to a git commit until 8.4 is released. | ||
# `minversion = 8.4` should also be set in .pytest.ini at that time. | ||
pytest @ git+https://github.com/pytest-dev/pytest.git@85760bff2776989b365167c7aeb35c86308ab76b | ||
Comment on lines
+12
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What feature are we using that will be in 8.4? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
pytest-django | ||
coverage | ||
uWSGI |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,8 @@ | |
from warnings import filterwarnings | ||
|
||
import settingshelper as helper | ||
from settings import * # noqa: F403 | ||
assert helper.is_testing(), 'test mode is required before importing settings' | ||
from settings import * # noqa: E402, F403 | ||
|
||
# Commenting out temporarily for tests | ||
# if os.environ.get('ELASTICSEARCH_MAJOR_VERSION'): | ||
|
@@ -53,45 +54,12 @@ | |
# it can be reverted whenever that's figured out. | ||
# https://github.com/dimagi/commcare-hq/pull/10034#issuecomment-174868270 | ||
INSTALLED_APPS = ( | ||
'django_nose', | ||
'testapps.test_elasticsearch', | ||
'testapps.test_pillowtop', | ||
) + tuple(INSTALLED_APPS) # noqa: F405 | ||
|
||
TEST_RUNNER = 'django_nose.BasicNoseRunner' | ||
NOSE_ARGS = [ | ||
#'--no-migrations' # trim ~120s from test run with db tests | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should look to see if test run time increases by ~120s after switching to pytest. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the answer is possibly "yes." The average build time does appear to have increased from 14 minutes to 16 minutes soon after Nov 11 when this PR was merged. |
||
#'--with-fixture-bundling', | ||
] | ||
NOSE_PLUGINS = [ | ||
'corehq.tests.nose.HqTestFinderPlugin', | ||
'corehq.tests.noseplugins.classcleanup.ClassCleanupPlugin', | ||
'corehq.tests.noseplugins.dbtransaction.DatabaseTransactionPlugin', | ||
'corehq.tests.noseplugins.dividedwerun.DividedWeRunPlugin', | ||
'corehq.tests.noseplugins.djangomigrations.DjangoMigrationsPlugin', | ||
'corehq.tests.noseplugins.cmdline_params.CmdLineParametersPlugin', | ||
'corehq.tests.noseplugins.patches.PatchesPlugin', | ||
'corehq.tests.noseplugins.redislocks.RedisLockTimeoutPlugin', | ||
'corehq.tests.noseplugins.uniformresult.UniformTestResultPlugin', | ||
|
||
# The following are not enabled by default | ||
'corehq.tests.noseplugins.logfile.LogFilePlugin', | ||
'corehq.tests.noseplugins.timing.TimingPlugin', | ||
'corehq.tests.noseplugins.output.OutputPlugin', | ||
'corehq.tests.noseplugins.elasticsnitch.ElasticSnitchPlugin', | ||
|
||
# Uncomment to debug tests. Plugins have nice hooks for inspecting state | ||
# before/after each test or context setup/teardown, etc. | ||
#'corehq.tests.noseplugins.debug.DebugPlugin', | ||
] | ||
|
||
# these settings can be overridden with environment variables | ||
for key, value in { | ||
'NOSE_DB_TEST_CONTEXT': 'corehq.tests.nose.HqdbContext', | ||
'NOSE_NON_DB_TEST_CONTEXT': 'corehq.tests.nose.ErrorOnDbAccessContext', | ||
'NOSE_IGNORE_FILES': '^localsettings', | ||
'NOSE_EXCLUDE_DIRS': 'scripts', | ||
|
||
'DD_DOGSTATSD_DISABLE': 'true', | ||
'DD_TRACE_ENABLED': 'false', | ||
}.items(): | ||
|
@@ -146,7 +114,6 @@ def _set_logging_levels(levels): | |
'urllib3': 'WARNING', | ||
}) | ||
|
||
# use empty LOGGING dict with --debug=nose,nose.plugins to debug test discovery | ||
# TODO empty logging config (and fix revealed deprecation warnings) | ||
LOGGING = { | ||
'disable_existing_loggers': False, | ||
|
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.
Should configure_warnings() take the CCHQ_TESTING env var into account?
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.
Fair question. Should a management command (that is not running tests) fail because of warnings if
CCHQ_TESTING
is set? For example, if you are runningCCHQ_TESTING=1 ./manage.py migrate
to migrate your test database, should it crash if warnings are emitted? I think that might be more annoying than helpful, but I'm happy to hear arguments in favor.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.
We reckoned that
CCHQ_TESTING=1 ./manage.py dbshell
would useful for inspecting the test database.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.
Are you saying you agree that that command should not crash on a warning? That's my position.