-
Notifications
You must be signed in to change notification settings - Fork 130
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
[joss-review] tests #314
Comments
In fact this is a requirement for the JOSS review to have automated or manual test instructions:
|
this regards the devel3 branch where almost all the commands of udocker are tested from the perspective of a user, the second script test udocker run with several images and all execution modes except singularity (S1) |
The have unit tests, integration tests, security tests with bandit and code style checking. We use Jenkins pipelines for our SQA. |
@jorge-lip great! Are there instructions anywhere for how a developer or interested user like myself could run the tests? |
just did a commit with information about how to run tests, it's in section 9 of the installation manual |
This is great! I had a few suggestions here -> #333 |
PR approved and merged |
I'm good with thew new additions - @mviereck when you've had a chance to take a look let me know, and if you are also good we can close the issue. |
I've tried to run the test as described:
I got a lot of errors at the
I could fix this installing
|
@mviereck I tried the tests and they did work - do you have python-dev headers available? (I'm seeing a reference to Python.h). Although that's a bit of a strange dependency, probably one of the libraries needs it to build. |
@vsoch I've installed
I could fix this with installing The step
shows several 'ok' and some errors. Because I don't know what is checked exactly (syntax and code style?), I can't assess its meaning. The results of the further tests also tell me nothing, but seem to be ok:
Currently I run |
Hmm, it probably would make sense to list the system dependencies somewhere or provide a container. I must have lucked out that I had them. |
"Nose’s tagline is “nose extends unittest to make testing easier”. "Pylint is a Python static code analysis tool which looks for programming errors, helps enforcing a coding standard, sniffs for code smells and offers simple refactoring suggestions." "Bandit is a tool designed to find common security issues in Python code. To do this Bandit processes each file, builds an AST from it, and runs appropriate plugins against the AST nodes. Once Bandit has finished scanning all the files it generates a report." I hope this clarifies what tests are being preformed |
|
Yes, the internet bandwith was the bottleneck here.
It makes sense to collect all errors and show them altogether when the script has finished. I found this one scrolling through the output.
Thank you! Yes that helps.
A similar/additional thought: |
Many thanks for the suggestions we will address those in a future release. |
I've run
|
The two test scripts have been improved to display a summary of the failed tests upon exit, when failures occur. The exit status also reflects the errors. |
Great!
|
Yes it is alerting of some TCP connect timeouts which means that your network must have some problems. |
Great! docker and podman just drop hours of download on such failures. So far I am good with the tests and we can close here. If it helps you, I would run Remaining possible improvements, but I won't insist:
|
Thanks, Yes please, send us those error messages. |
|
those are error messages from the code itself when the unit test passes though the branch containing that message and not a real error from the test. |
solved in latest commit to master, only affected the unit tests |
This would be a good addition. I tried to set
ok, I understand.
Set in the way you think it's best. I did not want to enforce a change here, just tried to understand. Thank you for all this, I am good with the tests. |
as per the last comment we will close this issue |
It looks like tests aren't included in the release, and at least for HPC, installers would want to be able to run tests before moving forward. Have you considered adding them?
The text was updated successfully, but these errors were encountered: