Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(6321)

Issue 7098074: Use nova service doubles in nova tests (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 3 months ago by wallyworld
Modified:
11 years, 3 months ago
Reviewers:
mp+144089
Visibility:
Public.

Description

Use nova service doubles in nova tests This branch plugs the nova service doubles into the nova live tests and makes everything work. So now, in the nova package, "go test -gocheck.v" runs the tests against the doubles, and "go test -gocheck.v -live" uses an Openstack instance. Also, the rate limit retry test is now only run locally against the service doubles, where a special request is used to trigger a rate limit service response. This replaces the previous test which hammered a real Openstack instance to try and induce a rate limit response. A few things needed to be done, and as a result of implementation changes to the service doubles, changes were also needed for the service double unit tests. Viewing highlights: 1. Default security group A real Openstack instance always has a default security group. The nova double and associated unit tests were modified accordingly. 2. Out of the box flavours The real Openstack instance used for testing has flavours defined out of the box. The live tests assume this behaviour so the test double needed to be modifed to match. 3. Response changes Some changes were required to certain response data eg creating a security group rule 4. ** This issue will need a follow up change ** The security group rule response contains a SecurityGroupRef struct: type SecurityGroupRule struct { FromPort *int `json:"from_port"` // Can be nil IPProtocol *string `json:"ip_protocol"` // Can be nil ToPort *int `json:"to_port"` // Can be nil ParentGroupId int `json:"parent_group_id"` IPRange map[string]string `json:"ip_range"` // Can be empty Id int Group SecurityGroupRef } The was defined as a pointer ie Group *SecurityGroupRef. However, nova double tests which do a DeepEquals to compare security group rules fail because the pointer deferencing isn't done. So to make stuff work, I've used a struct value instead and use the zeroSecurityGroupRef pattern as used on the ec2 side. This isn't desirable and should be looked at. But if we are prepared to go with it for now, it can be fixed in a followup branch. BTW, nothing uses this attribute value so the change should not break juju core. https://code.launchpad.net/~wallyworld/goose/service-double-cleanup/+merge/144089 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 4

Patch Set 2 : Use nova service doubles in nova tests #

Patch Set 3 : Use nova service doubles in nova tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+333 lines, -204 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M nova/live_test.go View 3 chunks +1 line, -34 lines 0 comments Download
M nova/local_test.go View 4 chunks +58 lines, -17 lines 0 comments Download
M nova/nova.go View 3 chunks +2 lines, -6 lines 0 comments Download
M nova/nova_test.go View 1 chunk +1 line, -2 lines 0 comments Download
M testservices/novaservice/service.go View 1 2 7 chunks +64 lines, -18 lines 0 comments Download
M testservices/novaservice/service_http.go View 1 30 chunks +64 lines, -6 lines 0 comments Download
M testservices/novaservice/service_http_test.go View 22 chunks +64 lines, -66 lines 0 comments Download
M testservices/novaservice/service_test.go View 10 chunks +62 lines, -54 lines 0 comments Download
M testservices/novaservice/setup_test.go View 1 chunk +14 lines, -0 lines 0 comments Download
M testservices/swiftservice/service_http.go View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 5
wallyworld
Please take a look.
11 years, 3 months ago (2013-01-21 11:41:11 UTC) #1
dimitern
LGTM
11 years, 3 months ago (2013-01-21 11:53:10 UTC) #2
jameinel
Overall LGTM. I'm a bit concerned about not having a live test for Retry, because ...
11 years, 3 months ago (2013-01-21 12:24:58 UTC) #3
wallyworld
https://codereview.appspot.com/7098074/diff/1/nova/local_test.go File nova/local_test.go (right): https://codereview.appspot.com/7098074/diff/1/nova/local_test.go#newcode92 nova/local_test.go:92: // occurs and the request ultimately succeeds. On 2013/01/21 ...
11 years, 3 months ago (2013-01-21 13:07:45 UTC) #4
wallyworld
11 years, 3 months ago (2013-01-21 23:59:09 UTC) #5
*** Submitted:

Use nova service doubles in nova tests

This branch plugs the nova service doubles into the nova live tests and makes
everything work.
So now, in the nova package, "go test -gocheck.v" runs the tests against the
doubles, and
"go test -gocheck.v -live" uses an Openstack instance.

Also, the rate limit retry test is now only run locally against the service
doubles, where a special
request is used to trigger a rate limit service response. This replaces the
previous test which
hammered a real Openstack instance to try and induce a rate limit response.

A few things needed to be done, and as a result of implementation changes to the
service doubles,
changes were also needed for the service double unit tests.

 Viewing highlights:

1. Default security group

A real Openstack instance always has a default security group. The nova double
and associated unit tests were modified
accordingly.

2. Out of the box flavours

The real Openstack instance used for testing has flavours defined out of the
box. The live tests assume this behaviour
so the test double needed to be modifed to match.

3. Response changes

Some changes were required to certain response data eg creating a security group
rule

4. ** This issue will need a follow up change **

The security group rule response contains a SecurityGroupRef struct:

type SecurityGroupRule struct {
	FromPort      *int              `json:"from_port"`   // Can be nil
	IPProtocol    *string           `json:"ip_protocol"` // Can be nil
	ToPort        *int              `json:"to_port"`     // Can be nil
	ParentGroupId int               `json:"parent_group_id"`
	IPRange       map[string]string `json:"ip_range"` // Can be empty
	Id            int
	Group         SecurityGroupRef
}

The was defined as a pointer ie  Group *SecurityGroupRef.
However, nova double tests which do a DeepEquals to compare security group rules
fail because the pointer deferencing isn't done. So to make stuff work, I've
used a struct value instead and use the zeroSecurityGroupRef pattern as used
on the ec2 side. This isn't desirable and should be looked at. But if we are
prepared to go with it for now, it can be fixed in a followup branch. BTW,
nothing
uses this attribute value so the change should not break juju core.

R=dimitern, jameinel
CC=
https://codereview.appspot.com/7098074
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b