Skip to content

Commit

Permalink
Merge branch 'main' into Remove-the-use-of-deprecated-tranport-method…
Browse files Browse the repository at this point in the history
…s-in-tests
  • Loading branch information
unkcpz authored Nov 25, 2024
2 parents 3c1a702 + c93fb4f commit be69050
Show file tree
Hide file tree
Showing 23 changed files with 154 additions and 70 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/
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ notebook = [
]
pre-commit = [
'aiida-core[atomic_tools,rest,tests,tui]',
'mypy~=1.10.0',
'mypy~=1.13.0',
'packaging~=23.0',
'pre-commit~=3.5',
'sqlalchemy[mypy]~=2.0',
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/common/lang.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,4 @@ def __init__(self, getter: Callable[[SelfType], ReturnType]) -> None:
self.getter = getter

def __get__(self, instance: Any, owner: SelfType) -> ReturnType:
return self.getter(owner)
return self.getter(owner) # type: ignore[arg-type]
2 changes: 1 addition & 1 deletion src/aiida/common/pydantic.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Model(BaseModel):
field_info = Field(default, **kwargs)

for key, value in (('priority', priority), ('short_name', short_name), ('option_cls', option_cls)):
if value is not None:
if value is not None and field_info is not None:
field_info.metadata.append({key: value})

return field_info
2 changes: 1 addition & 1 deletion src/aiida/engine/processes/calcjobs/calcjob.py
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ def presubmit(self, folder: Folder) -> CalcInfo:

def encoder(obj):
if dataclasses.is_dataclass(obj):
return dataclasses.asdict(obj)
return dataclasses.asdict(obj) # type: ignore[arg-type]
raise TypeError(f' {obj!r} is not JSON serializable')

subfolder = folder.get_subfolder('.aiida', create=True)
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/engine/processes/workchains/restart.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@


def validate_handler_overrides(
process_class: 'BaseRestartWorkChain', handler_overrides: Optional[orm.Dict], ctx: 'PortNamespace'
process_class: type['BaseRestartWorkChain'], handler_overrides: Optional[orm.Dict], ctx: 'PortNamespace'
) -> Optional[str]:
"""Validator for the ``handler_overrides`` input port of the ``BaseRestartWorkChain``.
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/manage/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ def create_runner(self, with_persistence: bool = True, **kwargs: Any) -> 'Runner
if with_persistence and 'persister' not in settings:
settings['persister'] = self.get_persister()

return runners.Runner(**settings)
return runners.Runner(**settings) # type: ignore[arg-type]

def create_daemon_runner(self, loop: Optional['asyncio.AbstractEventLoop'] = None) -> 'Runner':
"""Create and return a new daemon runner.
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/orm/nodes/data/array/projection.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ def array_list_checker(array_list, array_name, orb_length):
raise exceptions.ValidationError('Tags must set a list of strings')
self.base.attributes.set('tags', tags)

def set_orbitals(self, **kwargs):
def set_orbitals(self, **kwargs): # type: ignore[override]
"""This method is inherited from OrbitalData, but is blocked here.
If used will raise a NotImplementedError
"""
Expand Down
4 changes: 2 additions & 2 deletions src/aiida/orm/nodes/data/list.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ def remove(self, value):
self.set_list(data)
return item

def pop(self, **kwargs):
def pop(self, **kwargs): # type: ignore[override]
"""Remove and return item at index (default last)."""
data = self.get_list()
item = data.pop(**kwargs)
if not self._using_list_reference():
self.set_list(data)
return item

def index(self, value):
def index(self, value): # type: ignore[override]
"""Return first index of value.."""
return self.get_list().index(value)

Expand Down
4 changes: 1 addition & 3 deletions src/aiida/orm/nodes/data/singlefile.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ def open(self, path: FilePath, mode: t.Literal['rb']) -> t.Iterator[t.BinaryIO]:

@t.overload
@contextlib.contextmanager
def open( # type: ignore[overload-overlap]
self, path: None = None, mode: t.Literal['r'] = ...
) -> t.Iterator[t.TextIO]: ...
def open(self, path: None = None, mode: t.Literal['r'] = ...) -> t.Iterator[t.TextIO]: ...

@t.overload
@contextlib.contextmanager
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/plugins/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def __init__(self):
def logger(self) -> Logger:
return self._logger

def get_version_info(self, plugin: str | type) -> dict[t.Any, dict[t.Any, t.Any]]:
def get_version_info(self, plugin: str | t.Any) -> dict[t.Any, dict[t.Any, t.Any]]:
"""Get the version information for a given plugin.
.. note::
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/tools/pytest_fixtures/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def test(submit_and_await):
from aiida.engine import ProcessState

def factory(
submittable: 'Process' | 'ProcessBuilder' | 'ProcessNode',
submittable: type[Process] | ProcessBuilder | ProcessNode | t.Any,
state: ProcessState = ProcessState.FINISHED,
timeout: int = 20,
**kwargs,
Expand Down
2 changes: 1 addition & 1 deletion tests/cmdline/groups/test_dynamic.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Model(BaseModel):
union_type: t.Union[int, float] = Field(title='Union type')
without_default: str = Field(title='Without default')
with_default: str = Field(title='With default', default='default')
with_default_factory: str = Field(title='With default factory', default_factory=lambda: True)
with_default_factory: str = Field(title='With default factory', default_factory=lambda: True) # type: ignore[assignment]


def test_list_options(entry_points):
Expand Down
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
Loading

0 comments on commit be69050

Please sign in to comment.