Skip to content

Commit

Permalink
Tests: add --db-backend pytest option (#6625)
Browse files Browse the repository at this point in the history
Instead of choosing the database backend (SQLite or Postgres) implicitly via markers, we add an explicit `--db-backend ` option to pytest.

```
  --db-backend=DB_BACKEND
                        Database backend to be used for tests ('sqlite', 'psql')
```

- The default backend was changed from `psql` to `sqlite` to make local testing easier. 
- The tests running in CI are still using Postgres in most places.
- The only change in CI should be switching the rabbitmq tests and release tests to use sqlite.

Error if the user provides an invalid option

```
❯ pytest --db-backend=invalid
ERROR: Invalid --db-backend option 'invalid'
Must be one of: ('sqlite', 'psql')
```

To clarify, the existing presto marker works as before.
  • Loading branch information
danielhollas authored Nov 25, 2024
1 parent 1c5f109 commit a863d1e
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 55 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ jobs:
python-version: '3.10'

- name: Run benchmarks
run: pytest --benchmark-only --benchmark-json benchmark.json
run: pytest --db-backend psql --benchmark-only --benchmark-json benchmark.json tests/

- name: Store benchmark result
uses: aiidateam/github-action-benchmark@v3
Expand All @@ -73,4 +73,4 @@ jobs:
alert-threshold: 200%
comment-on-alert: true
fail-on-alert: false
alert-comment-cc-users: '@chrisjsewell,@giovannipizzi'
alert-comment-cc-users: '@giovannipizzi,@agoscinski,@GeigerJ2,@khsrali,@unkcpz'
4 changes: 2 additions & 2 deletions .github/workflows/ci-code.yml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ jobs:
AIIDA_WARN_v3: 1
# Python 3.12 has a performance regression when running with code coverage
# so run code coverage only for python 3.9.
run: pytest -v tests -m 'not nightly' ${{ matrix.python-version == '3.9' && '--cov aiida' || '' }}
run: pytest -v --db-backend psql -m 'not nightly' tests/ ${{ matrix.python-version == '3.9' && '--cov aiida' || '' }}

- name: Upload coverage report
if: matrix.python-version == 3.9 && github.repository == 'aiidateam/aiida-core'
Expand Down Expand Up @@ -139,7 +139,7 @@ jobs:
- name: Run test suite
env:
AIIDA_WARN_v3: 0
run: pytest -m 'presto'
run: pytest -m 'presto' tests/


verdi:
Expand Down
18 changes: 1 addition & 17 deletions .github/workflows/nightly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,6 @@ jobs:
rabbitmq-version: ['3.11', '3.12', '3.13']

services:
postgres:
image: postgres:16
env:
POSTGRES_DB: test_aiida
POSTGRES_PASSWORD: ''
POSTGRES_HOST_AUTH_METHOD: trust
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 5
ports:
- 5432:5432
rabbitmq:
image: rabbitmq:${{ matrix.rabbitmq-version }}-management
ports:
Expand All @@ -132,9 +119,6 @@ jobs:
with:
python-version: '3.11'

- name: Install system dependencies
run: sudo apt update && sudo apt install postgresql

- name: Setup SSH on localhost
run: .github/workflows/setup_ssh.sh

Expand All @@ -145,7 +129,7 @@ jobs:
id: tests
env:
AIIDA_WARN_v3: 0
run: pytest -sv -m 'requires_rmq'
run: pytest -sv --db-backend sqlite -m 'requires_rmq' tests/

- name: Slack notification
# Always run this step (otherwise it would be skipped if any of the previous steps fail) but only if the
Expand Down
20 changes: 1 addition & 19 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,6 @@ jobs:
timeout-minutes: 30

services:
postgres:
image: postgres:10
env:
POSTGRES_DB: test_aiida
POSTGRES_PASSWORD: ''
POSTGRES_HOST_AUTH_METHOD: trust
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 5
ports:
- 5432:5432
rabbitmq:
image: rabbitmq:3.8.14-management
ports:
Expand All @@ -76,16 +63,11 @@ jobs:
steps:
- uses: actions/checkout@v4

- name: Install system dependencies
run: |
sudo apt update
sudo apt install postgresql graphviz
- name: Install aiida-core
uses: ./.github/actions/install-aiida-core

- name: Run sub-set of test suite
run: pytest -sv -k 'requires_rmq'
run: pytest -sv -m requires_rmq --db-backend=sqlite tests/

publish:

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ jobs:
env:
AIIDA_TEST_PROFILE: test_aiida
AIIDA_WARN_v3: 1
run: pytest --verbose tests -m 'not nightly'
run: pytest -v --db-backend psql tests -m 'not nightly' tests/

- name: Freeze test environment
run: pip freeze | sed '1d' | tee requirements-py-${{ matrix.python-version }}.txt
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests_nightly.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ verdi -p test_aiida run ${SYSTEM_TESTS}/test_containerized_code.py
bash ${SYSTEM_TESTS}/test_polish_workchains.sh
verdi daemon stop

AIIDA_TEST_PROFILE=test_aiida pytest -v tests -m 'nightly'
AIIDA_TEST_PROFILE=test_aiida pytest -v --db-backend psql -m nightly tests/
52 changes: 48 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import types
import typing as t
import warnings
from enum import Enum
from pathlib import Path

import click
Expand All @@ -37,7 +38,14 @@
pytest_plugins = ['aiida.tools.pytest_fixtures', 'sphinx.testing.fixtures']


def pytest_collection_modifyitems(items):
class TestDbBackend(Enum):
"""Options for the '--db-backend' CLI argument when running pytest."""

SQLITE = 'sqlite'
PSQL = 'psql'


def pytest_collection_modifyitems(items, config):
"""Automatically generate markers for certain tests.
Most notably, we add the 'presto' marker for all tests that
Expand All @@ -47,6 +55,14 @@ def pytest_collection_modifyitems(items):
filepath_django = Path(__file__).parent / 'storage' / 'psql_dos' / 'migrations' / 'django_branch'
filepath_sqla = Path(__file__).parent / 'storage' / 'psql_dos' / 'migrations' / 'sqlalchemy_branch'

# If the user requested the SQLite backend, automatically skip incompatible tests
if config.option.db_backend is TestDbBackend.SQLITE:
if config.option.markexpr != '':
# Don't overwrite markers that the user already provided via '-m ' cmdline argument
config.option.markexpr += ' and (not requires_psql)'
else:
config.option.markexpr = 'not requires_psql'

for item in items:
filepath_item = Path(item.fspath)

Expand All @@ -68,6 +84,30 @@ def pytest_collection_modifyitems(items):
item.add_marker('presto')


def db_backend_type(string):
"""Conversion function for the custom '--db-backend' pytest CLI option
:param string: String provided by the user via CLI
:returns: DbBackend enum corresponding to user input string
"""
try:
return TestDbBackend(string)
except ValueError:
msg = f"Invalid --db-backend option '{string}'\nMust be one of: {tuple(db.value for db in TestDbBackend)}"
raise pytest.UsageError(msg)


def pytest_addoption(parser):
parser.addoption(
'--db-backend',
action='store',
default=TestDbBackend.SQLITE,
required=False,
help=f'Database backend to be used for tests {tuple(db.value for db in TestDbBackend)}',
type=db_backend_type,
)


@pytest.fixture(scope='session')
def aiida_profile(pytestconfig, 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.
Expand All @@ -77,18 +117,22 @@ def aiida_profile(pytestconfig, aiida_config, aiida_profile_factory, config_psql
be run against the main storage backend, which is ``core.sqlite_dos``.
"""
marker_opts = pytestconfig.getoption('-m')
db_backend = pytestconfig.getoption('--db-backend')

# By default we use RabbitMQ broker and psql_dos storage
# We use RabbitMQ broker by default unless 'presto' marker is specified
broker = 'core.rabbitmq'
if 'not requires_rmq' in marker_opts or 'presto' in marker_opts:
broker = None

if 'not requires_psql' in marker_opts or 'presto' in marker_opts:
if db_backend is TestDbBackend.SQLITE:
storage = 'core.sqlite_dos'
config = config_sqlite_dos()
else:
elif db_backend is TestDbBackend.PSQL:
storage = 'core.psql_dos'
config = config_psql_dos()
else:
# This should be unreachable
raise ValueError(f'Invalid DB backend {db_backend}')

with aiida_profile_factory(
aiida_config, storage_backend=storage, storage_config=config, broker_backend=broker
Expand Down
13 changes: 4 additions & 9 deletions tests/storage/psql_dos/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,12 @@
from aiida.common.exceptions import MissingConfigurationError
from aiida.manage.configuration import get_config

STORAGE_BACKEND_ENTRY_POINT = None
try:
if test_profile := os.environ.get('AIIDA_TEST_PROFILE'):
STORAGE_BACKEND_ENTRY_POINT = get_config().get_profile(test_profile).storage_backend
# TODO: The else branch is wrong
else:
STORAGE_BACKEND_ENTRY_POINT = 'core.psql_dos'
except MissingConfigurationError:
# TODO: This is actually not true anymore!
# Case when ``pytest`` is invoked without existing config, in which case it will rely on the automatic test profile
# creation which currently always uses ``core.psql_dos`` for the storage backend
STORAGE_BACKEND_ENTRY_POINT = 'core.psql_dos'
except MissingConfigurationError as e:
raise ValueError(f"Could not parse configuration of AiiDA test profile '{test_profile}'") from e

if STORAGE_BACKEND_ENTRY_POINT != 'core.psql_dos':
if STORAGE_BACKEND_ENTRY_POINT is not None and STORAGE_BACKEND_ENTRY_POINT != 'core.psql_dos':
collect_ignore_glob = ['*']
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@
from aiida.storage.psql_dos.migrator import PsqlDosMigrator


def test_all_tests_marked_as_nightly(request):
"""Test that all tests in this folder are tagged with 'nightly' marker"""
own_markers = [marker.name for marker in request.node.own_markers]

assert len(own_markers) == 2
assert 'nightly' in own_markers
assert 'requires_psql' in own_markers


def test_migrate(perform_migrations: PsqlDosMigrator):
"""Test that the migrator can migrate from the base of the django branch, to the main head."""
perform_migrations.migrate_up('django@django_0001') # the base of the django branch
Expand Down
8 changes: 8 additions & 0 deletions tests/storage/psql_dos/test_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@
from aiida.orm import User


def test_all_tests_marked_with_requires_psql(request):
"""Test that all tests in this folder are marked with 'requires_psql'"""
own_markers = [marker.name for marker in request.node.own_markers]

assert len(own_markers) == 1
assert own_markers[0] == 'requires_psql'


@pytest.mark.usefixtures('aiida_profile_clean')
def test_default_user():
assert isinstance(get_manager().get_profile_storage().default_user, User)
Expand Down
64 changes: 64 additions & 0 deletions tests/test_markers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
"""Tests markers that have custom in conftest.py"""

import pytest


def test_presto_auto_mark(request):
"""Test that the presto marker is added automatically"""
own_markers = [marker.name for marker in request.node.own_markers]
assert len(own_markers) == 1
assert own_markers[0] == 'presto'


@pytest.mark.sphinx
def test_presto_mark_and_another_mark(request):
"""Test that presto marker is added even if there is an existing marker (except requires_rmq|psql)"""
own_markers = [marker.name for marker in request.node.own_markers]

assert len(own_markers) == 2
assert 'presto' in own_markers
assert 'sphinx' in own_markers


@pytest.mark.requires_rmq
def test_no_presto_mark_if_rmq(request):
"""Test that presto marker is NOT added if the test is mark with "requires_rmq"""
own_markers = [marker.name for marker in request.node.own_markers]

assert len(own_markers) == 1
assert own_markers[0] == 'requires_rmq'


@pytest.mark.requires_psql
def test_no_presto_mark_if_psql(request):
"""Test that presto marker is NOT added if the test is mark with "requires_psql"""
own_markers = [marker.name for marker in request.node.own_markers]

assert len(own_markers) == 1
assert own_markers[0] == 'requires_psql'


@pytest.mark.nightly
def test_no_presto_mark_if_nightly(request):
"""Test that presto marker is NOT added if the test is mark with "requires_psql"""
own_markers = [marker.name for marker in request.node.own_markers]

assert len(own_markers) == 1
assert own_markers[0] == 'nightly'


@pytest.mark.requires_psql
def test_requires_psql_with_sqlite_impossible(pytestconfig):
db_backend = pytestconfig.getoption('--db-backend')
if db_backend.value == 'sqlite':
pytest.fail('This test should not have been executed with SQLite backend!')


def test_daemon_client_fixture_automarked(request, daemon_client):
"""Test that any test using ``daemon_client`` fixture is
automatically tagged with 'requires_rmq' mark
"""
own_markers = [marker.name for marker in request.node.own_markers]

assert len(own_markers) == 1
assert own_markers[0] == 'requires_rmq'

0 comments on commit a863d1e

Please sign in to comment.