googleapis / google-cloud-go

Google Cloud Client Libraries for Go.
https://cloud.google.com/go/docs/reference
Apache License 2.0
3.78k stars 1.3k forks source link

compute: List Firewalls - mock response Iterator #11082

Closed patrick-lerch closed 3 days ago

patrick-lerch commented 1 week ago

Client

compute

Environment

N.A.

Code and Dependencies

import (
compute "cloud.google.com/go/compute/apiv1"
)

package main

func TestListingFirewalls() {
  firewallIt := compute.FirewallIterator{} // how can I properly build such an object, so I can make my mock NewFirewallsRESTClient.List return it
}
go.mod ```text module modname go 1.23.0 require ( cloud.google.com/go/compute v1.28.0 ) ```

Expected behavior

I need a way to mock the List response of the NewFirewallsRESTClient

Actual behavior

Mocking not possible

Screenshots

N.A

Additional context

N.a

quartzmo commented 3 days ago

Hi @patrick-lerch,

Sorry for the late response. The fields of compute.FirewallIterator are private, so you can't set them externally. I believe you will need to mock compute.FirewallIterator as well.

patrick-lerch commented 3 days ago

Hey @quartzmo can you maybe provide an example how I can mock the https://pkg.go.dev/cloud.google.com/go/compute/apiv1#FirewallsClient.List call ?

codyoss commented 3 days ago

See https://github.com/googleapis/google-cloud-go/blob/main/testing.md for advice in this space.

patrick-lerch commented 3 days ago

I was aware of this guide already, but I think for the compute service it is not working or at least I was not able to come up with a working solution.

Can you point me to the unimplemented (firewall) service which I could use for this or (best case) provide me with a minimal example how to mock the mentioned LIST call? Last time I checked I didn't find one unimplemented service for the "new" sdk.

The UnimplementedTranslationServiceServer from the guide is stored in the translatepb package, so I was expecting to find something similar for Compute/FirewallList in the computepb package. However I can find anything if I do a string search for "Unimplemented" in the computepb package here

quartzmo commented 3 days ago

The top section in the guide is about gRPC, but the compute client uses HTTP. Check out the next section: https://github.com/googleapis/google-cloud-go/blob/main/testing.md#testing-using-mocks.

patrick-lerch commented 3 days ago

Well I think now we did a full circle.

maybe let me explain a bit better what the actual problem is.

So I was following the guide you have mentioned (https://github.com/googleapis/google-cloud-go/blob/main/testing.md#testing-using-mocks). The interface I have defined looks like this:

// GcpFirewallClient provides an interface around googles firewall client
type GcpFirewallClient interface {
    Delete(ctx context.Context, req *computepb.DeleteFirewallRequest, opts ...gax.CallOption) (*compute.Operation, error)
    List(ctx context.Context, req *computepb.ListFirewallsRequest, opts ...gax.CallOption) *compute.FirewallIterator // It needs to look exactly like this (most importantly I can not change the return type to an interface, otherwise the compute lib will no longer implement this interface
}

Now the unit test looks like this

func TestRemoveFirewallRules(t *testing.T) {
    // Unfortunately there is no way we can test/mock the listing of firewalls.
    // https://github.com/googleapis/google-cloud-go/issues/11082
    ctrl := gomock.NewController(t)
    defer ctrl.Finish()
    mockGcpFirewallClient := mocks.NewMockGcpFirewallClient(ctrl)
    gcpClient := lib.NewGcpClientWith(nil, nil, mockGcpFirewallClient)
    tests := map[string]struct {
        input    string
        mockFn   func()
        expected error
    }{
        "success": {
            input: "test-project",
            mockFn: func() {
                firewallIt := compute.FirewallIterator{} // Problem: I need to create a firewallIterator which is actually filled with something. Otherwise I can not test my production code
                mockGcpFirewallClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(&firewallIt)
            },
            expected: nil,
        },
    }
    for name, test := range tests {
        t.Run(name, func(t *testing.T) {
            test.mockFn()
            got := gcpClient.RemoveFirewallRules(test.input)
            assert.Equal(t, test.expected, got)
        })
    }

}

So basically I am not able to unit test the code inside the for loops of my production coding

// RemoveFirewallRules removes all non-required firewall rules in a given project
func (g gcpClient) RemoveFirewallRules(projectID string) error {
    ctx := context.Background()
    firewallIterator := g.gcpFirewallClient.List(ctx, &computepb.ListFirewallsRequest{Project: projectID})
    firewallEntriesToDelete := []*computepb.Firewall{}
    for {
        firewall, err := firewallIterator.Next()
        if errors.Is(err, iterator.Done) {
            break
        }
        if err != nil {
            return err
        }

        if ShouldFirewallEntryBeDeleted(firewall) {
            firewallEntriesToDelete = append(firewallEntriesToDelete, firewall)
        }
    }
    for _, firewall := range firewallEntriesToDelete {
        _, err := g.gcpFirewallClient.Delete(ctx, &computepb.DeleteFirewallRequest{
            Firewall: firewall.GetName(),
            Project:  projectID,
        })
        if err != nil {
            return err
        }
    }
    return nil
}

Please take another look

codyoss commented 2 days ago

In general with this SDK I would recommend using fakes and standing up a backing HTTP service. If you want to mock things like list rpcs you would first need to put a small facade in front of the types so you can better control the state of the iterator.