Discussion: How should you unit test code which uses the aws-sdk-go #88
Comments
|
@geneticgenesis the work in the // create mocked service object
db := dynamodb.New(nil)
db.Handlers.Clear()
db.Handlers.Send.PushBack(func(r *aws.Request) {
// always return a specific response to every request
data := r.Data.(*dynamodb.ListTablesOutput)
data.TableNames = []string{"hello", "world"}
})
// later in test
r, _ := db.ListTables(nil)
fmt.Printf("%+v\n", r)Will print: The exact API is still in flux, but mocking should be feasible all with the direct Go data structures so you maintain the same type safety in your tests as well. |
|
So this works very well in practise, but I can't see any way to combine this with expectations... IE, I can't actually assert that the Any suggestions on achieving this? Also, would you consider this bad encapsulation since it means that anyone with access to the dynamodb client can affect its behaviour? |
|
Hmm was assuming there would be a handler type that would allow you to intercept the request/input object and return properly typed response, but looking closer it doesnt appear that way so far...maybe #79 is still of use? |
|
@geneticgenesis you can assert the operation and params by checking I wouldn't call this poor encapsulation, the extensibility is a feature specifically allowing you to perform these use cases. |
|
@sclasen as mentioned above, you can write a handler to intercept the input object already-- any of the handlers can do this. The code listed above shows a concrete example for returning a mocked response, and you could do the same for input by setting |
|
Aha so Params is an interface{} and the operation gives you the info needed to assert it to the right type? In the example above, had there been a non nil ListTablesInput we could have asserted Params to be a ListTableInputt? And then wrote tests against it? If so 👍 |
|
Precisely. |
|
Would it be acceptable to have #79 too, in the interests of allowing consumers of the AWS-SDK to decide how they want to test within their applications? |
|
@geneticgenesis the handlers architecture should work as a superset of #79. Can you provide an example for what the interface enables that the handlers do not? |
|
One less desireable property of the handlers is having to know how the But having one way to do things may trump the extra boilerplate? Is there On Tue, Feb 10, 2015 at 10:35 AM, Loren Segal notifications@github.com
|
|
The primary use case would be for people who want to use tools like gomock or testify/mock rather than using library-specific approaches for changing behaviour in test scenarios. I'm not saying the hander approach is particularly bad, what I am saying is that lots of people are already using a standard set of tools and approaches. IMO it would be sensible for this library to allow people to continue to do that. |
aws-sdk-go should already be allowing users to continue using a library like gomock if they want. AFAIK, that library should work just fine with the API as is. In other words, you should be able to define the interfaces from #79 in your own test code: package mypkg_test
type AutoScalingAPI interface {
AttachInstances(req *autoscaling.AttachInstancesQuery) (err error)
// and anything else you might be using
}Even gomock's own documentation lists step 1 under the assumption that the interface is not yet defined for the thing you are trying to mock-- my guess is that this is likely the case for many libraries out there-- so it seems fairly reasonable to define a custom interface for the few methods of the API that you are using. I also think it seems fair to keep this declaration in your tests only, since this interface would only ever be useful to test code specifically using one of these testing libraries. I'm not sure there is value to having an extra interface in the documentation for every service when it is only useful for very specific unit testing scenarios. If we supported like #79, I would vote that we should probably at least be placing those interfaces in a separate package to simplify that documentation story. In that same vein, another solution would be to not actually generate the interfaces but allow those interface files to be generated on-demand using the same codebase as our generators and document supported instructions for that testing strategy when using something like gomock (using a command workflow similar to gomock). This, again, wouldn't be a necessary workflow path, it would simply be a convenience generator for doing the above en-masse in the cases where you are testing with a large body of API calls from a single service. Does any of this sound reasonable? |
|
So here's an example repo I put together to explore this: https://github.com/GeneticGenesis/aws-sdk-go-mock-demos - I'd like to expand it to cover more approaches. It actually turned out to be more elegant than I was expecting. I think a lot of my concerns were around still using the type definitions from within the AWS package, but this turned out to be just fine. It uses Testify and Mockery to achieve a mock of the DynamoDB Query() call. I hand generated an interface for the one call I required, and used that in my tests. I've used a compile time check to enforce that the interface I declare fits the one in DynamoDB. In practise, this compile time check is really useful, because it will compilation failures if the DynamoDB API changes, forcing me to regenerate my interface. I'm still not hugely comfortable with me owning this responsibility though. If I want to have a DynamoDB encapsulated in any of my structs, I have to declare it as my interface type (DynamoDBer in the case of my example), in order to be allowed to replace it with a mock at test time - I'd really prefer that to exist in one place (ideally with the real declaration), so I don't litter every usage of AWS I have with small, incomplete interfaces. I'd have no problem with the interfaces being in a different package at all, but yes, I'd prefer AWS to own the process of generating, versioning and releasing them. Happy to chip in on modifying #79 to change the generation location and add the interface verification as I had in my example. Thoughts?... @sclasen? |
|
FWIW I have been using hand build interfaces that contain the subset of operations I need in my projects as well, and not just for testing. As you are doing in the example, code that needs to talk to dynamo gets a I think I'll have to actually use the |
|
FWIW I would really prefer that aws-sdk-go provide interfaces and default implementations of those interfaces. Taking Kinesis as an example I would expect something like this: type Kinesis interface {
AddTagsToStream(*AddTagsToStreamInput) error
DescribeStream(*DescribeStreamInput) (*DescribeStreamOutput, error)
...
}
type KinesisImpl struct { ... }
func (ki *KinesisImpl) AddTagsToStream(req *AddTagsToStreamInput) (err error) {
...
}
func (ki *KinesisImpl) DescribeStream(req *DescribeStreamInput) (resp *DescribeStreamOutput, err error) {
...
}Note: This is a deviation from the idiomatic Yes this means I still need to mock stuff, but for the most part that's just managing the responses of concrete response structs, but that's what I'd want anyway. |
|
@freeformz I would really expect the client (user of this lib) to generate purpose build interfaces. *Impl classes really remind me of bad times! |
|
@kidoman Generally speaking I agree. But, every project that I've seen using this lib ends up having to re-create the interfaces at least for the purposes of testing. Maybe that's fine though and it's a mistake to try to optimize for this use case. In any case this now seems moot as the owners of the lib have gone down the path of allowing users to supply a mock Endpoint (probably via httptest), which is a fine, if slightly clunky alternative, at least upon initial examination. |
|
I've just added generating interfaces for each service. With the interface you'll be able to generate mocks using a tool like mockery. The new interfaces can be found in the iface package under each service. If you are wanting to use mocks with the service interfaces you'll need to make sure to use the service interfaces instead of concrete service structs. Here's an example: package main
import (
"github.com/awslabs/aws-sdk-go/service/s3"
"github.com/awslabs/aws-sdk-go/service/s3/s3iface"
"log"
)
func main() {
var svc s3iface.S3API = s3.New(nil)
// With Interface.
buckets, err := listBuckets(svc)
if err != nil {
log.Fatalln("Failed to list buckets", err)
}
log.Println("Buckets:", buckets)
}
func listBuckets(svc s3iface.S3API) ([]string, error) {
resp, err := svc.ListBuckets(&s3.ListBucketsInput{})
if err != nil {
return nil, err
}
buckets := make([]string, 0, len(resp.Buckets))
for _, b := range resp.Buckets {
buckets = append(buckets, *b.Name)
}
return buckets, nil
}In this example I manually ran mockery to generate the "mocks" package for the s3 interface. The mocked packages weren't included in this commit, because people might have different ways they would like to mock out the service apis. package main
import (
"github.com/awslabs/aws-sdk-go/aws"
"github.com/awslabs/aws-sdk-go/service/s3"
"github.com/awslabs/aws-sdk-go/service/s3/s3iface/mocks"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"testing"
)
func TestListBucketsWithInf(t *testing.T) {
svc := new(mocks.S3API)
svc.On("ListBuckets", mock.AnythingOfType("*s3.ListBucketsInput")).Return(&s3.ListBucketsOutput{
Buckets: []*s3.Bucket{
&s3.Bucket{Name: aws.String("First Bucket")},
&s3.Bucket{Name: aws.String("Second Bucket")},
},
}, nil)
b, err := listBuckets(svc)
assert.Nil(t, err, "Expected no error")
assert.Len(t, b, 2, "Expect two buckets")
assert.Equal(t, "First Bucket", b[0], "Expected first bucket")
assert.Equal(t, "Second Bucket", b[1], "Expected Second Bucket")
} |
|
For posterity's sake, here's the current tests showing how to use the handler system: Note the last test which shows how to add and remove "named" handlers. |
|
This works great with a single handler, but when I use multiple handlers each handler runs n times where n is the number of handlers I've added. For example with this: r53 := route53.New(session.New(), nil)
r53.Handlers.Clear()
r53.Handlers.Build.PushBack(func(r *request.Request) {
log.Printf("first: %T\n", r.Params)
})
r53.Handlers.Build.PushBack(func(r *request.Request) {
log.Printf("second: %T\n", r.Params)
})
r53.ListResourceRecordSets(&route53.ListResourceRecordSetsInput{
HostedZoneId: aws.String("abc"),
})
r53.ChangeResourceRecordSets(&route53.ChangeResourceRecordSetsInput{
HostedZoneId: aws.String("abc"),
ChangeBatch: &route53.ChangeBatch{},
})I get this output: but I expect to get this: If I add a third handler, it will execute each of the three functions three times even though I'm only calling it once. Am I misusing? |
|
Hello @cristiangraz - Thank you for coming here and asking this. When you add handlers to the package they affect all the operations in the package. So, let's define n to be the number of handlers and m be the number of operations called. We should expect n * m messages. For more clarification, you aren't calling it once. You are calling each handler every time you call an operation. I hope that makes sense. Please let me know if you have further questions. |
|
Note that the handlers are attached to the service object in this case, so you could create multiple r53 := route53.New(session.New(), nil)
r53.Handlers.Clear()
r53.Handlers.Build.PushBack(func(r *request.Request) {
log.Printf("first: %T\n", r.Params)
})
req, _ := r53.ListResourceRecordSetsRequest(&route53.ListResourceRecordSetsInput{
HostedZoneId: aws.String("abc"),
})
req.Handlers.Build.PushBack(func(r *request.Request) {
log.Printf("second: %T\n", r.Params)
})
// Callback executes on this operation
req.Send()
// But not on this one
r53.ChangeResourceRecordSets(&route53.ChangeResourceRecordSetsInput{
HostedZoneId: aws.String("abc"),
ChangeBatch: &route53.ChangeBatch{},
}) |
|
@lsegal - Absolutely! Thank you for a more clarified answer. |
|
Nice discussion. It would be awesome if some of these ideas would be written down into a wiki page dedicated to testing mechanisms for the SDK, since much of this is not really straightforward and much unlike anything I've seen in other golang projects. |
|
Hello @cristim, that sounds like a great idea! I'll put that in our backlog so we can plan it in one of our sprints. Thank you for the feedback! |
|
@xibz how's that backlog looking? |
|
Hello @shatil, thank you for reaching out to us. We are doing our best to have the best examples and up to date documentation for our users. I'll raise the priority on this in our backlog. Do you have any particular issue that you are trying to test? |
|
@xibz great to hear! I'm writing some unit tests for Go code interacting with DynamoDB. I discovered how to mock I'm still writing the tests. I'm wondering what approach to use for mocking things like an |
|
Sure, an For instance, package mocktest
var Session = session.New(&aws.Config{})But either is fine. Was there something you did not like with the |
|
@xbiz it's taken a while to wrap my head around this. How can I define a variable, or field in a (DynamoDB's |
|
@shatil - I think the best way is the Also, if I understood your questions correctly, it sounds like you want to do something like this: type MockDynamoDB struct {
dynamodbiface.DynamoDBAPI
}
var mock MockDynamoDBNow your tests will look like this func foo() {
err := mock.PutItem(/* put item params */)
// assert err
} |
|
That's actually what I'd like to avoid since: func (self *MyObject) MyMethod(svc *dynamodbiface.DynamoDBAPI) {svc.PutItem(/* ... */)}
my_object.MyMethod(svc) // every method then requires an additional parameterI'd like instead to define in a file like var DynamoDB *dynamodb.DynamoDB = dynamodb.New()Then func (self *MyObject) MyMethod() {lib.DynamoDB.PutItem(/* ... */)}
my_object.MyMethod()Go doesn't allow me to cast |
|
@shatil - Are you casting as Here's an example: package main
import (
"fmt"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/s3"
"github.com/aws/aws-sdk-go/service/s3/s3iface"
)
func main() {
svc := s3.New(session.New())
ifaceClient := s3iface.S3API(svc)
fmt.Println(ifaceClient)
} |
|
@xibz your suggestion worked perfectly. Thank you! Example package main
import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/dynamodb"
"github.com/aws/aws-sdk-go/service/dynamodb/dynamodbiface"
)
type AmazonWebServices struct {
Config *aws.Config
Session *session.Session
DynamoDB dynamodbiface.DynamoDBAPI
}
// Example configuration block, which runs fine as-is in tests.
func ConfigureAws() {
var Aws *AmazonWebServices = new(AmazonWebServices)
Aws.Config = &aws.Config{Region: aws.String("us-west-2"),}
Aws.Session, _ = session.NewSession(Aws.Config) // New() deprecated
var svc *dynamodb.DynamoDB = dynamodb.New(Aws.Session)
Aws.DynamoDB = dynamodbiface.DynamoDBAPI(svc) // Thanks for this!
}
func (self *AmazonWebServices) GetFromDynamoDB(key string) {
var parameters *dynamodb.GetItemInput = // Configure parameters...
var response *dynamodb.GetItemOutput
var err error
response, err = self.DynamoDB.GetItem(parameters)
// Do stuff w/ your fancy response! (Or error.)
}Then package main
import (
"testing"
"github.com/aws/aws-sdk-go/service/dynamodb"
"github.com/aws/aws-sdk-go/service/dynamodb/dynamodbiface"
)
type FakeDynamoDB struct {
dynamodbiface.DynamoDBAPI
// I piggyback expected payloads here, too.
}
func (self *FakeDynamoDB) GetItem(input *dynamodb.GetItemInput) (*dynamodb.GetItemOutput, error) {
// Your fake GetItem.
}
TestGetFromDynamoDB(t *testing.T) {
test_aws = new(AmazonWebService)
test_aws.DynamoDB = &FakeDynamoDB{}
test_aws.GetFromDynamoDB("this particular key")
} |
|
@shatil - Awesome, glad we could get that clarified for you :). Also, if you want to submit this example in the |
|
Hi guys, sorry to post in an old issue here. I couldnt find any solution elsewhere and figured my question is kinda related to this one. I have following code for signing a request to s3 req, _ := svc.s3.GetObjectRequest(&s3.GetObjectInput{
Bucket: aws.String("bucket"),
Key: aws.String("key"),
})
dst, err := req.Presign(5 * time.Minute)I can mock out |
|
@shangsunset |
|
@lsegal Thanks for you reply. You had a good point. I didnt really look into So in that case, theres no reason to mock |
Fixes Metadata, StorageClass, ACL, and RequestPayer fields in UploadInput. Also updates test unit to identify if the types go out of sync. Fix aws#88.
Release v2.0.0-preview.2 (2018-01-15) === ### Services * Synced the V2 SDK with latests AWS service API definitions. ### SDK Bugs * `service/s3/s3manager`: Fix Upload Manger's UploadInput fields ([aws#89](aws/aws-sdk-go-v2#89)) * Fixes [aws#88](aws/aws-sdk-go-v2#88) * `aws`: Fix Pagination handling of empty string NextToken ([aws#94](aws/aws-sdk-go-v2#94)) * Fixes [aws#84](aws/aws-sdk-go-v2#84)
Consider the following case:
I have a package within my project which is my DAO package. It has methods like
GetFooById("some-id"), which returns me the Foo object from dynamoDB which has the ID "some-id".This package constructs a dynamo condition, and calls
DynamoClient.Query().How should one unit test my package without hitting a real DynamoDB, or without running a stub server which responds with AWS-like responses?
Here's what I've considered/experimented with:
Cons: I have to then maintain a lot of extra interfaces, just for testing purposes
Cons: Withmock isn't well maintained, is slow, and doesn't play well with tools like godep or gocheck
Cons: Is much like suggestion 1, and implies you have lines of code which aren't technically tested.
Other options:
What are people's thoughts on this?
What is the intention for onward testing of this sdk? (I see a small amount of unit tests at the client level, and some integration tests on the modules...)
The text was updated successfully, but these errors were encountered: