Azure / azure-sdk-for-go

This repository is for active development of the Azure SDK for Go. For consumers of the SDK we recommend visiting our public developer docs at:
https://docs.microsoft.com/azure/developer/go/
MIT License
1.64k stars 839 forks source link

Fakes Support #16613

Closed MartinForReal closed 7 months ago

MartinForReal commented 2 years ago

I'm wondering if there will be support for mock client because it will help end user develop unit test case.

Rohitrajak1807 commented 2 years ago

A mock client like in the case of knative package for GoLang would be helpful. They define a reactor(this object holds the logic of how the client would "react" given some input), and a mock client which then requests this reactor for some data. To be specific: https://github.com/knative/client/blob/main/pkg/serving/v1/client_test.go

haitch commented 2 years ago

please priority this request, this is blocking AKS from moving to track2 API.

we rely on interface for our unittest too.

serbrech commented 2 years ago

I really don't think we need interfaces for all clients. these can be written by the consumer. as long as the sdk allows for testability, there is no blocker. if you do end up providing interfaces, please don't put them in the same package as the client. I personally wouldn't want these interfaces when I import the client.

what does help testability is some test fakes for some particular constructs. for example

haitch commented 2 years ago

we definitely need type Poller[T any] struct to have a interface.

and all BeginCreateOrUpdate/BeginDelete return this Interface, instead of concrete type.

@serbrech how do you get around this issue?

jhendrixMSFT commented 2 years ago

Returning an interface is not versioning tolerant, and we explicitly chose not to do that here for this reason.

Here's an example of how you could create a mock Poller[T] https://gist.github.com/jhendrixMSFT/530e6ddad752cd46ac5075a39ae73030

haitch commented 2 years ago
  1. for API defined on each resource-provider, yes, it is not versioning tolerant, but you can publish interface per version just as what you do on track 1 sdk.
  2. for Poller, I think publish interface is benefiting both side of us, you would have flexibility to change the implementation, even add more implementation, we can mock it without re-define the sdk method we are invoking.
jhendrixMSFT commented 2 years ago

A Poller[T] interface would still not be versioning tolerant. You can't add a new method to an interface in a non-breaking way (yes you can use non-publicly implementable interfaces but it's a code smell). In addition, adding this extra level of indirection in public surface area, specifically for testing purposes (which not everybody uses), is a bit of an anti-pattern.

We can (and do) change the implementation without the need for a public interface.

serbrech commented 2 years ago

as I said, the only part that felt less test friendly was the poller. but with a little helper, it became quite straight forward. you can provide your own implementation of a poller, so we just wrote a generic FakePollerHandler that we can configure on our fake clients :

p, _ := runtime.NewPoller(nil, runtime.Pipeline{}, &runtime.NewPollerOptions[v20220602preview.FleetMembershipsClientDeleteResponse]{

    Handler: &FakePollerHandler[v20220602preview.FleetMembershipsClientDeleteResponse]{
        Res: f.PollerResponse.BeginDeleteResponse,
        Err: f.PollerResponse.Err,

    },
})
serbrech commented 2 years ago

The gist from @jhendrixMSFT also works by decorating the real client. it's probably simpler than our own setup in fact.

serbrech commented 2 years ago

No matter how you go, the interface changes completely between track1 and track2, so in the scope of a migration from track1 to track2, asking for the interfaces in track 2 does not reduce the work of the migration at all.

you might as well generate the minimal interface you need yourself. That will ensure that you control how they look, and you are still free to put an adapter in between if you want a different api for your service. It also doesn't impose this unnecessary requirement on the SDK

serbrech commented 2 years ago

I do think that the SDK team should spend time writing client tests as an exercise, and documenting how they expect the end user to fake the clients. When I search the SDK repo for how you test the functionality, I found many occurences where your tests would instanciate an internal struct that wouldn't be accessible to me. (the poller for example).

That would force you to think about testability from the end user perspective. Maybe it will drive some API changes, or drive the creation of a test helper package.

jhendrixMSFT commented 2 years ago

We've done this to some degree, and it did end up slightly changing some of the APIs (earlier iterations of the Poller weren't mocking friendly at all).

For mocking, there are various tools floating around can be used to generate interfaces/stubs, so we didn't feel the need to generate them ourselves (there's also the whole "don't overuse mocks" so we wanted to explore other options). I can put together a doc/sample of how this would work.

Internally, we've been using recordings which works well but comes with its own baggage. We also haven't productized our recording framework at present, though there have been some asks for this.

I've also been experimenting with how we could achieve a fake, similar to what GCP offers (example). I'm hacking on a rough prototype to see if it has any merit. The nice thing is that this doesn't require any changes to the client; you just plug in the fake via the client options.

client, err := armresources.NewClient("subID", &fakeTokenCredential{}, &arm.ClientOptions{
    ClientOptions: azcore.ClientOptions{
        Transport: NewFakeARMResourcesServer(&myFakeARMResourcesServer{}), // magic happens here
    },
})

The fakes would likely be codegen'ed but live elsewhere (sub-package, or possibly separate repo?).

haitch commented 2 years ago

fake/mock on transport layer would require us to carry raw request payload.

while we prefer fake/mock on client layer, so we can just mock/fake the object directly.

jhendrixMSFT commented 2 years ago

It doesn't have to, and my current prototype doesn't require raw payloads.

jhendrixMSFT commented 2 years ago

This is my fake server proof-of-concept.

https://gist.github.com/jhendrixMSFT/efd5bcc94f2d545b5cb3a4f5e66446f4

It fakes three APIs from armresources.

serbrech commented 2 years ago

I like this direction @jhendrixMSFT. you provide testability without compromising the API, and the consumer still have all the flexibility they want.

As a consumer, if you're ok generating mocks for your tests, you might as well generate the interfaces too.

prashantrakheja commented 1 year ago

@jhendrixMSFT any tentative timeline when fakes would be available?

jhendrixMSFT commented 1 year ago

We've been performing some internal user-studies on a few different designs. And while I think we're close, the final design hasn't landed yet.

The current prototype, which hasn't incorporated feedback from our last round of internal testing, can be found here if you'd like to see the direction we're going.

Rohitrajak1807 commented 1 year ago

We've been performing some internal user-studies on a few different designs. And while I think we're close, the final design hasn't landed yet.

The current prototype, which hasn't incorporated feedback from our last round of internal testing, can be found here if you'd like to see the direction we're going.

Thanks @jhendrixMSFT !

yevgenypats commented 1 year ago

We've done this to some degree, and it did end up slightly changing some of the APIs (earlier iterations of the Poller weren't mocking friendly at all).

For mocking, there are various tools floating around can be used to generate interfaces/stubs, so we didn't feel the need to generate them ourselves (there's also the whole "don't overuse mocks" so we wanted to explore other options). I can put together a doc/sample of how this would work.

Internally, we've been using recordings which works well but comes with its own baggage. We also haven't productized our recording framework at present, though there have been some asks for this.

I've also been experimenting with how we could achieve a fake, similar to what GCP offers (example). I'm hacking on a rough prototype to see if it has any merit. The nice thing is that this doesn't require any changes to the client; you just plug in the fake via the client options.

client, err := armresources.NewClient("subID", &fakeTokenCredential{}, &arm.ClientOptions{
  ClientOptions: azcore.ClientOptions{
      Transport: NewFakeARMResourcesServer(&myFakeARMResourcesServer{}), // magic happens here
  },
})

The fakes would likely be codegen'ed but live elsewhere (sub-package, or possibly separate repo?).

@jhendrixMSFT This kind of approach will work nicely but you will need to expose another variable to each New*Client or potentially add the option struct to to be able change the host so it will talk to the test httpserver. See how we do this for GCP here - https://github.com/cloudquery/cloudquery/blob/main/plugins/source/gcp/resources/services/compute/addresses_mock_test.go#L43

We are heavy users of mock/unit-test for all the big cloud providers so this will be super valuable to us instead of writing interfaces manually for the Azure SDK.

haitch commented 1 year ago

actually we don't care version tolerant for those interface, we only care if it's mock friendly. (assume you publish interface per version, and I don't think you can easily publish interface regulate all versions)

bump version is a manual process with pull-request anyway, we used to update code along version bump.

jhendrixMSFT commented 1 year ago

Codegen changes https://github.com/Azure/autorest.go/pull/955

unmarshall commented 1 year ago

@jhendrixMSFT any timelines when these fakes will be available for consumption?

jhendrixMSFT commented 1 year ago

We should have a beta out for a select number of Azure Resource Manager modules early next week.

/sdk/resourcemanager/compute/armcompute
/sdk/resourcemanager/containerregisty/armcontainerregistry
/sdk/resourcemanager/containerservice/armcontainerservice
/sdk/resourcemanager/marketplaceordering/armmarketplaceordering
/sdk/resourcemanager/monitor/armmonitor
/sdk/resourcemanager/network/armnetwork
/sdk/resourcemanager/resourcegraph/armresourcegraph
/sdk/resourcemanager/resources/armresources
/sdk/resourcemanager/resources/armsubscriptions
/sdk/resourcemanager/storage/armstorage

Are there any you'd like added to the list?

haitch commented 1 year ago

can you add armcontainerservice as well? we are heavy go sdk user.

discentem commented 1 year ago

@jhendrixMSFT Forgive me as I'm not super familiar with Azure terminology, so this might be out of scope for ARM but: I'd personally love fakes for azblob things, such as blob client, blockblob client, and container client

jhendrixMSFT commented 1 year ago

@discentem for some terminology, Azure is conceptually split into two "planes", management-plane (i.e. ARM) and data-plane. Management plane is for creating/managing resources in Azure (e.g. creating a key vault, storage account, etc), and data-plane is for interacting (for lack of a better word) with those resources (inserting/retrieving items from a key vault, storage account, etc).

The reason we're starting with ARM SDKs is because they are 100% generated by our code generator, so it's simply a matter of regenerating those SDKs with the latest version of our code generator with fakes enabled.

The storage SDKs (azblob, azfile, etc) contain a lot of hand-written code on top of what our code generator produces. So lighting up fakes for them is more involved. That's not to say that they won't happen, we just need to think about how we want to integrate the fakes in those SDKs. Once we get the fakes out for this handful of ARM SDKs, we will start planning on how we want to enable this for the storage SDKs.

discentem commented 1 year ago

Thanks for the detailed reply @jhendrixMSFT!

unmarshall commented 1 year ago

Are there any you'd like added to the list?

@jhendrixMSFT If you can please add the following to fakes that will be great as well:

unmarshall commented 1 year ago

@jhendrixMSFT We have started using fake packages to write unit tests. Thanks for introducing these as it makes testing easier as compared to using Mocks for our use cases. We do see one issue - we have a code flow where we attempt to delete left over NICs and Disks (OS-Disk and Data-Disks) associated to a VM concurrently. All of these are not associated to any VM and thus can be deleted concurrently. When using fakes this errors out. Error stack trace:

execution panicked: runtime error: invalid memory address or nil pointer dereference
        , stack-trace: goroutine 24 [running]:
        runtime/debug.Stack()
            /opt/homebrew/Cellar/go/1.20.5/libexec/src/runtime/debug/stack.go:24 +0x64
        github.com/gardener/machine-controller-manager-provider-azure/pkg/azure/utils.capturePanicAsError({0x140001fd0e0, 0x4c}, 0x106345ba0?)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/pkg/azure/utils/concurrent.go:102 +0x48
        panic({0x105ab9640, 0x106317890})
            /opt/homebrew/Cellar/go/1.20.5/libexec/src/runtime/panic.go:884 +0x204
        github.com/Azure/azure-sdk-for-go/sdk/azcore/fake/internal/exported.(*PollerResponder[...]).More(...)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/fake/internal/exported/fake.go:261
        github.com/Azure/azure-sdk-for-go/sdk/azcore/fake/server.PollerResponderMore[...](...)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/fake/server/server.go:214
        github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/fake.(*DisksServerTransport).dispatchBeginDelete(0x140003b3100, 0x140002b5000?)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/fake/disks_server.go:198 +0x208
        github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/fake.(*DisksServerTransport).Do(0x40?, 0x140002b5000)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/fake/disks_server.go:95 +0x16c
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.transportPolicy.Do({{0x105c1b020?, 0x140003b3100?}}, 0x140001ccb01?)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/pipeline.go:50 +0x38
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0x140003f6080)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:107 +0x100
        github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.bodyDownloadPolicy(0x140003f6080)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_body_download.go:20 +0x20
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.PolicyFunc.Do(0xdc79651ceb92?, 0x14002310130?)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:212 +0x28
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0x140003f6040)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:107 +0x100
        github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*logPolicy).Do(0x140001ab290, 0x140003f6040)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_logging.go:122 +0x344
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0x140003f6000)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:107 +0x100
        github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*httpTracePolicy).Do(0x1400019a7b8, 0x140003f6000)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_http_trace.go:86 +0x684
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0x140003b3fc0)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:107 +0x100
        github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.httpHeaderPolicy(0x140003b3fc0)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_http_header.go:31 +0xb0
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.PolicyFunc.Do(0x105c2a138?, 0x14000192018?)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:212 +0x28
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0x140003b3f80)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:107 +0x100
        github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime.httpTraceNamespacePolicy(0x140003b3f80)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/policy_trace_namespace.go:29 +0x22c
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.PolicyFunc.Do(0x140001cd438?, 0x1052b0c68?)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:212 +0x28
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0x140003b3f40)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:107 +0x100
        github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*BearerTokenPolicy).Do(0x140003b3300, 0x140003e3470?)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_bearer_token.go:83 +0x114
        github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime.(*BearerTokenPolicy).Do(0x0?, 0x0?)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/policy_bearer_token.go:116 +0x20
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0x140001cd718)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:107 +0x100
        github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*retryPolicy).Do(0x14000188928?, 0x140003b3f00)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_retry.go:121 +0x408
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0x140003b3ec0)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:107 +0x100
        github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime.(*rpRegistrationPolicy).Do(0x140002bf680, 0x140003b3ec0)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/policy_register_rp.go:89 +0x80
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0x140003b3e80)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:107 +0x100
        github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.telemetryPolicy.Do({{0x140003ca540?, 0x105b6e8c0?}}, 0x140003b3e80)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_telemetry.go:66 +0x188
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0x140003b3e40)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:107 +0x100
        github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.includeResponsePolicy(0x140003b3e40)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_include_response.go:19 +0x20
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.PolicyFunc.Do(0x14000188ba8?, 0x105298888?)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:212 +0x28
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0x140003b3e00)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go:107 +0x100
        github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.Pipeline.Do({{0x140002b4900?, 0x105c2a1a8?, 0x140003e3440?}}, 0x140003b3e00)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/pipeline.go:96 +0x160
        github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5.(*DisksClient).deleteOperation(0x140001ab2a8, {0x105c2a138?, 0x14000192018?}, {0x140003adfb8, 0x7}, {0x140001b7620, 0x21}, 0x1057c4218?)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/disks_client.go:161 +0x158
        github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5.(*DisksClient).BeginDelete(0x140001ab2a8, {0x105c2a138?, 0x14000192018?}, {0x140003adfb8?, 0x0?}, {0x140001b7620?, 0x0?}, 0x0?)
            /Users/i062009/src/github.com/unmarshall/machine-controller-manager-provider-azure/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/disks_client.go:136 +0x74

This is seen when we attempt to delete 1 NIC, 1 OS-Disk and 1 Data-Disk concurrently. However if i reduce the total disks to 1 (remove the Data Disk) and delete 1 NIC and 1 OS-Disk concurrently then it works. I also saw DiskServerTransport maintains a bunch of PollerResponder which are set to nil here (taking BeginDelete as an example). If we now do concurrent deletes for 2 Disks then we run into this issue.

jhendrixMSFT commented 1 year ago

@unmarshall thanks for the feedback. Indeed we have a problem here when executing faked pager/poller APIs concurrently. I'll take a look at how we can fix this.

jhendrixMSFT commented 1 year ago

@unmarshall we've released updated betas with a fix for this.

unmarshall commented 1 year ago

@jhendrixMSFT I was trying to write tests using fake package for resourcegraph. Since we have more than one queries to test for i was wondering if there is an existing parser for the query string present inside armresourcegraph.QueryRequest. This would then give me a handle to the table and other aspects of the query using which i can then implement the behavior inside the fake server and dish out relevant response. I tried looking at code in this project and also in https://github.com/Azure/azure-kusto-go but could not find anything that given a query string constructs a golang type representing it. Can you provide any hint on how to achieve it?

jhendrixMSFT commented 1 year ago

@unmarshall unfortunately we don't have any such construct as we only generate client-side content.

unmarshall commented 1 year ago

@unmarshall unfortunately we don't have any such construct as we only generate client-side content.

Thanks for your response. With the introduction of fakes, do you think such a thing would be helpful to write tests?

unmarshall commented 1 year ago

@jhendrixMSFT I was writing fake implementation for armmarketplaceorder.MarketplaceAgreementsClient and found its behavior is weird. I have now documented my observations in ticket-1 and ticket-2. The current behavior makes the implementation of a fake server a bit complicated.

unmarshall commented 1 year ago

@jhendrixMSFT Any timeline when fake support will be out of beta?

jhendrixMSFT commented 1 year ago

@unmarshall we've been conducting some user-studies the past few weeks to see where we can make improvements. Those have concluded, so we're working on incorporating the feedback. I'm hoping that this can go GA in our September release but I'm not 100% sure yet.

unmarshall commented 1 year ago

Those have concluded, so we're working on incorporating the feedback. I'm hoping that this can go GA in our September release but I'm not 100% sure yet. @jhendrixMSFT Thanks for a quick update. This sounds encouraging. This tentative timeline works for us.

unmarshall commented 1 year ago

@jhendrixMSFT Any update on the promotion of fakes support to GA?

jhendrixMSFT commented 1 year ago

Yes! I meant to post this earlier here but got sidetracked. We're going to GA fakes in our November release window. So, the first week of November we'll release azcore@v1.9.0 with the fake package along with an updated code generator. The following week we'll begin refreshing all of the ARM modules with fakes enabled. Data-plane will come sometime later after we've had a bit of "bake time" to see if there's any feedback we need to incorporate.

stephengroat commented 1 year ago

thanks so much for this update with general timelines, super helpful for planning the refactor to take advantage of these new features

serbrech commented 1 year ago

Congrats to you and the SDK team on this accomplishment @jhendrixMSFT! We greatly appreciate the transparency and active engagement! 🎉

jhendrixMSFT commented 1 year ago

Just a heads-up to folks using fakes that fake.NewTokenCredential has been removed in prep for the v1.9.0 release. It doesn't add any value over using a literal &fake.TokenCredential{}.

jhendrixMSFT commented 11 months ago

All of the ARM SDKs have been updated with fakes support.

unmarshall commented 11 months ago

@jhendrixMSFT -sdk/azidentity is still in beta (https://github.com/Azure/azure-sdk-for-go/releases/tag/sdk%2Fazidentity%2Fv1.5.0-beta.2) Any plans to move that out of beta? I am assuming beta version of azidentity has fake support in it, right?

jhendrixMSFT commented 11 months ago

@unmarshall we haven't considered adding fakes to azidentity. Its main job is to provide credential types that satisfy the azcore.TokenCredential interface. So, we've added azcore/fake.TokenCredential for that purpose. Can we get more info about your scenario?

sbanic-venafi commented 9 months ago

@jhendrixMSFT : Hi, I noticed that the latest SDK has key vault secrets and keys fake clients (in resourcemanager/keyvault/armkeyvault/fake, but the certificates fake client is not present?

jhendrixMSFT commented 9 months ago

Hmm I don't see a client for certificates in resourcemanager/keyvault/armkeyvault. Can you link to the client in question?

sbanic-venafi commented 9 months ago

So, I may be looking incorrectly. Let's see... I'm looking to fake 3 clients:

Perhaps I'm not implementing this correctly? I'm just starting at this...