-
-
Notifications
You must be signed in to change notification settings - Fork 415
TST: Move full conftest.py back to package level #3268
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
because otherwise pytest cannot find it if run on installed code but we still need root level for tox pytest header
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3268 +/- ##
==========================================
+ Coverage 69.08% 69.13% +0.05%
==========================================
Files 232 233 +1
Lines 19673 19705 +32
==========================================
+ Hits 13591 13623 +32
Misses 6082 6082 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Dang, do we really have to do this, it was so great to finally get rid of the duplicate? |
At the time of packaging? How? 👀 |
I don't know, but surely there should be a way besides some script hacking. Btw, if you run pytest with the installed version how do you pick up the |
I can't. That is why I added a
Maybe @Cadair can advise, given his expertise in sunpy land? |
Would copying the conftest over would work as the same workaround then? I really don't want to keep two of the same (well, similar and always diverging) file around just for the sake of the testing repo. |
AFAIK there is no way to tell pytest to select this or that conftest.py for a given run (but please correct me if i am wrong!) so copying conftest.py over will pollute the testing of other packages and that is undesirable. Another way to ask the question here is how should an end-user test their installed version of astroquery? |
The end user's convenience was already take out of the picture when the TestRunner was removed, so I feel not having a file duplicate outweighs a few tests erroring out out is with |
But let's see if Stuart has a cunning plan. |
Just FYI that without this, I could not run astropy integration testing (https://github.com/astropy/astropy-integration-testing/actions/runs/14934084322) for astroquery to test against astropy v7.1.0rc1. |
I still wonder if you could just copy over the conftest file the same as you do it for e.g. the sunpy pytest ini file. Alternative if we could move only the fixtures in there but don't duplicate anything about the headers, etc, that could also work, but I very strongly want to keep the file non-duplicating now that we finally cleaned them up. |
It might work but unfortunately I do not have time to work on it currently. Would be happy to review PR to the integration testing setup though. However, it is important that it is set up in a way that does not affect the other packages under test. AFAIK there is no way to set up package specific |
Move full conftest.py back to package level because otherwise pytest cannot find it if run on installed code but we still need root level for tox pytest header. This is a follow-up of #3215
Example failing log without this patch: https://github.com/astropy/astropy-integration-testing/actions/runs/13929757409/job/39007964709
With this patch: astropy/astropy-integration-testing#27 produces https://github.com/astropy/astropy-integration-testing/actions/runs/13954373539/job/39061703535?pr=27
See also astropy/ccdproc#884