Code review comment for lp:~niemeyer/goamz/testing-cleanup

Revision history for this message
Roger Peppe (rogpeppe) wrote :

On 31 January 2013 15:58, <email address hidden> 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.

« Back to merge proposal