Mocking out new API? #70
Comments
|
Thanks for the feedback @jellevandenhooff. With the change to making the v2 SDK repo public I had to recreate PR #50 as PR #72. We plan on keeping the ability to mock out API requests. This is an important way of using the SDK that many users, and the SDK's on unit tests, rely on. How the API's are mocked is a little bit different now, and I think there is room for improvement. One way mocking experience could be improved is for the SDK to provide a utility that does just what you mentioned, configures the API Request to return a specific value without actually making a network request. This utility would also help ensure that any custom handlers installed by the application are exercised. The V1 SDK's mocking ignores custom handlers. I'd like to hear more about the impact on your application if the |
jasdel
added
the
response-requested
|
Thanks again for you feedback @jellevandenhooff lets use #50 and #72 to track this. I've copied your feedback into #50. I'll close this as a duplicate of #50, and we can track the discussion there. |
jasdel
removed
the
response-requested
|
@jasdel I think you missed the main portion of this issue, which is that since the new service APIs return request objects, which are in turn used to fetch results, testing behavior requires more than just mocking the service. It also requires a way to mock the request object. Does that make sense? |
|
Thanks for clarifying the request @jellevandenhooff. Correct, the SDK returning a typed request value does significantly change how the API would be mocked out. For a typed request I think the biggest challenge will be to mock out the behavior of the request's If the SDK's typed requests were refactored to have a |
|
Yes, I think a |
|
Just started using the V2 SDK and went to mock some ECS/Cloudwatch stuff and ran into this. It all appears mockable at first glance, but the concrete E.g, A mock for ECSAPI |
|
@jamisonhyatt Could you give some code sample on how you mocked twice to unit-test your Request implemenltation which has the Send method? Asking because I am struggling with the mock as well. |
|
I've had conversations around this in the #aws and #general channel on the Gopher Slack, specifically around the At Netflix I'm in the process of rewriting the internal tool we use for managing support tickets across our various accounts. Currently it's a NodeJS service, that we're looking to replace the backend of in Go. ( 🙏) While starting to bootstrap the application I started to evaluate v1 versus v2, and came to the realization that the support API, and maybe more of them, feels more idiomatic in the v1 version of the SDK. To me it seems that the direction we're taking in v2 doesn't feel like an improvement (in terms of the method sets). To provide a concrete example: the v1 API structure made it stupid-easy to define an interface describing the subset of the With the new API layout, it's become nigh impossible to do it for integration tests without writing your own wrapper around the AWS Go SDK. This is because now the method set for the I'd be happy to sit down with y'all, even over Amazon Chime 😉, to talk through these sorts of things if you feel it would be helpful. 🤜🤛 |
|
Thanks for your feedback. Our plan is briefly touched on in #80, but additional design is needed. Our idea is that each API request type would have a It is odd to have a The v1 SDK's clients several methods per API to perform different functionality. e.g. Having Some of the other ideas we looked at: Generic interfaces: func (*Support) Send(req APIRequest) (APIResponse, error)
//...
descCasesReq := support.DescribeCasesRequest{
Input: DescribeCasesInput{
// ... params ...
}
}
v, err := svc.Send(descCasesReq)
descCasesResp := v.(DescribeCasesResponse)Per API Send (aka similar to v1 SDK) But requires separate Pagination function, or Paginate off of DescribeCaseResponse but this would confuse users on if the first page has already been retrieved. func (*Support) DescribeCases(*DescribeCasesRequest) (DescribeCasesResponse, error)Per API Send with "request" options. Same as above issue with Pagination. func (*Support) DescribeCases(*DescribeCasesInput, ...RequestOption) (DescribeCasesResponse, error) |
|
The approach to slim down the API where We've gotten consistent negative feedback about the The The These are the the primary motivators to slim to a single API method which constructs the API's request. The user then has the ability to use the request to paginate, create presigned URL, or modify request prior to sending. A goal of the V2 SDK is to remove complexity and simplify interfaces. Using a API Request constructor without functional options allows this. The Removing functional options, and shifting to explicit configuration focused on struct parameter simplifies the discoverability. This pattern was used in the v2 SDK's redesign of config, and external config loading. Getting rid of the mess of |
|
Hi all involved. Apparently, I managed to stub the request enough in order to achieve a good coverage. Look at this create test: https://github.com/go-furnace/go-furnace/blob/c829118784b556dadcb174032806a0d8629003d3/furnace-aws/commands/create_test.go#L44 I have send extracted to this function: func (cf *CFClient) createStack(stackInputParams *cloudformation.CreateStackInput) *cloudformation.CreateStackOutput {
log.Println("Creating Stack with name: ", keyName(*stackInputParams.StackName))
req := cf.Client.CreateStackRequest(stackInputParams)
resp, err := req.Send()
handle.Error(err)
return resp
}With this stubbing of the Request object it still goes all the way to Send but it comes back with the data that is mocked out in aws.Request in the test. Namely with a Stack with id DummyID or the stack name "TestStack". I can also test errors by passing in the error I want from Request like this: func (fc *fakeCreateCFClient) CreateStackRequest(input *cloudformation.CreateStackInput) cloudformation.CreateStackRequest {
return cloudformation.CreateStackRequest{
Request: &aws.Request{
Data: &cloudformation.CreateStackOutput{
StackId: aws.String("DummyID"),
},
Error: fc.err,
},
Input: input,
}
}Here if fc.err is not nil Send will return that error like this:
This is a screenshot of a debug process in the middle of Send returning my error that I added to Error.
"failed to create stack" is my error message that I've set up to be returned by it. There you have it. You can stub the request object and the metadata too if you want something specific to be returned. If you look at the rest of the tests you'd find that I did the same thing for describe stack and delete stack and others. To be clear, this is not going out anywhere or calling out to anything. I tried it while disabling wifi on my laptop or on travis which is not configured with AWS access and it does work. Hopefully this helps. Pinging people for visibility: @jasdel @theckman @jamisonhyatt @jellevandenhooff. Good luck! |
|
I'm not sure you've acheived all that much as setting the error in the request makes it so that |
|
I am doing that. I simply mentioned the error case in detail. But there is a positive scenario as well. In case error is nil and Data property is defined then it wi return that. For example I'm returning a stack with the id DummyId and describe stack returns my TestStack. |
|
Here is a positive scenario where error was nil and the returned object is a concrete type I want to handle. func (cf *CFClient) createStack(stackInputParams *cloudformation.CreateStackInput) *cloudformation.CreateStackOutput {
log.Println("Creating Stack with name: ", keyName(*stackInputParams.StackName))
req := cf.Client.CreateStackRequest(stackInputParams)
resp, err := req.Send()
handle.Error(err)
return resp
}Here, |
|
Why those this work you might ask? Because in the stubbed request, handlers are empty.
And thus this part does not fail here: func (l *HandlerList) Run(r *Request) {
for i, h := range l.list {
h.Fn(r)
item := HandlerListRunItem{
Index: i, Handler: h, Request: r,
}
if l.AfterEachFn != nil && !l.AfterEachFn(item) {
return
}
}
}Effectively returning whatever is in func (fc *fakeCreateCFClient) CreateStackRequest(input *cloudformation.CreateStackInput) cloudformation.CreateStackRequest {
return cloudformation.CreateStackRequest{
Request: &aws.Request{
Data: &cloudformation.CreateStackOutput{
StackId: aws.String("DummyID"),
},
Error: fc.err,
},
Input: input,
}
}Tadaam. |
|
If you want even more granular mocking you could potentially replace the |
|
Maybe this will help someone looking for a quick workaround. I had a bunch of different requests that I eventually called Simplified code example: package dms
import (
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/databasemigrationservice"
)
type sender interface {
send(request interface{}) (interface{}, error)
}
type IDMS interface {
GetReplicationSubnetGroupOutput(identifier string) (*databasemigrationservice.ReplicationSubnetGroupOutput, error)
}
type DMS struct {
c aws.Config
d databasemigrationserviceiface.DatabaseMigrationServiceAPI
s sender
}
func New(c aws.Config) *DMS {
return &DMS{
d: dms.New(c),
s: awsSender{},
}
}
func (a *AWS) GetReplicationSubnetGroupOutput(identifier string) (*databasemigrationservice.ReplicationSubnetGroupOutput, error) {
describeRequest := a.d.DescribeReplicationSubnetGroupsRequest(
&databasemigrationservice.DescribeReplicationSubnetGroupsInput{
Filters: []databasemigrationservice.Filter{
{
Name: aws.String("replication-subnet-group-id"),
Values: []string{
identifier,
},
},
},
})
return a.s.send(describeRequest)
}
type awsSender struct{}
func (s awsSender) send(request interface{}) (interface{}, error) {
switch r := request.(type) {
case databasemigrationservice.DescribeReplicationSubnetGroupsRequest:
return r.Send()
// More case statements.
}
return nil, fmt.Errorf("unexpected request of type: %T", request)
}Silly test example mocking/stubbing things: package dms
import (
"testing"
"github.com/aws/aws-sdk-go-v2/service/databasemigrationservice"
"github.com/aws/aws-sdk-go-v2/service/databasemigrationservice/databasemigrationserviceiface"
)
func TestSample(t *testing.T) {
expectedOut := databasemigrationservice.ReplicationSubnetGroupOutput{},
d := &DMS{
d: &mockDMSAPI{
expectedReplicationSubnetGroupRequest: databasemigrationservice.DescribeReplicationSubnetGroupsRequest{},
},
s: &mockSender{
expectedOut: expectedOut,
expectedErr: nil,
},
}
out, err := d.GetReplicationSubnetGroupOutput("foo")
if out != expectedOut {
t.Error("output is not expected")
}
if err != nil {
t.Error("error is non-nil")
}
}
type mockDMSAPI struct {
databasemigrationserviceiface.DatabaseMigrationServiceAPI
expectedReplicationSubnetGroupRequest databasemigrationservice.DescribeReplicationSubnetGroupsRequest
}
func (m *mockDMSAPI) DescribeReplicationSubnetGroupsRequest(i *dms.DescribeReplicationSubnetGroupsInput) databasemigrationservice.DescribeReplicationSubnetGroupsRequest {
return m.expectedReplicationSubnetGroupRequest
}
type mockSender struct{
expectedOut interface{}
expectedErr error
}
func (m *mockSender) send(request interface{}) (interface{}, error) {
return m.expectedOut, m.expectedErr
} |
|
Thanks for creating the example for this issue. We're investigating how the V2 SDK's design could be updated to improve you're ability to mock out API operations for testing. Here is another example of mocking out the Amazon S3 ListObjects operation with a mock that has testing values, or error handling. https://play.golang.org/p/AJnxGn-gWzA package main
import (
"context"
"fmt"
"log"
"net/http"
"reflect"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/s3"
)
func main() {
// Create a mock ListObjects client.
mockClient := &MockListObjects{
Output: &s3.ListObjectsOutput{
Contents: []s3.Object{
{Key: aws.String("somePrefix/someKeyName")},
{Key: aws.String("somePrefix/otherKey")},
},
},
}
// Using the mock pass it into the ListObjects function. This will exercise the behavior
// of the ListObjects function, and the return can be compared against the expected value.
objects, err := ListObjects(context.Background(), mockClient, "myBucket", "somePrefix")
if err != nil {
log.Fatalf("expect no error, got %v", err)
}
if !reflect.DeepEqual(mockClient.Output.Contents, objects) {
log.Fatalf("expect mocked contents to match")
}
fmt.Println("Objects:", objects)
}
// ListObjectser provides the interface for a ListObjectsRequest operation.
type ListObjectser interface {
ListObjectsRequest(*s3.ListObjectsInput) *s3.ListObjectsRequest
}
// ListObjects will list the objects in a bucket which have a given prefix. Returning the object slice, or error.
func ListObjects(ctx context.Context, client ListObjectser, bucket, keyPrefix string) ([]s3.Object, error) {
// Make a request for the list of objects in the bucket.
resp, err := client.ListObjectsRequest(&s3.ListObjectsInput{
Bucket: &bucket,
Prefix: &keyPrefix,
}).Send(ctx)
if err != nil {
return nil, err
}
return resp.Contents, nil
}
// MockListObjects provides mocking for a ListObjects API operation call.
type MockListObjects struct {
Output *s3.ListObjectsOutput
Err error
}
func (m *MockListObjects) ListObjectsRequest(*s3.ListObjectsInput) *s3.ListObjectsRequest {
mockReq := &aws.Request{
HTTPRequest: &http.Request{},
HTTPResponse: &http.Response{},
}
mockReq.Handlers.Complete.PushBack(func(r *aws.Request) {
if m.Output != nil {
r.Data = m.Output
} else if m.Err != nil {
r.Error = m.Err
}
})
return &s3.ListObjectsRequest{
Request: mockReq,
}
} |
|
@jasdel can you please elaborate on why My solution for Kinesis was something like, but it feels a bit wonky :D type mockKinesisClient struct {
kinesisiface.ClientAPI
err error
output *kinesis.PutRecordOutput
}
// https://github.com/aws/aws-sdk-go-v2/issues/70
func (m *mockKinesisClient) PutRecordRequest(*kinesis.PutRecordInput) kinesis.PutRecordRequest {
mockReq := &aws.Request{
HTTPRequest: &http.Request{},
HTTPResponse: &http.Response{},
Error: m.err,
Data: m.output,
}
return kinesis.PutRecordRequest{
Request: mockReq,
}
} |
|
just wanted to pile on and indicate that this issue was deemed too large for our team to adopt -v2 in a new greenfield project. We typically mock like the example below. With the new 2 phase system we would have to mock 2 things 1) The Request builder to return our mocked Sender and 2) the Sender to return our desired returns... // in something.go
type Something {
dynamodbiface.ClientAPI // Embed an interface
}
// in something_test.go
type MockSomething {
dynamodbiface.ClientAPI // Embed an interface, but leave nil so it panics if we forget to mock
DoDescribeTable func (input *DescribeTableInput) (*DescribeTableOutput, error)
}
// DescribeTable executes the mocked function or panics if not mocked
func (ms *MockSomething) DescribeTable(input *DescribeTableInput) (*DescribeTableOutput, error) {
return ms.DoDescribeTable(input)
} |
|
Hope this will save someones time. After playing around with a modified version of @mrsufgi solution, this seems to work and doesn't make any external calls. type mockGetItem struct {
dynamodbiface.ClientAPI
Response dynamodb.GetItemOutput
}
func (d mockGetItem) GetItemRequest(*dynamodb.GetItemInput) dynamodb.GetItemRequest {
mockReq := &aws.Request{
HTTPRequest: &http.Request{},
HTTPResponse: &http.Response{},
Error: nil,
Data: &d.Response,
Retryer: aws.NoOpRetryer{}, // needed, otherwise will crash
}
return dynamodb.GetItemRequest{
Request: mockReq,
}
}
func TestHandler(t *testing.T) {
t.Run("Success", func(t *testing.T) {
m := mockGetItem{
Response: dynamodb.GetItemOutput{
Item: map[string]dynamodb.AttributeValue{
"id": {
S: aws.String("N"),
},
},
},
}
d := deps{
dynamodbClient: m,
}
// ... //
})
} |
|
Also want to chime in to note that this is just as painful (if not more) for code that's written to use paginators -- that is, testing logic written in the form: req := ec2Client.DescribeInstanceStatusRequest(&ec2.DescribeInstanceStatusInput{})
p := ec2.NewDescribeInstanceStatusPaginator(req)
for p.Next(ctx) {
page := p.CurrentPage()
// handle page
}With the v2 API, mocking for code that uses pagination requires something like: ec2Client.On("DescribeInstanceStatusRequest", mock.Anything).Return(ec2.DescribeInstanceStatusRequest{
Copy: func(in *ec2.DescribeInstanceStatusInput) ec2.DescribeInstanceStatusRequest {
return ec2.DescribeInstanceStatusRequest{
Request: &aws.Request{
Operation: &aws.Operation{},
Retryer: aws.NoOpRetryer{},
HTTPRequest: &http.Request{},
HTTPResponse: &http.Response{},
Error: nil,
Data: &ec2.DescribeInstanceStatusOutput{
// set desired data here
InstanceStatuses: statuses,
},
},
}
},
})Note that this is slightly different from the previous code examples since the This works, but leans heavily on the concrete types and their specific internal implementation details. In contrast, since the v1 API used interfaces for pagination as well, mocking pagination was easier: mockClient.On("DescribeInstanceStatusPagesWithContext", mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
visitorFn := args.Get(2).(func(*ec2.DescribeInstanceStatusOutput, bool) bool)
visitorFn(&ec2.DescribeInstanceStatusOutput{
// set desired data here
InstanceStatuses: statuses,
}, false)
}).Return(nil) |
|
Let's consolidate this discussion into #786 as we have made significant changes to the client APIs, and will be further simplifying the interfaces for pagination in a later release that will differ from versions of the V2 SDK released prior to |
A major change in the v2 API is the requirement to use Request methods (#33). We currently use the
ifacepackages to test our code using the v1 API with the strategy described inaws-sdk-go-v2/service/acm/acmiface/interface.go
Line 42 in ac26486
mockgen.Could you please ensure that this mocking remains possible? Perhaps the Request API can return a Request object that doesn't go out to the web and instead returns a mock Response? Other alternative mock strategies would be equally welcome.
To tack on, the suggestion in #50 to split up the
ifacepackages would likely make mocking for us more painful as we would have to generate more interfaces.The text was updated successfully, but these errors were encountered: