replace smoke test config files with env #50

Merged
merged 1 commit into from Oct 6, 2016

Conversation

Projects
None yet
3 participants
Member

kwmonroe commented Oct 5, 2016

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.

replace smoke test config files with env
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.

petevg commented Oct 6, 2016

LGTM/+1

Owner

johnsca commented Oct 6, 2016

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?

Owner

johnsca commented Oct 6, 2016

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

@johnsca johnsca merged commit e1073ce into master Oct 6, 2016

@kwmonroe kwmonroe deleted the feature/smoke-test-env branch Oct 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment