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

Move tests to pytest #3653

Merged
merged 2 commits into from
Dec 12, 2019
Merged

Conversation

ltalirz
Copy link
Member

@ltalirz ltalirz commented Dec 12, 2019

fixes #2771
affects #2835, #3602

This PR starts running the tests of aiida-core through pytest.
Changes to the AiiDATestCase class are kept minimal.
The code for verdi devel tests is removed, the command is deprecated and now prints the suggestion to use pytest.

To do, but not in this PR:

  • Fix (py)test warnings (will touch lots of files)
  • Move test files out of aiida-core inside top-level tests/ folder
  • Where it makes sense, start moving existing tests from the AiiDATestCase to pytest (e.g. where pytest fixtures are making things more effective)

from aiida.backends import BACKEND_DJANGO
from aiida.manage.tests import get_test_backend_name

if get_test_backend_name() != BACKEND_DJANGO:
Copy link
Member Author

@ltalirz ltalirz Dec 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice there is still a minor issue here.

If you set AIIDA_TEST_PROFILE, the test manager will simply use that profile and its backend (ignoring the value of AIIDA_TEST_BACKEND).

I.e. when you set the AIIDA_TEST_PROFILE variable to an sqlalchemy test profile without also setting AIIDA_TEST_BACKEND=sqlalchemy, then the pytest test discovery will think you are running on the default django backend and discover the wrong tests.

@greschd and me briefly looked into whether pytest offers a way to hook custom code into before test discovery but we didn't find a way to do that.
However, one could try to get information from the profile inside the conftest.py (without loading the dbenv).
Unless there are better suggestions, I will need to implement this.

Note: This does not affect the CI setup, where the AIIDA_TEST_BACKEND variable is set correctly.

Copy link
Member

@greschd greschd Dec 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@greschd and me briefly looked into whether pytest offers a way to hook custom code into before test discovery but we didn't find a way to do that.

If I remember correctly there are ways to hook custom code into pytest, but it's just not as simple as adding a pytest.skip somewhere.

Here's the API reference for pytest hooks: https://docs.pytest.org/en/latest/reference.html#hook-reference

Either pytest_configure or pytest_sessionstart seems to be the right place for that code.

I'm guessing the "wrong backend" issue can occur any time the fixtures are used, not only for the aiida-core tests (i.e., also for plugin testing).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either pytest_configure or pytest_sessionstart seems to be the right place for that code.

This looks promising. If you can find the time to look into this, that would be great.

I'm guessing the "wrong backend" issue can occur any time the fixtures are used, not only for the aiida-core tests (i.e., also for plugin testing).

Yes, but it only affects test discovery and in practice plugins are unlikely to have backend-dependent tests.


for module in modules_base:
test_modules[module] = None
Defunct - remove when removing the "verdi devel tests" command.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add the deprecation warning using v2.0.0 because that way it is easy to search the code base when removing deprecated stuff

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

Pytest is a powerful test framework that is already used by most aiida
plugins. This PR uses the pytest fixtures (actually, onlty the
aiida_profile fixture so far) in order to the AiiDA unit tests.

 * Fix errors at collection stage by using pytest.skip() in the setUp or
   the test function itself instead of using a decorator.
   Using 'collect_ignore_glob' to select tests by backend.
 * Run tests with pytest in GitHub actions.
 * remove unused tearDown_method, setUp_method
 * fix setupclass
@ltalirz ltalirz force-pushed the move_tests_to_pytest_rebased branch from d535946 to f2c407f Compare December 12, 2019 21:24
@ltalirz ltalirz force-pushed the move_tests_to_pytest_rebased branch from f2c407f to da93dd8 Compare December 12, 2019 21:45
@sphuber sphuber self-requested a review December 12, 2019 22:00
Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great success!

@sphuber sphuber merged commit 87a7d34 into aiidateam:develop Dec 12, 2019
@sphuber sphuber deleted the move_tests_to_pytest_rebased branch December 12, 2019 22:01
@ltalirz ltalirz mentioned this pull request Dec 13, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

move aiida-core tests to pytest
3 participants