feature req: Auto-generate interface and fake implementation

[colemickens]

I've observed two things regarding usages of the azure-sdk-for-go:

1. Creation of an interface that covers the surface of the various clients exposed. (For testing/mocking purposes)
2. Implementation of a faked out implementation of those interfaces.

I'm wondering if those could be auto-generated for the user?

(If someone can send me a DL for the SDK team I can forward an internal link to a PR that demonstrates the couple of things that could potentially be auto-generated)

[marstr]

I like this idea a lot! However, I believe it belongs in the backlog. Thoughts, @colemickens, @salameer, @jhendrixMSFT, @mcardosos?

[mcardosos]

Yessss! I looked into Cole's code, and yes, I think it is possible to auto-gen :D Sill, I agree this belongs in the backlog.

[colemickens]

Would it be possible to split this? If it were just "generate an interface for all clients" it should just be a matter of outputting the function prototypes into an interface type for each client.

Presumably that would be a short, simple task, certainly compared to generating an actual fake client somehow. As is, today, it's hard to use the Go SDK in places where you'd like to write good tests. I wind up "manually generating" the interface effectively and then writing the mock behind it.

[joshgav]

not sure if this is still needed. closing for now, please re-open if needed. thanks!

[colemickens]

Yes. It's needed. See this for an example: https://github.com/kubernetes/kubernetes/blob/34001d8c6a0ed4edf02d91c5b45920a2676418f9/pkg/cloudprovider/providers/azure/azure_client.go

Please auto-gen interfaces for the clients for mocking/testing purposes.

[jhendrixMSFT]

@colemickens What do you think of the following (a snippet from generating redis).

// MockClient contains the set of methods on the Client type.
type MockClient interface {
    CheckNameAvailability(ctx context.Context, parameters CheckNameAvailabilityParameters) (result autorest.Response, err error)
    Create(ctx context.Context, resourceGroupName string, name string, parameters CreateParameters) (result CreateFuture, err error)
    Delete(ctx context.Context, resourceGroupName string, name string) (result DeleteFuture, err error)
    ExportData(ctx context.Context, resourceGroupName string, name string, parameters ExportRDBParameters) (result ExportDataFuture, err error)
    ForceReboot(ctx context.Context, resourceGroupName string, name string, parameters RebootParameters) (result ForceRebootResponse, err error)
    Get(ctx context.Context, resourceGroupName string, name string) (result ResourceType, err error)
    ImportData(ctx context.Context, resourceGroupName string, name string, parameters ImportRDBParameters) (result ImportDataFuture, err error)
    List(ctx context.Context) (result ListResultPage, err error)
    ListByResourceGroup(ctx context.Context, resourceGroupName string) (result ListResultPage, err error)
    ListKeys(ctx context.Context, resourceGroupName string, name string) (result AccessKeys, err error)
    ListUpgradeNotifications(ctx context.Context, resourceGroupName string, name string, history float64) (result NotificationListResponse, err error)
    RegenerateKey(ctx context.Context, resourceGroupName string, name string, parameters RegenerateKeyParameters) (result AccessKeys, err error)
    Update(ctx context.Context, resourceGroupName string, name string, parameters UpdateParameters) (result ResourceType, err error)
}

I'm open to changing the interface type name.

[colemickens]

Hm, some of these interfaces will be large enough that folks may not use them, due to not wanting or needing to implement every bit of them. And I don't think you'll be able to use a generic interface anyway, given Go's type system.

Let me come back to this and see who added the small kubenetes interfaces and see what they think about this.

[mcardosos]

What about an interface per operation?

type MockClientCheckNameAvailability interface {
    CheckNameAvailability(ctx context.Context, parameters CheckNameAvailabilityParameters) (result autorest.Response, err error)
}

type MockClientCreate interface {
    Create(ctx context.Context, resourceGroupName string, name string, parameters CreateParameters) (result CreateFuture, err error)
}

[jhendrixMSFT]

@colemickens did you ever find out about this? @tombuildsstuff would Terraform find this useful?

[colemickens]

I think @brendandburns added the smaller interfaces. He might be more apt to tell you if this is a good idea or not. I've changed my desire for this quite a bit after considering how large the autorest interfaces are... it may well be the case that consumers would prefer to author their own interfaces for testingm etc.

[tombuildsstuff]

@colemickens looking at the code in https://github.com/kubernetes/kubernetes/blob/34001d8c6a0ed4edf02d91c5b45920a2676418f9/pkg/cloudprovider/providers/azure/azure_client.go - I may be wrong, but I have a feeling that behaviour could be achieved using the Inspectors within AutoRest instead?

@jhendrixMSFT from Terraform's perspective, I don't believe we'd have a use for this functionality since we're almost entirely using acceptance tests / calling the Azure API directly.

[jhendrixMSFT]

Given that the desire for this has changed I'm going to close this.

[0xmichalis]

This is a must for unit-testing code that uses azure clients. Please consider reopening.

[jhendrixMSFT]

@kargakis this was added to the code generator today and will ship in the next major version of the SDK.

[0xmichalis]

@jhendrixMSFT great news! Can you please let me know the exact version this change will be part of?

[jhendrixMSFT]

@kargakis It will ship in v23 of the SDK at the end of this month. Take a look at https://github.com/Azure/azure-sdk-for-go/pull/3282 to see the changes, should merge into latest later today.

[jim-minter]

Hi @jhendrixMSFT, is there more to this than just the interface definitions? Do you supply any sort of mock/fake classes which implement those interfaces?

[jhendrixMSFT]

@jim-minter at present no although I'm open to suggestions. Does something like this work for you?

package main

import (
    "context"

    "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute"
    "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute/computeapi"
)

type mockVirtualMachinesClient struct {
    computeapi.VirtualMachinesClientAPI
}

func (mc mockVirtualMachinesClient) List(ctx context.Context, resourceGroupName string) (result compute.VirtualMachineListResultPage, err error) {
    return compute.VirtualMachineListResultPage{}, nil
}

func main() {
    c := mockVirtualMachinesClient{}
    c.List(context.Background(), "rg")
}

[sopatwar]

@jhendrixMSFT This would very useful for me. Is there any plan to add these kind of mockXXClient APIs for Microsoft.Network APIs ?

Thanks, Sourabh

[jhendrixMSFT]

@sopatwar take a look at https://github.com/Azure/azure-sdk-for-go/blob/master/services/network/mgmt/2019-07-01/network/networkapi/interfaces.go

[sopatwar]

Hi @jhendrixMSFT, is there more to this than just the interface definitions? Do you supply any sort of mock/fake classes which implement those interfaces?

@jhendrixMSFT I was actually referring to mock/fake classes as mentioned in this comment.

[jhendrixMSFT]

Oh I see, you must provide your own mock types. Here's a sample, mocking one of the APIs.

package main

import (
    "context"

    "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute"
    "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute/computeapi"
)

func main() {
    client := getClient(true)
    client.Get(context.Background(), "rg", "vm", compute.InstanceView)
}

func getClient(mock bool) computeapi.VirtualMachinesClientAPI {
    if mock {
        return mockVMClient{}
    }
    return compute.NewVirtualMachinesClient("subID")
}

type mockVMClient struct {
    compute.VirtualMachinesClient
}

func (m mockVMClient) Get(ctx context.Context, resourceGroupName string, VMName string, expand compute.InstanceViewTypes) (result compute.VirtualMachine, err error) {
    return compute.VirtualMachine{}, nil
}

[sopatwar]

Ok, I see how this can be done for Get() or List() APIs.

Any pointers on how to mock CreateOrUpdate() or Delete() APIs? Typically, such APIs would return a Future object.

Thanks, Sourabh

[jhendrixMSFT]

Hmm this is where it gets a little more complicated. I think the only way to achieve this at present is to mock up a JSON response then unmarshal that into the future. Take a look at https://github.com/Azure/go-autorest/blob/master/autorest/azure/async.go#L319 for the JSON format. Please feel free to open an issue in the go-autorest repo to simplify mocking future responses however I don't know when we'll have cycles to implement it (you could also submit a PR if you're up for it and we can iterate on it).

[sopatwar]

@jhendrixMSFT I tried to code up a sample implementation. The code as written will not compile because of the "ERROR" lines. Is there any good way to avoid type checks when using the mock client in the code below:

package main

import (
    "context"
    "fmt"
    "net/http"
    "testing"

    "github.com/Azure/azure-sdk-for-go/profiles/latest/eventhub/mgmt/eventhub"
    "github.com/Azure/azure-sdk-for-go/profiles/latest/eventhub/mgmt/eventhub/eventhubapi"
    "github.com/Azure/go-autorest/autorest"
    "github.com/Azure/go-autorest/autorest/azure"
    "github.com/Azure/go-autorest/autorest/to"
)

var (
    // TODO: init these vars
    gFutureResp *http.Response
    gAuth       autorest.Authorizer
)

func TestMockCreate(t *testing.T) {
    nsClient := getNamespacesClient(true)
    // ERROR on next line
    nsClient.Authorizer = gAuth

    ctx := context.Background()
    rgName := "TestRG"
    nsName := "TestNS"
    rgLocation := "westus"

    nsFuture, err := nsClient.CreateOrUpdate(
        ctx,
        rgName,
        nsName,
        eventhub.EHNamespace{
            Location: to.StringPtr(rgLocation),
            Sku: &eventhub.Sku{
                Name: eventhub.Basic,
                Tier: eventhub.SkuTierBasic,
            },
        },
    )
    if err != nil {
        fmt.Println("Error:", err)
    }

    // ERROR on next line
    err = nsFuture.Future.WaitForCompletionRef(ctx, nsClient.BaseClient.Client)
    if err != nil {
        fmt.Println("Error:", err)
    }

    // ERROR on next line
    evhNamespace, err := nsFuture.Result(nsClient)
    if err != nil {
        fmt.Println("Error:", err)
    }
    fmt.Println("Created NS:", *evhNamespace.ID)
}

func getNamespacesClient(mock bool) eventhubapi.NamespacesClientAPI {
    if mock {
        return mockNamespacesClient{}
    }
    return eventhub.NewNamespacesClient("subID")
}

type mockNamespacesClient struct {
    eventhub.NamespacesClient
}

func (client mockNamespacesClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, namespaceName string, parameters eventhub.EHNamespace) (result eventhub.NamespacesCreateOrUpdateFuture, err error) {
    result.Future, err = azure.NewFutureFromResponse(gFutureResp)
    return
}

[jhendrixMSFT]

Looks like completely mocking the future isn't really possible. This is tracked in https://github.com/Azure/go-autorest/issues/469