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

TST: Move full conftest.py back to package level #3268

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pllim
Copy link
Member

@pllim pllim commented Mar 19, 2025

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

because otherwise pytest cannot find it if run on installed code
but we still need root level for tox pytest header
Copy link

codecov bot commented Mar 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.13%. Comparing base (c1772fd) to head (0167cf2).
Report is 4 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pllim pllim added the testing label Mar 19, 2025
@pllim pllim marked this pull request as ready for review March 19, 2025 18:57
@pllim pllim requested a review from bsipocz March 19, 2025 18:57
@bsipocz
Copy link
Member

bsipocz commented Mar 19, 2025

Dang, do we really have to do this, it was so great to finally get rid of the duplicate?
What about adding a symlink/copy at time of packaging?

@pllim
Copy link
Member Author

pllim commented Mar 20, 2025

What about adding a symlink/copy at time of packaging?

At the time of packaging? How? 👀

@bsipocz
Copy link
Member

bsipocz commented Mar 20, 2025

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 [tool:pytest] section from the config files either? A lot is set up there.

@pllim
Copy link
Member Author

pllim commented Mar 20, 2025

how do you pick up the [tool:pytest] section

I can't. That is why I added a astroquery_pytest.ini in the astropy-integration-testing PR linked above, following sunpy's example.

surely there should be a way

Maybe @Cadair can advise, given his expertise in sunpy land?

@bsipocz
Copy link
Member

bsipocz commented Mar 20, 2025

I can't. That is why I added a astroquery_pytest.ini in the astropy-integration-testing PR linked above, following sunpy's example.

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.

@pllim
Copy link
Member Author

pllim commented Mar 20, 2025

Would copying the conftest over would work as the same workaround then?

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?

@bsipocz
Copy link
Member

bsipocz commented Mar 20, 2025

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 pytest --pyargs astroquery. I mean these numbers are close enough: 1679 passed, 731 skipped, 2 xfailed, 44 warnings, 4 errors

@bsipocz
Copy link
Member

bsipocz commented Mar 20, 2025

But let's see if Stuart has a cunning plan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants