4

I use the following function, and I need to raise the coverage of it (if possible to 100%), the problem is that typically I use interface to handle such cases in Go and for this specific case not sure how to do it, as this is a bit more tricky, any idea?

The package https://pkg.go.dev/google.golang.org/genproto/googleapis/cloud/compute/v1

Which I use doesn't have interface so not sure how can I mock it?

import (
    "context"
    "errors"
    "fmt"
    "os"

    compute "cloud.google.com/go/compute/apiv1"
    "google.golang.org/api/iterator"
    "google.golang.org/api/option"
    computev1 "google.golang.org/genproto/googleapis/cloud/compute/v1"
)

func Res(ctx context.Context, project string, region string,vpc string,secret string) error {

    c, err := compute.NewAddressesRESTClient(ctx, option.WithCredentialsFile(secret))

    if err != nil {
        return err
    }

    defer c.Close()
    addrReq := &computev1.ListAddressesRequest{
        Project: project,
        Region:  region,
    }
    it := c.List(ctx, addrReq)

    for {
        resp, err := it.Next()
        if err == iterator.Done {
            break
        }
        if err != nil {
            return err
        }
        if *(resp.Status) != "IN_USE" {
            return ipConverter(*resp.Name, vpc)
        }
    }
    return nil
}
13
  • You can adapt the ideas laid out in stackoverflow.com/questions/71659292/…
    – jub0bs
    Apr 28, 2022 at 16:39
  • @jub0bs - thanks, but not sure how it's related as here I didnt find interface to mock, can you please provide the same way for my contexts ?
    – JME
    May 1, 2022 at 19:21
  • 1
    @jub0bs - no it doesnt change during life time of the program , however diffrent invocation can have diff secret . does it answer your question ?
    – JME
    May 2, 2022 at 10:57
  • 1
    Yes, it does. That's going to simplify things :)
    – jub0bs
    May 2, 2022 at 12:38
  • 1
    I'll have more time to post an answer later today :)
    – jub0bs
    May 3, 2022 at 6:36

2 Answers 2

3

Whenever I find myself in this scenario, I found that the easiest solution is to create missing interfaces myself. I limit these interfaces to the types and functions that I am using, instead of writing interfaces for the entire library. Then, in my code, instead of accepting third-party concrete types, I accept my interfaces for those types. Then I use gomock to generate mocks for these interfaces as usual.

The following is a descriptive example inspired by your code.

type RestClient interface {
    List(context.Context, *computev1.ListAddressesRequest) (ListResult, error) // assuming List returns ListResult type.
    Close() error
}

func newRestClient(ctx context.Context, secret string) (RestClient, error) {
    return compute.NewAddressesRESTClient(ctx, option.WithCredentialsFile(secret))
}

func Res(ctx context.Context, project string, region string, vpc string, secret string) error {
    c, err := newRestClient(ctx, secret)
    if err != nil {
        return err
    }

    defer c.Close()
    return res(ctx, project, region, vpc, c)
}

func res(ctx context.Context, project string, region string, vpc string, c RestClient) error {
    addrReq := &computev1.ListAddressesRequest{
        Project: project,
        Region:  region,
    }

    it, err := c.List(ctx, addrReq)
    if err != nil {
        return err
    }

    for {
        resp, err := it.Next()
        if err == iterator.Done {
            break
        }

        if err != nil {
            return err
        }

        if *(resp.Status) != "IN_USE" {
            return ipConverter(*resp.Name, vpc)
        }
    }

    return nil
}

Now you can test the important bits of the Res function by injecting a mock RestClient to the internal res function.

8
  • Close, but you can't test the exported API (Res) that way.
    – jub0bs
    May 2, 2022 at 8:33
  • @jub0bs and why would I want to test a piece of code that is so simple that its test case would need modification every time it changes? martinfowler.com/bliki/TestCoverage.html
    – ashu
    May 2, 2022 at 8:38
  • 1
    I simply favour black-box testing (i.e. testing stuff through its public API), that's all.
    – jub0bs
    May 2, 2022 at 8:56
  • 1
    The problem here isn't the testing approach, it's the way the code is written. The function under test creates its dependencies, rather than accepting them from the caller. You can move that part around, but the dependency instantiation part is simply untestable (in most cases including this one).
    – ashu
    May 2, 2022 at 9:13
  • Thanks, how can I run it e2e with the test, can you please provide the full example?
    – JME
    May 4, 2022 at 6:06
2
+100

One obstacle to testability here is that you instantiate a client inside your Res function rather than injecting it. Because

  • the secret doesn't change during the lifetime of the programme,
  • the methods of *compute.AddressesClient (other than Close) are concurrency-safe,

you could create one client and reuse it for each invocation or Res. To inject it into Res, you can declare some Compute type and turn Res into a method on that type:

type Compute struct {
  Lister Lister // some appropriate interface type
}

func (cp *Compute) Res(ctx context.Context, project, region, vpc string) error {
addrReq := &computev1.ListAddressesRequest{
        Project: project,
        Region:  region,
    }
    it := cp.Lister.List(ctx, addrReq)
    for {
        resp, err := it.Next()
        if err == iterator.Done {
            break
        }
        if err != nil {
            return err
        }
        if *(resp.Status) != "IN_USE" {
            return ipConverter(*resp.Name, vpc)
        }
    }
    return nil
}

One question remains: how should you declare Lister? One possibility is

type Lister interface {
    List(ctx context.Context, req *computev1.ListAddressesRequest, opts ...gax.CallOption) *compute.AddressIterator
}

However, because compute.AddressIterator is a struct type with some unexported fields and for which package compute provides no factory function, you can't easily control how the iterator returned from List behaves in your tests. One way out is to declare an additional interface,

type Iterator interface {
    Next() (*computev1.Address, error)
}

and change the result type of List from *compute.AddressIterator to Iterator:

type Lister interface {
    List(ctx context.Context, req *computev1.ListAddressesRequest, opts ...gax.CallOption) Iterator
}

Then you can declare another struct type for the real Lister and use that on the production side:

type RealLister struct {
    Client *compute.AddressesClient
}

func (rl *RealLister) List(ctx context.Context, req *computev1.ListAddressesRequest, opts ...gax.CallOption) Iterator {
    return rl.Client.List(ctx, req, opts...)
}

func main() {
    secret := "don't hardcode me"
    ctx, cancel := context.WithCancel(context.Background()) // for instance
    defer cancel()
    c, err := compute.NewAddressesRESTClient(ctx, option.WithCredentialsFile(secret))
    if err != nil {
        log.Fatal(err) // or deal with the error in some way
    }
    defer c.Close()
    cp := Compute{Lister: &RealLister{Client: c}}
    if err := cp.Res(ctx, "my-project", "us-east-1", "my-vpc"); err != nil {
        log.Fatal(err) // or deal with the error in some way
    }
}

For your tests, you can declare another struct type that will act as a configurable test double:

type FakeLister func(ctx context.Context, req *computev1.ListAddressesRequest, opts ...gax.CallOption) Iterator

func (fl FakeLister) List(ctx context.Context, req *computev1.ListAddressesRequest, opts ...gax.CallOption) Iterator {
    return fl(ctx, req, opts...)
}

To control the behaviour of the Iterator in your test, you can declare another configurable concrete type:

type FakeIterator struct{
    Err error
    Status string
}

func (fi *FakeIterator) Next() (*computev1.Address, error) {
    addr := computev1.Address{Status: &fi.Status}
    return &addr, fi.Err
}

A test function may look like this:

func TestResStatusInUse(t *testing.T) {
    // Arrange
    l := func(_ context.Context, _ *computev1.ListAddressesRequest, _ ...gax.CallOption) Iterator {
        return &FakeIterator{
            Status: "IN_USE",
            Err:    nil,
        }
    }
    cp := Compute{Lister: FakeLister(l)}
    dummyCtx := context.Background()
    // Act
    err := cp.Res(dummyCtx, "my-project", "us-east-1", "my-vpc")
    // Assert
    if err != nil {
        // ...
    }
}
6
  • thanks, im trying to debug it to learn from this example, the code doesnt compile , the error is for cp := Compute{Lister: &RealLister{Client: c}} error is Cannot use '&RealLister{Client: c}' (type *RealLister) as the type ListerType does not implement 'Lister'need the method: List(ctx context.Context, req *computev1.ListAddressesRequest, opts ...gax.CallOption) *compute.AddressIteratorhave the method: List(ctx context.Context, req *computev1.ListAddressesRequest, opts ...gax.CallOption) Iterator
    – JME
    May 4, 2022 at 14:15
  • i've changed it to func (rl *RealLister) List(ctx context.Context, req *computev1.ListAddressesRequest, opts ...gax.CallOption) *compute.AddressIterator {
    – JME
    May 4, 2022 at 14:30
  • it would be great if you will provide the full files, the main and the test.
    – JME
    May 4, 2022 at 14:40
  • This is the function test could you show please how to mock and check it? this is the only thing that im struggling with , go.dev/play/p/tNNPsS_PKBI
    – JME
    May 4, 2022 at 14:58
  • @JDC I'll update my answer later. In the meantime, check out this repo: github.com/jub0bs/so-question-72046805
    – jub0bs
    May 5, 2022 at 8:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.