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

Fix a few tests #96

Merged
merged 1 commit into from
Sep 22, 2024
Merged

Fix a few tests #96

merged 1 commit into from
Sep 22, 2024

Conversation

rolandmas
Copy link

Hi,
Roland Mas here, as the maintainer of the Debian packages for Freesas. I started updating the packages to v2024.09.0 today, and I found a few errors in the testsuite. I fixed them as best I could, so I submit them to you in the hope that the patch can be included in your repository so as to reduce the need for a Debian-specific patch in the future.
Feel free to comment or request changes :-)

Gbp-Pq: Name 0001-Fix-a-few-tests.patch
@rolandmas
Copy link
Author

I added a second commit that tweaked the imports in the testsuite, to allow running it on an installed system as well as during the build, for the autopkgtests CI system in Debian.

@kif
Copy link
Member

kif commented Sep 19, 2024

Hi Rolland,
Thanks for your work on the packaging. It is appreciated. While I totally agree with your first commit, the usage of relative imports should be possible and is always recommended ...
Indeed, it is possible to run the test on the installed version with
./run_tests.py --installed

which prints out where the library is loaded:

WARNING:run_tests:Test freesas 2024.9.1-dev0 from /home/jerome/.venv/py311/lib/python3.11/site-packages/freesas
TEST_LOW_MEM=False TEST_RANDOM=False

while the same script ruun without the option tests a local installation.

I don't know precisely how debian tests are launched but this should be fixed in the debian scripts.

Cheers,

Jerome

@rolandmas
Copy link
Author

The autopkgtests (as well as the build-time testsuite) are run with a simple "pythonX -m unittest discover -v" (with X iterating across supported Python versions). This leaves the logic in unittest's hands, greatly simplifiyng the running of the testsuite, and avoids having to ship run_tests.py, bootstrap.py and the pyproject.toml file in the temporary dir from where the tests are run, avoids having two different ways to run tests, and also avoids having to ship the tests themselves in the installed package.

So I'm curious, what does run_tests.py do in addition to that? Is it something that's relevant to the packaging, or mainly to the upstream authors? I can of course revert that part of the patch if it's not relevant to you, but I'm unsure about how best to run the tests in the packaging/distro environments.

@kif
Copy link
Member

kif commented Sep 20, 2024

run_test.py is not mandatory for running the tests... you can for example use:
python3 -c "import freesas.test, sys;sys.exit(freesas.test.run_tests())"

@rolandmas
Copy link
Author

Okay, I removed the second commit from the PR and will only keep it as Debian-specific patch.

@kif kif self-requested a review September 22, 2024 13:13
Copy link
Member

@kif kif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@kif kif merged commit 830da6e into silx-kit:main Sep 22, 2024
24 checks passed
@kif
Copy link
Member

kif commented Sep 22, 2024

Thanks Roland

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.

2 participants