Comment 5 for bug 1459345

Revision history for this message
Cory Johns (johnsca) wrote :

Francesco,

The tests.yaml change will make it play nicer with bootstrap, but there are still a couple of issues with how the Makefile tries to manage the Juju environment:

* Since the bootstrap is unconditional, if it is run with an already-bootstrapped environment, it will fail due to attempting to re-bootstrap the environment. An easy work-around for this would be to add " || true" to the end of the bootstrap command. That will still show bootstrap error messages in the test output, which could be confusing, but it will at least continue on to run the tests.

* Because you're assuming that you bootstrapped the environment, you're also unconditionally destroying it. However, with the change above, you might be destroying an environment that was created before `make test` was run. It also prevents inspecting the environment in the case of failure, to figure out what went wrong with the charm. I'd lean toward removing the destroy step.

* Probably the biggest issue, to me, is that the environment is hard-coded to local. This will not work in the automated test runner (which hasn't picked up this charm yet for some reason), because it attempts to test against multiple environments such as AWS, and MAAS. It also makes reviewing more difficult, because I like to use charmbox to get isolation so that charms whose tests install apt packages don't pollute my system, and the local provider doesn't work within charmbox. I'd like to see the hard-coded env removed and just have it use the currently active environment.