-
Notifications
You must be signed in to change notification settings - Fork 228
Add more pytest fixture tests #6882
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6882 +/- ##
==========================================
+ Coverage 79.20% 79.31% +0.12%
==========================================
Files 566 566
Lines 43461 43461
==========================================
+ Hits 34417 34468 +51
+ Misses 9044 8993 -51 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@agoscinski just heads up that I think this should be included in the 2.7.0 release otherwise we risk breaking people's CIs (as it broke aiidalab-widgets-base). I can work on this over the weekend to drive it home. |
…ests/test_pytest_fixtures.py
6db7e2d
to
c34f72e
Compare
@danielhollas what do you think about restructuring the tests into |
@agoscinski I agree that we should not ship a test file as part of the package. However, splitting the test suite at the top like that seems like a very heavy-handed solution to this. Ultimately we just need to move one test file out of the |
9329183
to
e5adde7
Compare
a692148
to
dab0af6
Compare
For now I've moved the test file inside the |
from aiida.orm import Computer | ||
from aiida.transports import AsyncTransport, BlockingTransport | ||
from aiida.transports.plugins import _AsyncSSH, _OpenSSH | ||
|
||
|
||
def test_profile_config(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is already covered in tests/tools/pytest_fixtures/test_configuration.py
so I removed it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this file was basically just moved from tests/manage/tests/test_pytest_fixtures.py
to tests/tools/pytest_fixtures/test_orm.py
. And since the tests here work for both old and new fixtured, I've also copied the to the .github/test_legacy_pytest_fixtures.py
.
@agoscinski I've merged in main so this is ready for review. The diff on Github might be a bit confusing, please see my comments. |
pytest_plugins = ['aiida.manage.tests.pytest_fixtures'] | ||
|
||
|
||
def test_aiida_config(tmp_path_factory): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests were copied here from tests/manage/tests/test_pytest_fixtures.py
, they are valid for both the legacy and new fixtures.
I see you've picked up I'm looking into the |
Follow-up to #6904