Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Direct mocking of clients in v0.25.0? #786

Closed
2 tasks done
zubinmadon opened this issue Sep 30, 2020 · 9 comments
Closed
2 tasks done

Direct mocking of clients in v0.25.0? #786

zubinmadon opened this issue Sep 30, 2020 · 9 comments
Assignees
Labels
guidance Question that needs advice or information.

Comments

@zubinmadon
Copy link

Confirm by changing [ ] to [x] below:

In previous versions of the sdk, we used interface types in the various *iface packages (e.g. s3iface, dynamodbiface, etc) to easily mock things. I see these are no longer present in v0.25.0. A few questions around this:

  • What is the recommended method of writing mock objects for the new Client types? Do you have any examples?
  • Will interfaces for Client types be added?
  • While it would not be ideal for our uses, is mocking the HTTPClient interfaces sufficient to avoid network traffic, in a pinch?
  • Alternatively, do you recommend writing custom middleware for simple mocks? If so, any examples?

Looking for some guidance before diving too deep into this, thank you!

@zubinmadon zubinmadon added the guidance Question that needs advice or information. label Sep 30, 2020
@skmcgrail skmcgrail self-assigned this Oct 1, 2020
@skmcgrail
Copy link
Member

skmcgrail commented Oct 1, 2020

Thank you for reaching out to us @zubinmadon.

What is the recommended method of writing mock objects for the new Client types? Do you have any examples?

The best way to mock the Client types is to define an interface in your package that defines the API(s) that you require. You can then provide any type that satisfies this interface to mock output responses or errors from the client. Here is one example method that can be used to mock responses and validate inputs to the client.

type s3GetObjectAPI interface {
	PutObject(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error)
}

func GetObjectFromS3(api s3GetObjectAPI, bucket, key string) ([]byte, error) {
	object, err := api.PutObject(context.Background(), &s3.GetObjectInput{
		Bucket: &bucket,
		Key:    &key,
	})
	if err != nil {
		return nil, err
	}

	all, err := ioutil.ReadAll(object.Body)
	if err != nil {
		return nil, err
	}
	return all, nil
}

func TestGetObjectFromS3(t *testing.T) {
	cases := []struct {
		client func(t *testing.T) s3GetObjectAPI
		bucket string
		key    string
		expect []byte
	}{
		{
			client: func(t *testing.T) s3GetObjectAPI {
				return mockGetObjectAPI(func(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error) {
					t.Helper()
					if params.Bucket == nil {
						t.Fatal("expect bucket to not be nil")
					}
					if e, a := "fooBucket", *params.Bucket; e != a {
						t.Errorf("expect %v, got %v", e, a)
					}
					if params.Key == nil {
						t.Fatal("expect key to not be nil")
					}
					if e, a := "barKey", *params.Key; e != a {
						t.Errorf("expect %v, got %v", e, a)
					}

					return &s3.GetObjectOutput{
						Body: ioutil.NopCloser(bytes.NewReader([]byte("this is the body foo bar baz"))),
					}, nil
				})
			},
			bucket: "fooBucket",
			key:    "barKey",
			expect: []byte("this is the body foo bar baz"),
		},
	}

	for i, tt := range cases {
		t.Run(strconv.Itoa(i), func(t *testing.T) {
			content, err := GetObjectFromS3(tt.client(t), tt.bucket, tt.key)
			if err != nil {
				t.Fatalf("expect no error, got %v", err)
			}
			if e, a := tt.expect, content; bytes.Compare(e, a) != 0 {
				t.Errorf("expect %v, got %v", e, a)
			}
		})
	}
}

type mockGetObjectAPI func(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error)

func (m mockGetObjectAPI) PutObject(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error) {
	return m(ctx, params, optFns...)
}

var _ s3GetObjectAPI = mockGetObjectAPI(nil)

Will interfaces for Client types be added?

Our current plan is to not include generated interface definitions for the clients. (Previously generated types such as s3iface.ClientAPI etc). We will be evaluating feedback from users and can revisit this decision based on broader feedback from the community. To give some background on this decision, one reason is that the ClientAPI interface can be "broken" between minor/patch versions release of a client. As new supported API's are added the client they would need to be added to this interface type. This addition of new method signatures to the interface can break users who rely on either generated or handwritten mocks. This is counter to the goal of semantic versioning of the module. Second it is traditionally considered idiomatic for applications to define the interfaces for their respective dependencies, rather then libraries generating interface definitions for the express purpose of users taking a dependency on them. The interface definitions we want to expose from the SDK types should be ones that are there to allow users to augment or plug in their own types.

While it would not be ideal for our uses, is mocking the HTTPClient interfaces sufficient to avoid network traffic, in a pinch?

Mocking can be done via the HTTPClient interface, but this unnecessarily exposes your tests to having to have knowledge about the protocol responses so that they can be properly de-serialized by the client. It would be better to mock the client as shown earlier.

Alternatively, do you recommend writing custom middleware for simple mocks? If so, any examples?

This is also an alternative that could be used for mocking as you could simply uses the middleware to return the mocked output response type. I would still suggest mocking via the client though.

@skmcgrail skmcgrail added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 1, 2020
@zubinmadon
Copy link
Author

Thanks for your response!

This all makes sense. Given the scope and size of AWS, we had been treating the *iface.ClientAPI interface types as the API, of which the SDK was an implementation. I understand your desire to have clients define their own interfaces. If we stick only to this, it sort of collapses our "integration-style unit tests" where we effectively were unit testing our use of the AWS SDK (again given the scope of AWS, these were quite helpful to catch simple breaking issues early and often). We may however be able to replace them with a lower level HTTPClient mocks or middleware stubbing.

This addition of new method signatures to the interface can break users who rely on either generated or handwritten mocks.

Incidentally, there's a nice workaround for this in golang: composing the interface into your mock struct. Then you only need to mock the methods you use.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 3, 2020
This was referenced Oct 9, 2020
@shogo82148
Copy link
Contributor

Here is one example method that can be used to mock responses and validate inputs to the client.

If I use few APIs, this method might work. However, I sometimes use hundreds of APIs. Defining those interfaces is a very boring task.

Incidentally, there's a nice workaround for this in golang: composing the interface into your mock struct. Then you only need to mock the methods you use.

aws-sdk-go v1 recommends this workaround.
e.g. https://github.com/aws/aws-sdk-go/blob/36f57c5b29d6bf8c21cc5c05bd648b28ce4872fe/service/s3/s3iface/interface.go#L41-L47

    // Define a mock struct to be used in your unit tests of myFunc.
    type mockS3Client struct {
        s3iface.S3API
    }
    func (m *mockS3Client) AbortMultipartUpload(input *s3.AbortMultipartUploadInput) (*s3.AbortMultipartUploadOutput, error) {
        // mock response/functionality
    }

Why has aws-sdk-go v2 stopped using this approach?

@shogo82148
Copy link
Contributor

here is another approach.

https://gist.github.com/haya14busa/27a12284ad74477a6fd6ed66d0d153ee
http://haya14busa.com/golang-how-to-write-mock-of-interface-for-testing/ (in Japanese)

type fakeGitHub struct {
	GitHub // this is an interface.
	FakeCreateRelease func(ctx context.Context, opt *Option) (string, error)
	FakeGetRelease    func(ctx context.Context, tag string) (string, error)
}

func (c *fakeGitHub) CreateRelease(ctx context.Context, opt *Option) (string, error) {
	return c.FakeCreateRelease(ctx, opt)
}

func (c *fakeGitHub) GetRelease(ctx context.Context, tag string) (string, error) {
	return c.FakeGetRelease(ctx, tag)
}

@strongishllama
Copy link

strongishllama commented Jan 22, 2021

Second it is traditionally considered idiomatic for applications to define the interfaces for their respective dependencies, rather then libraries generating interface definitions for the express purpose of users taking a dependency on them. The interface definitions we want to expose from the SDK types should be ones that are there to allow users to augment or plug in their own types.

@skmcgrail This makes sense to keep things as idiomatic as possible. To make unit testing easier, and validation in general, it would be nice to have the validate methods from V1 of the SDK brought into V2. Then you would be able to do the following for unit tests.

// Interface only defining methods that are required.
type S3API interface {
	PutObject(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error)
}

// Mock implementation for unit tests.
type MockS3API struct {}

func (m *MockS3API) PutObject(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error) {
	// Check basic validation. For example, not using invalid characters in the bucket name.
        if err := params.Validate(); err != nil {
		return nil, err
	}

        // Perform additional validation here...

        // Then...
        return output, nil
}

This still keeps the implementation idiomatic while reducing the work required to get basic validation for inputs. Curious to hear your thoughts on this. Thanks!

@StummeJ
Copy link

StummeJ commented May 18, 2021

Is there any movement on this? We're trying to use go mock's mockgen and the current API is not possible to mock out. So we either can create a proxy layer or have the interfaces that enable mock generation. The proxy layer being pushed on the end consumer seems a bit ridiculous when you're using more than 1 or 2 URLs, which in AWS you need almost 10 to do anything

@soroushatarod
Copy link

its difficult to mock the v2 library. it would be great if someone looks into this issue

@Skarlso
Copy link

Skarlso commented Jul 5, 2021

To be fair, Go recommends actually doing just that. Write your own interfaces. It's not the third party's responsibility to generate an interface and expose it for external usage if it doesn't make sense internally just so the user can mock it.

Go is awesome in the way that it lets use accept anything that has a set functions attached to it. So it's your, aka the user's, responsibility to create interfaces for the thing you are using and require those. If you require interfaces and return concretes then you'll be decoupled from your third party software's working structure.

Anyways, here's an example to how to mock v2 which worked about 2 years ago... https://github.com/go-furnace/go-furnace/blob/master/furnace-aws/commands/create_test.go

@Cyberax
Copy link
Contributor

Cyberax commented Jul 26, 2021

If you're interested in mocking the AWS API, you can try this approach: https://gist.github.com/Cyberax/eb42d249d022c55ce9dc6572309200ce

Basically, I take a Go AWS SDK service client and add exterminate most of the existing handlers, short-circuiting services to the mock handlers.

This allows to mock code with minimum overhead. Example:

	am := NewAwsMockHandler()
	am.AddHandler(func(ctx context.Context, arg *ec2.TerminateInstancesInput) (
		*ec2.TerminateInstancesOutput, error) {

		if arg.InstanceIds[0] != "i-123" {
			panic("BadInstanceId")
		}
		return &ec2.TerminateInstancesOutput{}, nil
	})

	ec := ec2.NewFromConfig(am.AwsConfig())

	_, _ = ec.TerminateInstances(context.Background(), &ec2.TerminateInstancesInput{
		InstanceIds: []string{"i-123"},
	})
       
        // Will panic:
	_, _ = ec.TerminateInstances(context.Background(), &ec2.TerminateInstancesInput{
		InstanceIds: []string{"i-456"},
	})

@aws aws locked and limited conversation to collaborators Dec 17, 2021
@KaibaLopez KaibaLopez converted this issue into discussion #1536 Dec 17, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
guidance Question that needs advice or information.
Projects
None yet
Development

No branches or pull requests

8 participants