Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
1538735: Pass BootstrapSeries to provider bootstrap #4899
Conversation
added some commits
Mar 27, 2016
kat-co
reviewed
Mar 28, 2016
| + instance.Instance, *instance.HardwareCharacteristics, []network.InterfaceInfo, error, | ||
| + ) { | ||
| + return &mockInstance{id: checkInstanceId}, &checkHardware, nil, nil | ||
| + } |
kat-co
Mar 28, 2016
Contributor
Can you please reformat this? Typically if you want a multi-line function, you align it as you would a struct.
kat-co
reviewed
Mar 28, 2016
| + return &mockInstance{id: checkInstanceId}, &checkHardware, nil, nil | ||
| + } | ||
| + var mocksConfig = minimalConfig(c) | ||
| + var getConfigCalled int |
kat-co
reviewed
Mar 28, 2016
| @@ -135,6 +135,55 @@ func (s *BootstrapSuite) TestCannotStartInstance(c *gc.C) { | ||
| c.Assert(err, gc.ErrorMatches, "cannot start bootstrap instance: meh, not started") | ||
| } | ||
| +func (s *BootstrapSuite) TestBootstrapSeries(c *gc.C) { | ||
| + s.PatchValue(&jujuversion.Current, coretesting.FakeVersionNumber) | ||
| + s.PatchValue(&series.HostSeries, func() string { return "precise" }) |
kat-co
Mar 28, 2016
Contributor
I'm going to be that person and say -- even though we don't have time to fix this right now -- patching values is a smell that our design is bad, and we should stop doing this in tests.
Carry on! ;)
kat-co
reviewed
Mar 28, 2016
| + }}) | ||
| + c.Assert(err, jc.ErrorIsNil) | ||
| + c.Assert(result.Arch, gc.Equals, "ppc64el") // based on hardware characteristics | ||
| + c.Assert(result.Series, gc.Equals, bootstrapSeries) |
kat-co
Mar 28, 2016
Contributor
Change the prior 2 asserts to .Check so that we get all errors in a failing test. .Assert needs to be kept on the err as a gate.
kat-co
reviewed
Mar 28, 2016
| + c.Assert(err, jc.ErrorIsNil) | ||
| + c.Assert(env.bootstrapCount, gc.Equals, 1) | ||
| + c.Assert(env.args.BootstrapSeries, gc.Equals, "trusty") | ||
| + c.Assert(env.args.AvailableTools.AllSeries(), jc.SameContents, []string{"trusty"}) |
kat-co
Mar 28, 2016
Contributor
Change the prior 3 asserts to .Check so that we get all errors in a failing test. .Assert needs to be kept on the err as a gate.
|
LGTM |
cherylj
commented
Mar 29, 2016
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
cherylj commentedMar 28, 2016
--bootstrap-series is used to filter tools before
calling the provider bootstrap method, but then the
bootstrap-series is not passed into the provider when
calling bootstrap.
The provider then used either the specified
default-series, or the default default-series to choose
tools. This led to a "no matching tools available"
error if the bootstrap series differed from the default
series used.
In this patch, I update the BootstrapParams to include
the bootstrap series and have the provider use that
to check for tools.
Unit tests reference unsupported "utopic" as support
for xenial and wily has yet to land for non-ubuntu
clients: juju/utils#201