Mocking out new API? about aws-sdk-go-v2 HOT 23 CLOSED

I've had conversations around this in the #aws and #general channel on the Gopher Slack, specifically around the service/support subpackage, and was asked chime in here. 👋

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 Support client you wanted to use, so that you could use dependency injection to provide it to any code that needed it. You declared an interface that included the methods you wanted to consume from the client, and had your constructor functions take this instead of the concrete type. This method of dependency injection made testing really easy, because you could mock my own implementation to satisfy the interface and control responses.

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 Support client includes DescribeCasesRequest, which returns a concrete type and not an interface value. This means that if I want to mock that method, and avoid the call to Amazon, I cannot do it without defining my own interface and set of methods to do the conversion and return the interface instead. I then need to refer to my own wrapper interface / method sets instead of the AWS Go SDK, which isn't the most ideal thing. I end up having to do this is because I cannot mock a method on a concrete type, like is possible in other languages that support metaprogramming.

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. 🤜🤛

from aws-sdk-go-v2.

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 Send method. I think the refactor work of the SDK's request lifecycle and redesign in #80. I think this is still an outstanding problem that needs to be solved in the SDK before it can be made general availability.

If the SDK's typed requests were refactored to have a Sender interface member, a mocked service client would set the Sender member to a value which satisfies the Sender interface, and implements the desired mocked behavior.

from aws-sdk-go-v2.

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 Send() kinda makes it fall down. I actually went searching for a Sender interface, so that seems like a logical solution. The unfortunate side effect of the new SDK structure is that it will require 2 mocks for every logical "action" exposed by the SDK

E.g, A mock for ECSAPI DescribeClustersRequest func to return a concrete type DescribeClustersRequest, and another mock to implement the Sender interface to return DescribeClustersOutput

from aws-sdk-go-v2.

@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?

from aws-sdk-go-v2.

@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.

from aws-sdk-go-v2.

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,
		}

		// ... //
	})
}

from aws-sdk-go-v2.

The approach to slim down the API where WithContext would be dropped and ctx would be a required parameter. There would still need to be at least two to three methods per API. Three for pagination. DescribeServices, DescribeServicesRequest, DescribeServicesPages.

We've gotten consistent negative feedback about the request.Option functional parameter (aka func(*request.Request)). Mostly focused on discoverability, and in the v2 SDK the request.Request type most likely won't exist at all, or will be drastically different.

The DescribeServicesPages method would be refactored to return a DescribeServicesPaginator instead of using the callback function. The callback function was another significant point of confusion for users.

The DescribeServiceRequest is needed by users to have direct access to the API's request shape before it is sent. Request functional options help some with this, but they have been a point of confusion for users. There are use cases such as Presigned requests which must be performed with the API's input parameters and client state, but never sent. Its possible a func (*Support) DescribeServicesPresigned(in *DescribeServicesInput, dur time.Duration) (url string, signedHeaders http.Header, err error) API method could be created for APIs which might be useful with presigned request. Though, this is largely unknown from a codegen lens which APIs are useful for presigned other than S3 and GET method APIs.

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 Sender on the constructed API request value allows for modification of how the request is Sent when the API request's Send, Paginate, Presign methods are called. We still have a major refactor of the v2 SDK's request lifecycle which will certainly impact this design, but I think the Sender field on the API's request will facilitate this. Its possible that Sender as a single method interface might be limiting.

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 Session vs aws.Config was a major improvement.

from aws-sdk-go-v2.

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)
}

from aws-sdk-go-v2.

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 ifaces are split into different APIs. Could you go into more details the impact this would have?

from aws-sdk-go-v2.

Maybe this will help someone looking for a quick workaround. I had a bunch of different requests that I eventually called Send on and I ended up using a wrapper that switches on the type to cast and return/run Send.

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
}

from aws-sdk-go-v2.

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.

from aws-sdk-go-v2.

Yes, I think a Sender interface would do the trick.

from aws-sdk-go-v2.

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 Sender member. This member's type probably is an interface for a type that knows how to send and receive the API request. Our Mocking plan was to have users provide a mock version of the Sender for the API request being made. The concrete API request type would call a method on the Sender to process the API request. In the case of mocking, the mock Sender would probably return the mocked response output.

It is odd to have a Send method on the API's Request type instead of making the call directly on the client. We were motivated to have Send on the request to prevent generic interfaces for request and response, while also having the flexibility of creating an API request. An API request type allows the SDK to easily expose request configuration, and improve pagination discoverability.

The v1 SDK's clients several methods per API to perform different functionality. e.g. ListObjectsRequest, ListObjectsWithContext, ListObjectsPages, ListObjectsPagesWithContext`. Reducing this list down to a single method per API, without loosing functionality is our goal.

Having Sender as a member on the DescribeCasesRequest as either an interface or function would allow a mock to replace the behavior of individual API call.

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)

from aws-sdk-go-v2.

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!

from aws-sdk-go-v2.

I'm not sure you've acheived all that much as setting the error in the request makes it so that Send() returns early, as seen here. If I understand the problem correctly is that there is still no easy method of simply returning a mocked response in a "success" case. I wonder if httptest.NewServer would be the best way to acheive this at the moment as one could configure the client to make requests to it instead of the actual Amazon APIs for testing.

from aws-sdk-go-v2.

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.

from aws-sdk-go-v2.

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, resp will be my returned stubbed data object. And I'm not mocking out the http client. Look at my tests in create_test the one I linked. :)

from aws-sdk-go-v2.

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 Data which is my stubbed stack response here:

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.

from aws-sdk-go-v2.

If you want even more granular mocking you could potentially replace the aws.Handlers list with something you have. Though I don't see interfaces for the handlers. 🤔. Also I don't think that would be needed. Usually you want to deal with the response's Data.

from aws-sdk-go-v2.

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,
	}
}

from aws-sdk-go-v2.

@jasdel can you please elaborate on why mockReq.Handlers.Complete.PushBack is a better approach?

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,
	}
}

from aws-sdk-go-v2.

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 Copy function is what needs to be set, since that's what New*Paginator functions typically use (and the paginator functions themselves also can't be mocked since they're not on an interface).

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)

from aws-sdk-go-v2.

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 v0.25.0

from aws-sdk-go-v2.