Fix tox version issue on trusty #66
Conversation
| tox_prereqs: | ||
| ifneq ($(TOX_GT_1_8),true) | ||
| @echo "Pip installing tox" | ||
| sudo apt-get install -y python-pip && sudo pip install -U 'tox>=1.8' |
There was a problem hiding this comment.
Noooooooooooo do not pip install on my machine
There was a problem hiding this comment.
Run it in the charmbox like you're supposed to. ;)
There was a problem hiding this comment.
no, that is not a standard workflow, and until we get charm test to properly containerize runs we shouldn't just stomp on people's machines
There was a problem hiding this comment.
But in all seriousness, it's already installing via pip if tox isn't available. The reason it's failing in charmbox is because the apt version of tox for trusty is really old. Creating a venv to install tox so that it can then create another venv seems overly complicated and unnecessary.
There was a problem hiding this comment.
Very many tests assume sudo powers and apt-get or pip install things. Tests are containerized already using charmbox which is what this is assuming. Maybe we should just fix charmbox instead, but then we'd still already have the pip behavior that's in there currently.
There was a problem hiding this comment.
could we just, for now, install to --user? even though I'd prefer a virtualenv for tox. If I run the Makefile in this layer, it's only going to flake and charm proof, and unit tests. None of these should run scripts that do a sudo install, so your argument that because tests do this should don't follow through.
There was a problem hiding this comment.
I'm inclined to remove the tox dependency management from the layer entirely and fix the issue in charmbox, since that's our standard env for running tests and it is expected to manage most of the dependencies, of which the correct version of tox is one.
I'm going to close this PR and we'll open on against charmbox, and we can revisit removing the dep management from this layer entirely later.
No description provided.