testutil: new package for test clean up

testutil: new package for test clean up

Killed all the repetition in suite_test files, and improved the
implementation of the test HTTP server.

A few test bugs were also fixed on the way.

https://code.launchpad.net/~niemeyer/goamz/testing-cleanup/+merge/145878

(do not edit description out of merge proposal)

[niemeyer, 2013-01-31 15:01:04]

Please take a look.

[rog, 2013-01-31 15:42:43]

LGTM except for the query about the default security group test.

nice to see all that code factored out.

https://codereview.appspot.com/7232065/diff/1/ec2/ec2_test.go
File ec2/ec2_test.go (right):

https://codereview.appspot.com/7232065/diff/1/ec2/ec2_test.go#newcode24
ec2/ec2_test.go:24: testServer.Start()
that makes much more sense, given we're using the global below.

https://codereview.appspot.com/7232065/diff/1/ec2/ec2t_test.go
File ec2/ec2t_test.go (right):

https://codereview.appspot.com/7232065/diff/1/ec2/ec2t_test.go#newcode447
ec2/ec2t_test.go:447: //resp, err :=
s.ec2.SecurityGroups([]ec2.SecurityGroup{{Name: "default"}}, nil)
why commented out? this was testing specific ec2test functionality.

have you tried these tests live? i seem to remember that
i needed to add the "default" group because it was
being returned for some of the filtering requests.

https://codereview.appspot.com/7232065/diff/1/ec2/ec2test/server.go
File ec2/ec2test/server.go (right):

https://codereview.appspot.com/7232065/diff/1/ec2/ec2test/server.go#newcode420
ec2/ec2test/server.go:420: //	ImageId: 			accept anything, we can verify later
align?
(and below)

https://codereview.appspot.com/7232065/diff/1/ec2/sign.go
File ec2/sign.go (right):

https://codereview.appspot.com/7232065/diff/1/ec2/sign.go#newcode23
ec2/sign.go:23: // be in natural order of the keys. This is distinct from the
s/in natural/in the natural/
?

https://codereview.appspot.com/7232065/diff/1/exp/mturk/suite_test.go
File exp/mturk/suite_test.go (left):

https://codereview.appspot.com/7232065/diff/1/exp/mturk/suite_test.go#oldcode1
exp/mturk/suite_test.go:1: package mturk_test
great to see all this duplication go away.

https://codereview.appspot.com/7232065/diff/1/s3/s3.go
File s3/s3.go (right):

https://codereview.appspot.com/7232065/diff/1/s3/s3.go#newcode231
s3/s3.go:231: // into different groupings of keys, similar to how folders would
work.
s/would// ?

[niemeyer, 2013-01-31 15:58:33]

https://codereview.appspot.com/7232065/diff/1/ec2/ec2t_test.go
File ec2/ec2t_test.go (right):

https://codereview.appspot.com/7232065/diff/1/ec2/ec2t_test.go#newcode447
ec2/ec2t_test.go:447: //resp, err :=
s.ec2.SecurityGroups([]ec2.SecurityGroup{{Name: "default"}}, nil)
On 2013/01/31 15:42:43, rog wrote:
> why commented out? this was testing specific ec2test functionality.

Sorry, it shouldn't be commented out. It should be removed. There's nothing
special about the default group, and the test is broken because it trusts the
system to only have a single group named default, which is not a given.

> have you tried these tests live? i seem to remember that
> i needed to add the "default" group because it was
> being returned for some of the filtering requests.

Yeah, it was testing it live that forced me to fix it.

[rog, 2013-01-31 17:01:29]

On 31 January 2013 15:58,  <n13m3y3r@gmail.com> wrote:
>
> https://codereview.appspot.com/7232065/diff/1/ec2/ec2t_test.go
> File ec2/ec2t_test.go (right):
>
> https://codereview.appspot.com/7232065/diff/1/ec2/ec2t_test.go#newcode447
> ec2/ec2t_test.go:447: //resp, err :=
> s.ec2.SecurityGroups([]ec2.SecurityGroup{{Name: "default"}}, nil)
> On 2013/01/31 15:42:43, rog wrote:
>>
>> why commented out? this was testing specific ec2test functionality.
>
>
> Sorry, it shouldn't be commented out. It should be removed. There's
> nothing special about the default group, and the test is broken because
> it trusts the system to only have a single group named default, which is
> not a given.

The default is created by automatically, can't be removed, and has
default permissions
set up for tcp and udp (which means that it gets selected by some of
the filters)

if you try to create another group called "default", you get a
"The security group 'default' is reserved" error.

That seems fairly special to me - I *think* we're always guaranteed
a single group named default... but maybe that's only true of us-east.

How did the test fail for you? I'd definitely like to understand more about
the problem before changing those tests.

[niemeyer, 2013-01-31 17:03:47]

On 2013/01/31 17:01:29, rog wrote:
> The default is created by automatically, can't be removed, and has
> default permissions set up for tcp and udp (which means that it
> gets selected by some of the filters)

None of that seems relevant for filtering.
 
> if you try to create another group called "default", you get a
> "The security group 'default' is reserved" error.

I have two groups named "default" in my account, and that's breaking the test
case. One of them comes from VPC. Amazon created it.

> That seems fairly special to me - I *think* we're always guaranteed
> a single group named default... but maybe that's only true of us-east.

I'm not lying, promise.

[niemeyer, 2013-01-31 17:07:03]

Please take a look.

https://codereview.appspot.com/7232065/diff/1/ec2/ec2test/server.go
File ec2/ec2test/server.go (right):

https://codereview.appspot.com/7232065/diff/1/ec2/ec2test/server.go#newcode420
ec2/ec2test/server.go:420: //	ImageId: 			accept anything, we can verify later
On 2013/01/31 15:42:43, rog wrote:
> align?
> (and below)

Done.

https://codereview.appspot.com/7232065/diff/1/ec2/sign.go
File ec2/sign.go (right):

https://codereview.appspot.com/7232065/diff/1/ec2/sign.go#newcode23
ec2/sign.go:23: // be in natural order of the keys. This is distinct from the
On 2013/01/31 15:42:43, rog wrote:
> s/in natural/in the natural/
> ?

Done.

https://codereview.appspot.com/7232065/diff/1/s3/s3.go
File s3/s3.go (right):

https://codereview.appspot.com/7232065/diff/1/s3/s3.go#newcode231
s3/s3.go:231: // into different groupings of keys, similar to how folders would
work.
On 2013/01/31 15:42:43, rog wrote:
> s/would// ?

That's how they would work if S3 used the concept of folders. It doesn't, so it
doesn't work quite that way.

[niemeyer, 2013-01-31 17:12:04]

*** Submitted:

testutil: new package for test clean up

Killed all the repetition in suite_test files, and improved the
implementation of the test HTTP server.

A few test bugs were also fixed on the way.

R=rog
CC=
https://codereview.appspot.com/7232065