-
Notifications
You must be signed in to change notification settings - Fork 12
AEP: Testing toolchain for plugin packages #6
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
base: master
Are you sure you want to change the base?
Conversation
instead of copying plugin folder to the docker container, mount it.
8840c0f to
d14eb5c
Compare
yakutovicha
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my suggestions below.
| ``` | ||
| docker run -v .:/home/aiida/plugin aiida-cp2k-docker-stack -t test-container | ||
| docker exec --user aiida test-container pip install --user -e .[pre-commit,testing] | ||
| docker exec --user aiida test-container reentry scan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestions:
- Put the whole content of
.travis.ymlhere. - Put the code above into
before_installsection of the.travis.ymlfile - Add an aiida version checker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea. Let's do this one you have a working setup with aiida-cp2k
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| docker run -v .:/home/aiida/plugin aiida-cp2k-docker-stack -t test-container | ||
| docker exec --user aiida test-container pip install --user -e .[pre-commit,testing] | ||
| docker exec --user aiida test-container reentry scan | ||
| docker exec --user aiida test-container py.test --cov aiida_cp2k --cov-append . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget that before running pytest one should wait a little bit before the container starts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess most of the time here is spent in creating the AiiDA profile, right?
When running tests via pytest, this is actually not needed.
Perhaps it would be a good idea to use an image for the tests that does not enforce the creation of a new profile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the current setup, I don't think it is really possible. But I agree that we should modify the base docker image to allow this. Maybe by providing some variable CREATE_PROFILE_AT_STARTUP
thanks @yakutovicha Co-Authored-By: Aliaksandr Yakutovich <yakutovicha@gmail.com>
submittedREADME.md