Added a minimal job to check that tests pass when existing config/cache folders are present by astrofrog · Pull Request #19740 · astropy/astropy

astrofrog · 2026-05-14T09:30:42Z

This fails in CI and does pick up the issues discussed offline - I think we should include this in the CI for now at least during all the config/cache refactoring.

By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

…ache and .astropy/config folders are present

github-actions · 2026-05-14T09:30:53Z

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

Do the proposed changes actually accomplish desired goals?
Do the proposed changes follow the Astropy coding guidelines?
Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

neutrinoceros · 2026-05-14T10:18:51Z

+          - ubuntu-latest
+          - macos-latest


I don't expect any difference to show up within UNIX-based systems, but I guess it doesn't hurt to check 2 of them, especially if the job is only temporary.

neutrinoceros · 2026-05-14T10:21:42Z

+      - name: Install astropy with minimal test dependencies
+        run: python -m pip install -e ".[test]"
+      - name: Create empty .astropy cache and config directories
+        run: python -c "from pathlib import Path; [(Path.home() / '.astropy' / d).mkdir(parents=True, exist_ok=True) for d in ('cache', 'config')]"


is this written in Python for portability (windows...) ?

Yes exactly

cool. Maybe add a comment if you feel like it. Otherwise, LGTM

neutrinoceros · 2026-05-14T10:22:54Z

+        with:
+          python-version: "3.12"
+      - name: Install astropy with minimal test dependencies
+        run: python -m pip install -e ".[test]"


editable mode shouldn't be needed ?

Suggested change

run: python -m pip install -e ".[test]"

run: python -m pip install ".[test]"

neutrinoceros · 2026-05-14T10:25:01Z

+      - name: Create empty .astropy cache and config directories
+        run: python -c "from pathlib import Path; [(Path.home() / '.astropy' / d).mkdir(parents=True, exist_ok=True) for d in ('cache', 'config')]"
+      - name: Run tests
+        run: pytest --pyargs astropy


instead of using editable mode, set this env var so the current directory isn't included in sys.path

Suggested change

run: pytest --pyargs astropy

run: pytest --pyargs astropy

env:

PYTHONSAFEPATH: 1

neutrinoceros · 2026-05-14T10:28:24Z

I only took a quick glance at the failures and I think some (maybe most) of them should simply be fixed by enforcing isolation with @pytest.mark.usefixtures("ignore_config_paths_global_state")

neutrinoceros · 2026-05-14T10:49:08Z

(nothing blocking, just nits)

… job

The legacy backward-compat fallback in _DirectoryFinder ran unconditionally, so a leftover ~/.astropy/cache or ~/.astropy/config directory would override an explicit set_temp_cache/set_temp_config target or an ASTROPY_CACHE_DIR/ASTROPY_CONFIG_DIR environment variable. Move the fallback into the default resolution branch so it only applies when no override or environment variable is in effect.

With a non-editable install, running pytest --pyargs astropy from the repo root makes pytest collect the source astropy/conftest.py under a module name that resolves to the installed copy, raising ImportPathMismatchError. Run from an empty directory instead, mirroring the tox changedir convention, so only the installed package is used.

astrofrog · 2026-05-14T13:51:15Z

@neutrinoceros - this actually implements a proper fix for the issue rather than using the fixtures, as I think there was a real bug. Do you want to review again?

pllim · 2026-05-14T13:54:16Z

Yay and thank you so much for the fix and tests!

However, I dunno how I feel about a whole new matrix (3 extra jobs) just for this part. Is there no way we can piggy back this config into existing test matrix? We have so many jobs as it is.

astrofrog · 2026-05-14T14:00:24Z

@pllim - I'll double check

Fold the empty .astropy/cache and .astropy/config check into the existing direct-pytest job in the weekly cron, which avoids adding a separate multi-platform CI job.

astrofrog · 2026-05-14T14:57:55Z

@pllim - added to weekly cron, I'll let you decide whether it's worth running Extra CI here

pllim · 2026-05-14T17:34:41Z

devinfra still failed on rerun, so I am running it off main for comparison: https://github.com/astropy/astropy/actions/runs/25875214616

pllim

Thanks! devinfra failure is unrelated (#19742). Merging.

… existing config/cache folders are present

… existing config/cache folders are present (#19741) Co-authored-by: Thomas Robitaille <[email protected]>

Added a minimal job to check that tests pass when existing .astropy/c…

28ce888

…ache and .astropy/config folders are present

github-actions Bot added the testing label May 14, 2026

astrofrog changed the title ~~EXP: Added a minimal job to check that tests pass when existing config/cache folders are present~~ Added a minimal job to check that tests pass when existing config/cache folders are present May 14, 2026

Try different OSes

4b629a2

astrofrog marked this pull request as ready for review May 14, 2026 10:12

astrofrog requested a review from neutrinoceros as a code owner May 14, 2026 10:12

neutrinoceros reviewed May 14, 2026

View reviewed changes

neutrinoceros approved these changes May 14, 2026

View reviewed changes

astrofrog added 3 commits May 14, 2026 11:38

Install astropy non-editable and set PYTHONSAFEPATH in empty-cache CI…

ac3c32b

… job

pllim added this to the v8.0.0 milestone May 14, 2026

Added comment

c39e7c5

pllim added no-changelog-entry-needed Affects-dev PRs and issues that do not impact an existing Astropy release backport-v8.0.x on-merge: backport to v8.0.x labels May 14, 2026

pllim mentioned this pull request May 14, 2026

ENH/BUG: avoid test pollution from data tests by making APIs resilient against missing cache directories #19650

Merged

1 task

Check empty config/cache dirs in weekly cron instead of a dedicated job

7b610eb

Fold the empty .astropy/cache and .astropy/config check into the existing direct-pytest job in the weekly cron, which avoids adding a separate multi-platform CI job.

pllim added the Extra CI Run cron CI as part of PR label May 14, 2026

This comment was marked as outdated.

Sign in to view

pllim approved these changes May 14, 2026

View reviewed changes

pllim merged commit cf7e12d into astropy:main May 14, 2026
59 of 61 checks passed

meeseeksmachine mentioned this pull request May 14, 2026

Backport PR #19740 on branch v8.0.x (Added a minimal job to check that tests pass when existing config/cache folders are present) #19741

Merged

pllim pushed a commit that referenced this pull request May 14, 2026

Backport PR #19740: Added a minimal job to check that tests pass when…

6d6338a

… existing config/cache folders are present

pllim added a commit that referenced this pull request May 14, 2026

Backport PR #19740: Added a minimal job to check that tests pass when…

30986ba

… existing config/cache folders are present (#19741) Co-authored-by: Thomas Robitaille <[email protected]>

neutrinoceros mentioned this pull request May 20, 2026

RFC: spread complexity more evenly across functions #19776

Closed

1 task

	run: python -m pip install -e ".[test]"
	run: python -m pip install ".[test]"

Conversation

astrofrog commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 14, 2026

Uh oh!

neutrinoceros May 14, 2026

Choose a reason for hiding this comment

Uh oh!

neutrinoceros May 14, 2026

Choose a reason for hiding this comment

Uh oh!

astrofrog May 14, 2026

Choose a reason for hiding this comment

Uh oh!

neutrinoceros May 14, 2026

Choose a reason for hiding this comment

Uh oh!

astrofrog May 14, 2026

Choose a reason for hiding this comment

Uh oh!

neutrinoceros May 14, 2026

Choose a reason for hiding this comment

Uh oh!

neutrinoceros May 14, 2026

Choose a reason for hiding this comment

Uh oh!

neutrinoceros commented May 14, 2026

Uh oh!

neutrinoceros commented May 14, 2026

Uh oh!

astrofrog commented May 14, 2026

Uh oh!

pllim commented May 14, 2026

Uh oh!

astrofrog commented May 14, 2026

Uh oh!

astrofrog commented May 14, 2026

Uh oh!

This comment was marked as outdated.

pllim commented May 14, 2026

Uh oh!

pllim left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

astrofrog commented May 14, 2026 •

edited

Loading

pllim left a comment •

edited

Loading