Add test for DescribeInstances

[nckturner]

• Add test for DescribeInstances (the lower level wrapper around the API call).
• Use interface in awsSdkEC2 to allow for mock.
• Check len(request.InstanceIds) == 0 to account for both empty structs and nil pointers.

See #274 for the change and #269 for some context.

What type of PR is this?

/kind cleanup

What this PR does / why we need it:
Add a test and make a comparison more resilient.

Does this PR introduce a user-facing change?:

NONE

[wongma7]

/home/prow/go/src/k8s.io/cloud-provider-aws/pkg/providers/v1/aws_test.go:3756:6: don't use underscores in Go names; func expectDescribeInstances_EmptyInput should be expectDescribeInstancesEmptyInput

[wongma7]

why is it interface{}? what is expect supposed to do? what would a failure look like? if in every test case I tell DescribeInstances what to take as input and what to return, what is being tested?

[nckturner]

I think I'm missing an AssertExpectations call in the test (I was a little confused as to whether I needed it while writing the test, because the test does fail if DescribeInstances is not called as expected). Let me see if I can determine if its needed and clear up the logic.

[wongma7]

Documentation for On: https://pkg.go.dev/github.com/stretchr/testify/mock#Call.On
so I am telling it what to return when given so and so input. And I think ultimately I have to assert that this call happened at all by doing https://pkg.go.dev/github.com/stretchr/testify/mock#Mock.AssertExpectations

(new to this package, slightly more familiar with gomock)

[wongma7]

we dont need Assert since Called basically does it for us . https://pkg.go.dev/github.com/stretchr/testify/mock?utm_source=godoc#Mock.Called

[nckturner]

Right, that's the reason that I didn't have an explicit call to AssertExpectations -- because I was seeing tests fail if I called the expected functions with unexpected arguments. That being said, I think we do want an explicit call to AssertExpectations because of the missing case. If there is an assert, and a missing call, it will only be caught by the explicit AssertExpectations, whereas Called will catch calls with improper arguments.

[wongma7]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nckturner, wongma7

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [nckturner,wongma7]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment