aiidateam / aiida-core

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

[devops] Run tests with sqlite backend #6425

Closed danielhollas closed 3 weeks ago

danielhollas commented 1 month ago

The purpose of this PR is twofold:

  1. Being able to run the test suite locally without requiring any services.
  2. Run the full test suite with sqlite_dos as a storage backend

Two new pytest markers are added to the test suite:

Two new CI jobs are added:

In exchange for introducing two new ones, I removed the two verdi jobs and moved the corresponding tests (such as verdi devel check-imports after the normal test run. These are super fast and setting them up separately takes much more time then running them. Happy to discuss this though.

sphuber commented 1 month ago

You can use this for the config_sqlite_dos fixture:

(aiida-py311) sph@invader:~/code/aiida/env/dev/aiida-core$ git stash show stash@{0} -p
diff --git a/src/aiida/tools/pytest_fixtures/__init__.py b/src/aiida/tools/pytest_fixtures/__init__.py
index 6181183d1..79d027f02 100644
--- a/src/aiida/tools/pytest_fixtures/__init__.py
+++ b/src/aiida/tools/pytest_fixtures/__init__.py
@@ -25,7 +25,7 @@ from .orm import (
     aiida_localhost,
     ssh_key,
 )
-from .storage import config_psql_dos, postgres_cluster
+from .storage import config_psql_dos, config_sqlite_dos, postgres_cluster

 __all__ = (
     'aiida_code_installed',
diff --git a/src/aiida/tools/pytest_fixtures/storage.py b/src/aiida/tools/pytest_fixtures/storage.py
index d76c9b445..e216df664 100644
--- a/src/aiida/tools/pytest_fixtures/storage.py
+++ b/src/aiida/tools/pytest_fixtures/storage.py
@@ -2,6 +2,7 @@

 from __future__ import annotations

+import pathlib
 import typing as t

 import pytest
@@ -85,3 +86,24 @@ def config_psql_dos(
         return storage_config

     return factory
+
+
+
+@pytest.fixture(scope='session')
+def config_sqlite_dos(
+    tmp_path_factory: pytest.TempPathFactory,
+) -> t.Callable[[str | None, str | None, str | None], dict[str, t.Any]]:
+    """Return a profile configuration for the :class:`~aiida.storage.sqlite_dos.backend.SqliteDosStorage`.
+
+    The factory has the following signature to allow further configuring the database that is created:
+
+    :param database_name: Name of the database to be created.
+    :returns: The dictionary with the storage configuration for the ``core.sqlite_dos`` storage plugin.
+    """
+
+    def factory(filepath: str | pathlib.Path | None = None) -> dict[str, t.Any]:
+        return {
+            'filepath': str(filepath or tmp_path_factory.mktemp('test_sqlite_dos_storage'))
+        }
+
+    return factory
diff --git a/tests/conftest.py b/tests/conftest.py
index 55bf01a18..4c7f7f7d0 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -35,7 +35,7 @@ pytest_plugins = ['aiida.tools.pytest_fixtures', 'sphinx.testing.fixtures']

 @pytest.fixture(scope='session')
-def aiida_profile(aiida_config, aiida_profile_factory, config_psql_dos):
+def aiida_profile(aiida_config, aiida_profile_factory, config_psql_dos, config_sqlite_dos):
     """Create and load a profile with ``core.psql_dos`` as a storage backend and RabbitMQ as the broker.

     This overrides the ``aiida_profile`` fixture provided by ``aiida-core`` which runs with ``core.sqlite_dos`` and
@@ -43,7 +43,7 @@ def aiida_profile(aiida_config, aiida_profile_factory, config_psql_dos):
     be run against the main storage backend, which is ``core.sqlite_dos``.
     """
     with aiida_profile_factory(
-        aiida_config, storage_backend='core.psql_dos', storage_config=config_psql_dos(), broker_backend='core.rabbitmq'
+        aiida_config, storage_backend='core.sqlite_dos', storage_config=config_sqlite_dos(), broker_backend='core.rabbitmq'
     ) as profile:
         yield profile
agoscinski commented 1 month ago

Thanks for the PR! This would be super useful for running tests in tox locally. Will try it out.

danielhollas commented 1 month ago

@agoscinski thanks for taking a look! I will be working on finishing this up today so you might want to wait with testing till tomorrow.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 77.80%. Comparing base (ef60b66) to head (4ba5e09). Report is 23 commits behind head on main.

Files Patch % Lines
src/aiida/tools/pytest_fixtures/storage.py 92.60% 2 Missing :warning:
...ida/storage/psql_dos/migrations/utils/integrity.py 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6425 +/- ## ========================================== + Coverage 77.51% 77.80% +0.30% ========================================== Files 560 562 +2 Lines 41444 41788 +344 ========================================== + Hits 32120 32511 +391 + Misses 9324 9277 -47 ``` | [Flag](https://app.codecov.io/gh/aiidateam/aiida-core/pull/6425/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam) | Coverage Δ | | |---|---|---| | [presto](https://app.codecov.io/gh/aiidateam/aiida-core/pull/6425/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam) | `73.14% <70.00%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

danielhollas commented 1 month ago

@agoscinski to fix the SSH errors in your local setup, take a look at the setup_ssh.sh script that I extracted from .github/workflows/setup.sh: https://github.com/aiidateam/aiida-core/pull/6425/files#diff-55611b6e2b8ad89832bc6695fffc24acdcc8f34bfb2d086abf01b110cfa33e01

Note that before running ssh-keyscan I had to restart the SSH Daemon with systemctl restart sshd. You might need to do something similar on your system.

danielhollas commented 1 month ago

@sphuber @agoscinski apart from two small TODOs this is now ready for review / testing.

EDIT: Both TODOs are now resolved.

danielhollas commented 1 month ago

Why does every PR that touched GHA end up with tens of commits? :sob:

@sphuber this should now be ready. There might be some controversial changes to the test suite, happy to discuss. (see the updated OP)