Openstack regions default to cloud URL #6730

Merged
merged 1 commit into from Dec 20, 2016

Conversation

Projects
None yet
3 participants
Contributor

natefinch commented Dec 16, 2016

This change adds the concept of defaults to pollster Enter* functions.
It then uses those to implement defaults for schema queries, which are
then exercised with the Openstack provider's schema for endpoints.

QA:
run
juju add-cloud
choose openstack, give it a valid openstack URI, like canonistack (https://keystone.canonistack.canonical.com:443/v2.0/)
Then add a region and just hit enter for the url.
View $JUJU_DATE/clouds.yaml and ensure the region endpoint gets set the same as the cloud URI.

@@ -184,6 +184,78 @@ func (PollsterSuite) TestEnterEmpty(c *gc.C) {
c.Assert(squash(w.String()), jc.Contains, "Enter your name: Enter your name: ")
}
+func (PollsterSuite) TestEnterVerify(c *gc.C) {
@kat-co

kat-co Dec 16, 2016

Contributor

I've been complaining in recent reviews that test names should not just state what they're testing, but the expected outcome.

@natefinch

natefinch Dec 16, 2016

Contributor

I don't think hugely long test names have a lot of benefit, and that's what would be required to make a test name fully descriptive. Just as with any function, you give it the most descriptive name you can without making it into a 300 character menace. I usually try to give an idea of what the test covers in the name, and the specifics go in the code and/or comments. It seems like a reasonable convention that Test<FunctionName> would do a generic "normal usage" style test of FunctionName, which is what this does. Just underneath it is TestEnterVerifyBad which indicates that we'll test what happens when verification fails.

Side note - I noticed I hadn't added tests for EnterVerify, which is why I added them here as a driveby.

cmd/juju/interact/pollster_test.go
+ s, ok := v.(string)
+ c.Check(ok, jc.IsTrue)
+ c.Check(s, gc.Equals, "foo")
+ c.Assert(w.String(), jc.Contains, "Enter region [not foo]:")
@kat-co

kat-co Dec 16, 2016

Contributor

Anything that should not preclude tests from continuing -- even if they are currently the last line in the function -- should utilize c.Check.

@natefinch

natefinch Dec 16, 2016

Contributor

Fixed.

Openstack regions default to cloud URL
This change adds the concept of defaults to pollster Enter* functions.
It then uses those to implement defaults for schema queries, which are
then exercised with the Openstack provider's schema for endpoints.
Contributor

natefinch commented Dec 20, 2016

$$merge$$

Contributor

jujubot commented Dec 20, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit a967717 into juju:develop Dec 20, 2016

1 check passed

github-check-merge-juju Built PR, ran unit tests, and tested LXD deploy. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment