lsmo-epfl / aiida-lsmo

AiiDA workflows for the LSMO laboratory at EPFL
Other
9 stars 13 forks source link

update testing to use upstream repository #106

Closed mpougin closed 1 year ago

mpougin commented 1 year ago

update aiida-testing to track directly from https://github.com/aiidateam/aiida-testing as discussed with @ltalirz and @chrisjsewell . Can you please double-check if the syntax is correct?

mpougin commented 1 year ago

are errors related to dependency conflicts?

ltalirz commented 1 year ago

The error seems to arise from this line https://github.com/lsmo-epfl/aiida-lsmo/blob/47f080615fef31b144339a5dd2af64f3d6bb6ac6/conftest.py#L13 which is now no longer needed, since aiida-testing registers the fixtures automatically.

@chrisjsewell I am still not entirely convinced that saving the user from writing this one line is really worth the additional "magic" - and, as you can see, this can lead to confusion. Should we revert it?

mpougin commented 1 year ago

The error seems to arise from this line

https://github.com/lsmo-epfl/aiida-lsmo/blob/47f080615fef31b144339a5dd2af64f3d6bb6ac6/conftest.py#L13

which is now no longer needed, since aiida-testing registers the fixtures automatically.

Thanks for the clarification @ltalirz . From my point of you, as you said this is only confusing, and requires additional modification of the conftest.py file

chrisjsewell commented 1 year ago

the additional "magic"

Its not magic, its exactly the way pytest says to make plugins: https://docs.pytest.org/en/7.1.x/how-to/writing_plugins.html#making-your-plugin-installable-by-others

it only leads to confusion, when it was done the wrong way intially

mpougin commented 1 year ago

it only leads to confusion, when it was done the wrong way intially

Maybe it's just lack of experience but I wouldn't be able to tell from the error message/without knowing about the changes you made what the problem is

ltalirz commented 1 year ago

the additional "magic"

Its not magic, its exactly the way pytest says to make plugins: https://docs.pytest.org/en/7.1.x/how-to/writing_plugins.html#making-your-plugin-installable-by-others

it only leads to confusion, when it was done the wrong way intially

I see your point.

At the same time, I understand that with this approach there is no longer a way to turn off fixtures that ship with autouse=True, right?

If I want to run the test suite on any package I have installed in my Python environment (AiiDA-related or not), the aiida_profile fixture will now be executed before every test.

chrisjsewell commented 1 year ago

the aiida_profile fixture will now be executed before every test.

yep exactly, the aiida_profile fixture should not be using autouse=True 😉 I'm not really sure why it was added