openlibhums / janeway

A web-based platform for publishing journals, preprints, conference proceedings, and books
https://janeway.systems/
GNU Affero General Public License v3.0
168 stars 63 forks source link

Audit and document unit and integration testing procedures #3136

Open joemull opened 2 years ago

joemull commented 2 years ago

Describe the bug Janeway development teams use several different tools and testing procedures for unit and integration testing. Some of the differences between test file structures (app/tests.py vs. app/tests/tests.py), runners (nose vs. others), environments (jenkins vs. local installations), and database configurations (PostgresQL vs. SQLite) can lead to tests not being run, tests failing, and collaborators not sure what they should be seeing vs. what they are seeing.

Janeway version 1.4.3

Proposal Let's audit the test running procedures we use, including the development teams we collaborate with, and document a set of processes that works for everyone and is transparent for everyone.

joemull commented 2 years ago

@hardyoyo has started looking at some of these issues with https://github.com/BirkbeckCTP/janeway/pull/3108

joemull commented 2 years ago

@mauromsl points out it may be within scope to write a base test class for Janeway that tidies up all this.

joemull commented 1 year ago

@gamboz helpfully did some digging into related issues in May and June of this year. His notes are pasted below:

django standard

The usual way to run django tests is python manage.py test.

Beware that manage.py has been modified by Janeway to read settings.py and merge it with janeway_global_settings.py (the same is true for wsgi.py). But django-admin does not do that. E.g. django-admin test does not work out of the box, because it expects a single settings module.

A workaround is to merge janeway_global_settings.py and settings.py into, let's say wjs_settings.py and use it as DJANGO_SETTINGS_MODULE.

A possible way to merge the two files "automagically" may be as follow.\ Have a folder structure such as this:

src/core/janeway_global_settings.py
src/core/settings/
├──__init__.py
└── wjs_settings.py

where __init__.py contains what usually would go into the module core/settings.py (which does not exist any more)\ and wjs_settings.py contains this:

"""Merge janeway_global_settings and custom settings for pytest."""

from ..janeway_global_settings import *
from . import *

from ..janeway_global_settings import INSTALLED_APPS
from ..janeway_global_settings import MIDDLEWARE_CLASSES as default_middleware
from . import INSTALLED_APPS as custom_apps
from . import MIDDLEWARE_CLASSES as custom_middleware

INSTALLED_APPS.extend(custom_apps)
# MIDDLEWARE_CLASSES is a tuple, not a list
MIDDLEWARE_CLASSES = default_middleware + custom_middleware

As an added bonus, the folder src/settings is already in Janeway's .gitignore, so your repo stays clean.

pytest

pytest provides post-mortem debugging with one command line switch. This is very convenient, especially during development of tests.

To use pytest with django, install pytest-django.

Naturally pytest expects a single settings file, so the workaround described early must be used.

Configuration

pytest can be configured with a file such as the following (see also here):

# pytest.ini

[pytest]
DJANGO_SETTINGS_MODULE = core.wjs_settings

# recommended but optional:
python_files = tests.py test\_\*.py \*\_tests.py
filterwarnings =
    ignore::django.utils.deprecation.RemovedInDjango20Warning
    ignore::_pytest.warning_types.PytestCollectionWarning
    ignore::DeprecationWarning

The filterwarnings option is convenient because, with python 3.9, Janeway's tests emit \~150 warnings.

Exception import file mismatch

pytest will report exceptions about import file mismatch. It seems to be an issue related to pip install . (and similar, see here) and there is a workaround:

export PY_IGNORE_IMPORTMISMATCH=1

but the issue also suggests a possible "broken folder layout" (that I don't know how to estimate).

Testing an app

As of today (June '22) I have the wjs.jcom_profile package installed as a django app (i.e. not as a Janeway's plugin), and I run the tests from Janeway's src/ dir. So the automatic test discovery mechanism does not find the tests of my app. A simple workaround is to specify the source dir of the app:

pytest  ./ .../wjs-profile-project/wjs/ -k JCOM --reuse-db -v

(please note that the first positional argument ./, that indicates the cwd, is necessary to let pytest read pytest.ini configuration file.

Use -c .../pytest.ini to let pytest know where to find the ini file if necessary.

Some examples:

DJANGO_SETTINGS_MODULE=core.settings.test_j_settings pytest -Wignore::RuntimeWarning -c .../wjs-profile-project/pytest.ini src/core/

Quirks

The package dynamicsites contains a tests.py module with python-2 code, but, apparently the package is not used, so I just removed it (TBV!!).


CC0: This work has been marked as dedicated to the public domain.

gamboz commented 1 year ago

My notes as reported above are basically correct, but I'll keep adding to them here.

Also, I just casually saw that the last release of nose is 2015: https://pypi.org/project/nose/#history and it seems to be slowly dying:

Nose has been in maintenance mode for the past several years and will likely cease without a new person/team to take over maintainership. New projects should consider using Nose2, py.test, or just plain unittest/unittest2.

https://nose.readthedocs.io/en/latest/

joemull commented 8 months ago

I'll just note that this was a problem again today as CI tests were running differently than local tests. Also, Nose has been in maintenance mode for 9 years now, and Django Nose does not support Django 3 or Python 3.8+. We need to migrate away from Nose.