Skip to content

Fix tests-develop test - #510

Closed
mdondrup wants to merge 1 commit into
mainfrom
508-ci-tests-of-development-scripts-dont-work
Closed

Fix tests-develop test#510
mdondrup wants to merge 1 commit into
mainfrom
508-ci-tests-of-development-scripts-dont-work

Conversation

@mdondrup

@mdondrup mdondrup commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

This PR fixes regression #508 introduced in #506 by loading fairmd.lipids module during a test stage which the test wasn't designed for.

When imported the fairmd.lipids package does some heavy lifting, whose init.py raises RuntimeError when FMDL_DATA_PATH points to a non-existent folder. Develop tests shouldn't trigger that heavy package initialization, so loading the schema from the filesystem keeps them isolated.

Now loading the schema directly again. If you think, loading the package initialization is necessary, you need to provide it with the proper environment in the test.

(Btw, did I mention that unconditional loading of data from an environment variable was a bad idea? init should be lazy loading and side-effect free )


📚 Documentation preview 📚: https://databank--510.org.readthedocs.build/

We imported the fairmd.lipids package, whose __init__.py raises RuntimeError when FMDL_DATA_PATH points to a non-existent folder. Develop tests shouldn't trigger that heavy package initialization, so loading the schema from the filesystem keeps them isolated.

Now loading the schema directly. If you think, loading the package initialization is necessary, you need to provide it with the proper environment in the test.

(Btw, did I mention that unconditional loading of data from an environment variblble was a bad idea?))
@mdondrup mdondrup linked an issue Jun 14, 2026 that may be closed by this pull request
@mdondrup
mdondrup requested a review from comcon1 June 14, 2026 11:41
@mdondrup

mdondrup commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator Author
tox -e tests-develop
[...]

tests/develop/test_autocomplete_metadata.py ..                           [100%]

====================== 2 passed, 205 deselected in 6.07s =======================
  tests-develop: OK (12.66=setup[5.56]+cmd[0.09,7.00] seconds)
  congratulations :) (13.68 seconds)

@comcon1

comcon1 commented Jun 14, 2026

Copy link
Copy Markdown
Member

I will not approve it. If we use something from the package, we must import it from the package. The error should be fixed in the manifests.

@mdondrup

Copy link
Copy Markdown
Collaborator Author

I will not approve it. If we use something from the package, we must import it from the package. The error should be fixed in the manifests.

Ok just fix it the way you want.

@mdondrup mdondrup changed the title Causee of the error: Fix tests-develop test Jun 14, 2026
@mdondrup
mdondrup marked this pull request as draft June 14, 2026 16:51
@mdondrup mdondrup closed this Jun 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI tests of development scripts don't work

2 participants