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

Fixtures: Add aiida_computer_localhost and aiida_computer_ssh #5786

Merged
merged 2 commits into from
Nov 24, 2022

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Nov 21, 2022

These fixtures return an instance of Computer that represents the
localhost computer with core.local and core.ssh as the transport
type, respectively. By default, the computers are also configured. The
aiida_computer_ssh will even create an SSH key pair automatically for
the session such that the SSH transport can actually be opened.

The fixtures both call the more generic aiida_computer which returns a
factory that gives greater control on how the computer is created and
configured. Usuallly the two wrappers are all that is needed for the
majority of tests.

The fixtures are placed in pytest_fixtures instead of the conftest.py
of aiida-core, because aiida_computer and aiida_computer_localhost
should be available to plugins. The aiida_computer_ssh is probably
mostly useful for tests in aiida-core, but for clarity it is kept
together with aiida_computer_localhost.

These new fixtures provide the same functionality as aiida_localhost
but are more flexible. The aiida_localhost fixture is therefore
deprecated.

@sphuber sphuber requested a review from unkcpz November 21, 2022 21:19
@sphuber sphuber changed the title Fixtures: Add aiida_computer_localhost and aiida_computer_ssh Fixtures: Add aiida_computer_localhost and aiida_computer_ssh Nov 21, 2022
@sphuber
Copy link
Contributor Author

sphuber commented Nov 21, 2022

These fixtures are required for #5787 . Once this is approved and merged, I will go through the unit tests and clean them up, using the fixtures instead of test manually constructing computers.

@sphuber sphuber mentioned this pull request Nov 23, 2022
@ltalirz ltalirz self-requested a review November 23, 2022 19:46
Copy link
Member

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

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

thanks a lot @sphuber !

here a few comments from a quick review

aiida/manage/tests/pytest_fixtures.py Outdated Show resolved Hide resolved
aiida/manage/tests/pytest_fixtures.py Outdated Show resolved Hide resolved
aiida/manage/tests/pytest_fixtures.py Outdated Show resolved Hide resolved
aiida/manage/tests/pytest_fixtures.py Outdated Show resolved Hide resolved
aiida/orm/computers.py Show resolved Hide resolved
@sphuber sphuber force-pushed the feature/fixture-aiida-computer branch from 66fb394 to 286804a Compare November 23, 2022 20:28
@sphuber sphuber requested a review from ltalirz November 23, 2022 20:28
This is a convenience property that calls `is_user_configured` but
automatically passed the current default user.
These fixtures return an instance of `Computer` that represents the
localhost computer with `core.local` and `core.ssh` as the transport
type, respectively. By default, the computers are also configured. The
`aiida_computer_ssh` will even create an SSH key pair automatically for
the session such that the SSH transport can actually be opened.

The fixtures both call the more generic `aiida_computer` which returns a
factory that gives greater control on how the computer is created and
configured. Usuallly the two wrappers are all that is needed for the
majority of tests.

The fixtures are placed in `pytest_fixtures` instead of the `conftest.py`
of `aiida-core`, because `aiida_computer` and `aiida_computer_local`
should be available to plugins. The `aiida_computer_ssh` is probably
mostly useful for tests in `aiida-core`, but for clarity it is kept
together with `aiida_computer_local`.

The existing fixture `aiida_localhost` now simply uses the
`aiida_computer_local` fixture as the implementation.
@sphuber sphuber force-pushed the feature/fixture-aiida-computer branch from 286804a to 5d3c55a Compare November 23, 2022 20:59
Copy link
Member

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

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

thanks a lot @sphuber !

looks good to me

@sphuber sphuber merged commit fda91e2 into aiidateam:main Nov 24, 2022
@sphuber
Copy link
Contributor Author

sphuber commented Nov 24, 2022

Cheers @ltalirz 👍

@sphuber sphuber deleted the feature/fixture-aiida-computer branch November 24, 2022 08:49
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.

2 participants