search

aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
436 stars 189 forks source link

Tests: Fixture implementation duplicated between deprecated `manage/tests/pytest_fixtures.py` and `tools/pytest_fixtures/` #6612

Open GeigerJ2 opened 4 hours ago

GeigerJ2 commented 4 hours ago

For instance, aiida_localhost in manage/tests/pytest_fixtures.py:

https://github.com/aiidateam/aiida-core/blob/dd866ce816e986285f2c5794f431b6e3c68a369b/src/aiida/manage/tests/pytest_fixtures.py#L657-L668

and in tools/pytest_fixtures/orm.py:

https://github.com/aiidateam/aiida-core/blob/dd866ce816e986285f2c5794f431b6e3c68a369b/src/aiida/tools/pytest_fixtures/orm.py#L193-L204

with manage/tests/pytest_fixtures.py being deprecated:

https://github.com/aiidateam/aiida-core/blob/dd866ce816e986285f2c5794f431b6e3c68a369b/src/aiida/manage/tests/pytest_fixtures.py#L54-L57

Of course, we still need to keep access from the old module for now for backwards compatibility, otherwise plugins break, e.g.:

https://github.com/aiidateam/aiida-quantumespresso/blob/57e7463c5727775d6a0470a41d1aca0ec4083b9a/tests/conftest.py#L13

But rather than duplicating the source code as it seems to be done now, we should let the fixtures in manage/tests/pytest_fixtures.py call the actual implementation in tools/pytest_fixtures/orm.py. Pinging the aiida-core office @agoscinski, @unkcpz, @khsrali.

unkcpz commented 3 hours ago

I totally agree. I think the doc page https://aiida.readthedocs.io/projects/aiida-core/en/stable/topics/plugins.html#plugin-test-fixtures Is very helpful to show what fixtures that plugin developers can use. We may also want to add same paper for public APIs.

danielhollas commented 39 minutes ago

I honestly don't know if it is worth the churn to be refactoring deprecated functions. I think it is easier to let them be and eventually remove them, and only do new development in the new ones.