-
-
Notifications
You must be signed in to change notification settings - Fork 82
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
Docker container for Pytest #901
Conversation
@nucleogenic Minor thing, but it'd be helpful if you could make branches in this project instead of your fork. It makes it slightly more convenient to check out your work branch and try out the code. :) |
python/web/tests/conftest.py
Outdated
parser.addoption("--base_url", action="store", default="http://localhost:8080") | ||
parser.addoption("--httpserver_host", action="store", default="host.docker.internal") | ||
parser.addoption("--httpserver_listen_address", action="store", default="127.0.0.1") | ||
parser.addoption("--base_url", action="store", default="http://rascsi_web") |
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.
This change means that you have to override --base_url now when running in a default non-dockerized environment. Is there a particular reason for changing it for the purpose of the docker environment?
BTW, this is also a good reminder to write a README with some pointers / reminders when running the tests. :)
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.
Fair point! I can probably define an environment var inside the container and set some defaults based on container vs non container environments. Can you describe the environment you want to run the tests on? e.g. on dev machine against local web.py process, dev machine against remote Pi, executed directly on the Pi?
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.
My main use case is "executed directly on the Pi" but I can imagine that the other two scenarios are potentially useful for other people.
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.
Base URL now defaults to http://localhost:8080 as before, unless the env var DOCKER
is truthy.
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.
Seems good. Just one remark.
780c987
to
b3ec6e2
Compare
@rdmark Please could you re-test this in your Pi environment? You can now use the |
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.
Thank for the excellent improvements!
Container providing an alternative to a local Python venv for test execution.
Usage
Basic usage:
Running with specific parameters:
Accessing a shell:
The container can be rebuilt if needed (e.g. when
requirements-dev.txt
is updated) with: