search

juju-solutions / layer-apache-bigtop-base

Apache License 2.0
1 stars 5 forks source link

replace smoke test config files with env #50

Closed kwmonroe closed 8 years ago

kwmonroe commented 8 years ago

We currently allow charms to specify a smoke_test_configs layer opt, which is supposed to be files that we source prior to running the smoke test. This is hard to handle because we don't know if that file will be an ini, a dotfile, or some other thing that we have to grok and source.

Seems smarter for the smoke test itself to construct a dict of all the environment stuff it needs and pass that into the smoke test runner.

This does that. I do not believe anyone is using the config file madness, and it certainly didn't do anything in the smoke test runner function, so I feel like now's the time to get on the right path.

pengale commented 8 years ago

LGTM/+1

johnsca commented 8 years ago

So, the downside of this over having a layer option which points to file(s) is that it means we have to copypasta the whole smoke-test action file just to provide params. The file is pretty small, so that's not a huge burden, but I feel like there is plenty of boilerplate code in there that could reasonably be refactored into Bigtop.run_smoke_tests so that the default action is nothing more than:

from charms.layer.apache_bigtop_base import Bigtop
Bigtop().run_smoke_tests()

Alternatively, could we make the layer option be an entry-point style reference to a callback that generates and returns the config?

johnsca commented 8 years ago

Opened #51 to address my comment above at a future date. +1 for merging this now.