maxbrunsfeld / counterfeiter

A tool for generating self-contained, type-safe test doubles in go
MIT License
979 stars 93 forks source link

Internal type reference #290

Closed elgohr closed 2 months ago

elgohr commented 3 months ago

When using counterfeiter in context with a type that references an internal type, the internal type is resolved - which can't be used. For example azcore.TokenCredential in https://github.com/Azure/azure-sdk-for-go/blob/main/sdk/azcore/core.go#L24 references exported.TokenCredential. exported.TokenCredential is defined in an internal package. Counterfeiter resolves exported.TokenCredential instead of azcore.TokenCredential.

joefitzgerald commented 2 months ago

Hey @elgohr. That sounds frustrating! Do you have an example interface I could use as a test case / reproduction, ideally in a public repo?

elgohr commented 2 months ago

Hey @joefitzgerald , the example from Azure is a public repository. But I could try to create a test

elgohr commented 2 months ago

Talking about internal package (https://go.dev/doc/go1.4#internalpackages) not internal repository

joefitzgerald commented 2 months ago

We actually have a test in the test suite that validates we work with aliased types correctly: https://github.com/maxbrunsfeld/counterfeiter/blob/main/fixtures/internalpkg/aliased_interface.go#L8.

For the TokenCredential example you provided, the issue is that the interface method includes an argument that is an internal type and a return type that is also internal. In both cases, this requires an import of the internal package in which those types are defined, which is invalid because it is an internal package outside your package.

type TokenCredential interface {
    // GetToken requests an access token for the specified set of scopes.
    GetToken(ctx context.Context, options TokenRequestOptions) (AccessToken, error)
}

While the type alias for TokenCredential makes the interface accessible outside the package, the options TokenRequestOptions is actually options internal.TokenRequestOptions as seen by a caller outside the internal package, and the AccessToken return type is actually internal.AccessToken.

Unfortunately, given the design of this interface, it cannot be faked, even if you were not using Counterfeiter and were doing it manually. The general solution is to define your own interface that includes the method(s) you depend on from the Azure SDK. You can then ensure that it uses the exported types:

Therefore, you could build your own interface and then generate a fake with counterfeiter:

package yourpackagehere

import (
    "github.com/Azure/azure-sdk-for-go/sdk/azcore"
    "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
)

//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate
//counterfeiter:generate . TokenCredential

// TokenCredential represents a credential capable of providing an OAuth token.
// Exported as azcore.TokenCredential.
type TokenCredential interface {
    // GetToken requests an access token for the specified set of scopes.
    GetToken(ctx context.Context, options policy.TokenRequestOptions) (azcore.AccessToken, error)
}

This will generate a fake that is satisfied by the various types in the Azure SDK that implement GetToken, like this.

As a general rule you should define interfaces in your package that describe the behavior you depend on from other packages. Depending on someone else's interface is suboptimal, as you've found out here. Hopefully this explanation helps!

elgohr commented 2 months ago

Thank you for the extensive explanation! Totally missed the internal argument types. I tried to use a composite interface that wraps the external interface (also for maintenance) within my package. I guess here I don't come around defining it as suggested.

joefitzgerald commented 2 months ago

I just made a tweak to my suggested interface definition. I had the package name for TokenRequestOptions and AccessToken reversed.

elgohr commented 2 months ago

@joefitzgerald when I try to generate your suggestion, types from azcore and policy are resolved to the internal types...

joefitzgerald commented 2 months ago

Yes, I think what you’re now experiencing is the same as #174. I need to take a look at some of the changes in go 1.23 to see if they can help with that issue.