Merge branch 'refactor/client-mockgen' into 'master'

test: migrate client stub to mockgen

Closes #276

See merge request gitlab-org/gitlab-pages!623

5 files changed, 222 insertions, 140 deletions

diff --git a/Makefile.build.mk b/Makefile.build.mk
index 3202b0de..562c9b86 100644
--- a/Makefile.build.mk
+++ b/Makefile.build.mk
@@ -17,6 +17,7 @@ cisetup: .GOPATH/.ok
 generate-mocks: .GOPATH/.ok
 	$Q bin/mockgen -source=internal/interface.go -destination=internal/mocks/mocks.go -package=mocks
 	$Q bin/mockgen -source=internal/source/source.go -destination=internal/mocks/source.go -package=mocks
+	$Q bin/mockgen -source=internal/mocks/api/client_stub.go -destination=internal/mocks/client.go -package=mocks
 
 build: .GOPATH/.ok
 	$Q GOBIN=$(CURDIR)/bin go install $(if $V,-v) $(VERSION_FLAGS) -tags "${GO_BUILD_TAGS}" -buildmode exe $(IMPORT_PATH)
diff --git a/internal/mocks/api/client_stub.go b/internal/mocks/api/client_stub.go
new file mode 100644
index 00000000..f1a20754
--- /dev/null
+++ b/internal/mocks/api/client_stub.go
@@ -0,0 +1,8 @@
+package mocks
+
+import "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
+
+type ClientStub interface {
+	api.Client
+	api.Resolver
+}
diff --git a/internal/mocks/client.go b/internal/mocks/client.go
new file mode 100644
index 00000000..22b0757e
--- /dev/null
+++ b/internal/mocks/client.go
@@ -0,0 +1,64 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: internal/mocks/api/client_stub.go
+
+// Package mocks is a generated GoMock package.
+package mocks
+
+import (
+	context "context"
+	reflect "reflect"
+
+	gomock "github.com/golang/mock/gomock"
+	api "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
+)
+
+// MockClientStub is a mock of ClientStub interface.
+type MockClientStub struct {
+	ctrl     *gomock.Controller
+	recorder *MockClientStubMockRecorder
+}
+
+// MockClientStubMockRecorder is the mock recorder for MockClientStub.
+type MockClientStubMockRecorder struct {
+	mock *MockClientStub
+}
+
+// NewMockClientStub creates a new mock instance.
+func NewMockClientStub(ctrl *gomock.Controller) *MockClientStub {
+	mock := &MockClientStub{ctrl: ctrl}
+	mock.recorder = &MockClientStubMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockClientStub) EXPECT() *MockClientStubMockRecorder {
+	return m.recorder
+}
+
+// GetLookup mocks base method.
+func (m *MockClientStub) GetLookup(ctx context.Context, domain string) api.Lookup {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "GetLookup", ctx, domain)
+	ret0, _ := ret[0].(api.Lookup)
+	return ret0
+}
+
+// GetLookup indicates an expected call of GetLookup.
+func (mr *MockClientStubMockRecorder) GetLookup(ctx, domain interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLookup", reflect.TypeOf((*MockClientStub)(nil).GetLookup), ctx, domain)
+}
+
+// Resolve mocks base method.
+func (m *MockClientStub) Resolve(ctx context.Context, domain string) *api.Lookup {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Resolve", ctx, domain)
+	ret0, _ := ret[0].(*api.Lookup)
+	return ret0
+}
+
+// Resolve indicates an expected call of Resolve.
+func (mr *MockClientStubMockRecorder) Resolve(ctx, domain interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Resolve", reflect.TypeOf((*MockClientStub)(nil).Resolve), ctx, domain)
+}
diff --git a/internal/source/gitlab/client/client_stub.go b/internal/source/gitlab/client/client_stub.go
deleted file mode 100644
index 2cd54f10..00000000
--- a/internal/source/gitlab/client/client_stub.go
+++ /dev/null
@@ -1,47 +0,0 @@
-package client
-
-import (
-	"context"
-	"encoding/json"
-	"os"
-
-	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
-)
-
-// StubClient is a stubbed client used for testing
-type StubClient struct {
-	File      string
-	StatusErr func() error
-	Lookup    *api.Lookup
-}
-
-// Resolve implements api.Resolver
-func (c StubClient) Resolve(ctx context.Context, host string) *api.Lookup {
-	if c.Lookup != nil {
-		return c.Lookup
-	}
-
-	lookup := c.GetLookup(ctx, host)
-
-	return &lookup
-}
-
-// GetLookup reads a test fixture and unmarshalls it
-func (c StubClient) GetLookup(ctx context.Context, host string) api.Lookup {
-	lookup := api.Lookup{Name: host}
-
-	f, err := os.Open(c.File)
-	if err != nil {
-		lookup.Error = err
-		return lookup
-	}
-	defer f.Close()
-
-	lookup.Error = json.NewDecoder(f).Decode(&lookup.Domain)
-
-	return lookup
-}
-
-func (c StubClient) Status() error {
-	return c.StatusErr()
-}
diff --git a/internal/source/gitlab/gitlab_test.go b/internal/source/gitlab/gitlab_test.go
index d1ef2a95..d7fbf454 100644
--- a/internal/source/gitlab/gitlab_test.go
+++ b/internal/source/gitlab/gitlab_test.go
@@ -2,119 +2,133 @@ package gitlab
 
 import (
 	"context"
+	"encoding/json"
+	"io"
+	"net/http"
 	"net/http/httptest"
+	"os"
 	"testing"
 
+	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
 
+	"gitlab.com/gitlab-org/gitlab-pages/internal/mocks"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/client"
 )
 
 func TestGetDomain(t *testing.T) {
-	t.Run("when the response if correct", func(t *testing.T) {
-		client := client.StubClient{File: "client/testdata/test.gitlab.io.json"}
-		source := Gitlab{client: client}
-
-		domain, err := source.GetDomain(context.Background(), "test.gitlab.io")
-		require.NoError(t, err)
-
-		require.Equal(t, "test.gitlab.io", domain.Name)
-	})
+	tests := map[string]struct {
+		file          string
+		domain        string
+		mockLookup    *api.Lookup
+		expectedError error
+	}{
+		"when the response is correct": {
+			file:   "client/testdata/test.gitlab.io.json",
+			domain: "test.gitlab.io",
+		},
+		"when the response is not valid": {
+			file:          "/dev/null",
+			domain:        "test.gitlab.io",
+			expectedError: io.EOF,
+		},
+		"when the response is unauthorized": {
+			mockLookup:    &api.Lookup{Error: client.ErrUnauthorizedAPI},
+			domain:        "test",
+			expectedError: client.ErrUnauthorizedAPI,
+		},
+	}
 
-	t.Run("when the response is not valid", func(t *testing.T) {
-		client := client.StubClient{File: "/dev/null"}
-		source := Gitlab{client: client}
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			mockClient := NewMockClient(t, tc.file, tc.mockLookup)
+			source := Gitlab{client: mockClient}
+
+			domain, err := source.GetDomain(context.Background(), tc.domain)
+			if tc.expectedError == nil {
+				require.NoError(t, err)
+				require.Equal(t, tc.domain, domain.Name)
+			} else {
+				require.Error(t, err)
+				require.Nil(t, domain)
+			}
+		})
+	}
+}
 
-		domain, err := source.GetDomain(context.Background(), "test.gitlab.io")
+func TestResolve(t *testing.T) {
+	tests := map[string]struct {
+		file                string
+		target              string
+		expectedPrefix      string
+		expectedPath        string
+		expectedSubPath     string
+		expectedIsNamespace bool
+	}{
+		"when requesting nested group project with root path": {
+			file:                "client/testdata/test.gitlab.io.json",
+			target:              "https://test.gitlab.io:443/my/pages/project/",
+			expectedPrefix:      "/my/pages/project/",
+			expectedPath:        "some/path/to/project/",
+			expectedSubPath:     "",
+			expectedIsNamespace: false,
+		},
+		"when requesting a nested group project with full path": {
+			file:                "client/testdata/test.gitlab.io.json",
+			target:              "https://test.gitlab.io:443/my/pages/project/path/index.html",
+			expectedPrefix:      "/my/pages/project/",
+			expectedPath:        "some/path/to/project/",
+			expectedSubPath:     "path/index.html",
+			expectedIsNamespace: false,
+		},
+		"when requesting the group root project with root path": {
+			file:                "client/testdata/test.gitlab.io.json",
+			target:              "https://test.gitlab.io:443/",
+			expectedPrefix:      "/",
+			expectedPath:        "some/path/to/project-3/",
+			expectedSubPath:     "",
+			expectedIsNamespace: true,
+		},
+		"when requesting the group root project with full path": {
+			file:                "client/testdata/test.gitlab.io.json",
+			target:              "https://test.gitlab.io:443/path/to/index.html",
+			expectedPrefix:      "/",
+			expectedPath:        "some/path/to/project-3/",
+			expectedSubPath:     "path/to/index.html",
+			expectedIsNamespace: true,
+		},
+		"when request path has not been sanitized": {
+			file:            "client/testdata/test.gitlab.io.json",
+			target:          "https://test.gitlab.io:443/something/../something/../my/pages/project/index.html",
+			expectedPrefix:  "/my/pages/project/",
+			expectedPath:    "some/path/to/project/",
+			expectedSubPath: "index.html",
+		},
+	}
 
-		require.NotNil(t, err)
-		require.Nil(t, domain)
-	})
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			mockClient := NewMockClient(t, tc.file, nil)
+			source := Gitlab{client: mockClient, enableDisk: true}
 
-	t.Run("when pages endpoint is unauthorized", func(t *testing.T) {
-		c := client.StubClient{Lookup: &api.Lookup{Error: client.ErrUnauthorizedAPI}}
-		source := Gitlab{client: c}
+			request := httptest.NewRequest(http.MethodGet, tc.target, nil)
 
-		_, err := source.GetDomain(context.Background(), "test")
-		require.EqualError(t, err, client.ErrUnauthorizedAPI.Error())
-	})
-}
+			response, err := source.Resolve(request)
+			require.NoError(t, err)
 
-func TestResolve(t *testing.T) {
-	client := client.StubClient{File: "client/testdata/test.gitlab.io.json"}
-	source := Gitlab{client: client, enableDisk: true}
-
-	t.Run("when requesting nested group project with root path", func(t *testing.T) {
-		target := "https://test.gitlab.io:443/my/pages/project/"
-		request := httptest.NewRequest("GET", target, nil)
-
-		response, err := source.Resolve(request)
-		require.NoError(t, err)
-
-		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
-		require.Equal(t, "some/path/to/project/", response.LookupPath.Path)
-		require.Equal(t, "", response.SubPath)
-		require.False(t, response.LookupPath.IsNamespaceProject)
-	})
-
-	t.Run("when requesting a nested group project with full path", func(t *testing.T) {
-		target := "https://test.gitlab.io:443/my/pages/project/path/index.html"
-		request := httptest.NewRequest("GET", target, nil)
-
-		response, err := source.Resolve(request)
-		require.NoError(t, err)
-
-		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
-		require.Equal(t, "some/path/to/project/", response.LookupPath.Path)
-		require.Equal(t, "path/index.html", response.SubPath)
-		require.False(t, response.LookupPath.IsNamespaceProject)
-	})
-
-	t.Run("when requesting the group root project with root path", func(t *testing.T) {
-		target := "https://test.gitlab.io:443/"
-		request := httptest.NewRequest("GET", target, nil)
-
-		response, err := source.Resolve(request)
-		require.NoError(t, err)
-
-		require.Equal(t, "/", response.LookupPath.Prefix)
-		require.Equal(t, "some/path/to/project-3/", response.LookupPath.Path)
-		require.Equal(t, "", response.SubPath)
-		require.True(t, response.LookupPath.IsNamespaceProject)
-	})
-
-	t.Run("when requesting the group root project with full path", func(t *testing.T) {
-		target := "https://test.gitlab.io:443/path/to/index.html"
-		request := httptest.NewRequest("GET", target, nil)
-
-		response, err := source.Resolve(request)
-		require.NoError(t, err)
-
-		require.Equal(t, "/", response.LookupPath.Prefix)
-		require.Equal(t, "path/to/index.html", response.SubPath)
-		require.Equal(t, "some/path/to/project-3/", response.LookupPath.Path)
-		require.True(t, response.LookupPath.IsNamespaceProject)
-	})
-
-	t.Run("when request path has not been sanitized", func(t *testing.T) {
-		target := "https://test.gitlab.io:443/something/../something/../my/pages/project/index.html"
-		request := httptest.NewRequest("GET", target, nil)
-
-		response, err := source.Resolve(request)
-		require.NoError(t, err)
-
-		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
-		require.Equal(t, "index.html", response.SubPath)
-	})
+			require.Equal(t, tc.expectedPrefix, response.LookupPath.Prefix)
+			require.Equal(t, tc.expectedPath, response.LookupPath.Path)
+			require.Equal(t, tc.expectedSubPath, response.SubPath)
+			require.Equal(t, tc.expectedIsNamespace, response.LookupPath.IsNamespaceProject)
+		})
+	}
 }
 
 // Test proves fix for https://gitlab.com/gitlab-org/gitlab-pages/-/issues/576
 func TestResolveLookupPathsOrderDoesNotMatter(t *testing.T) {
-	client := client.StubClient{File: "client/testdata/group-first.gitlab.io.json"}
-	source := Gitlab{client: client, enableDisk: true}
-
 	tests := map[string]struct {
+		file                string
 		target              string
 		expectedPrefix      string
 		expectedPath        string
@@ -122,6 +136,7 @@ func TestResolveLookupPathsOrderDoesNotMatter(t *testing.T) {
 		expectedIsNamespace bool
 	}{
 		"when requesting the group root project with root path": {
+			file:                "client/testdata/group-first.gitlab.io.json",
 			target:              "https://group-first.gitlab.io:443/",
 			expectedPrefix:      "/",
 			expectedPath:        "some/path/group/",
@@ -129,6 +144,7 @@ func TestResolveLookupPathsOrderDoesNotMatter(t *testing.T) {
 			expectedIsNamespace: true,
 		},
 		"when requesting another project with path": {
+			file:                "client/testdata/group-first.gitlab.io.json",
 			target:              "https://group-first.gitlab.io:443/my/second-project/index.html",
 			expectedPrefix:      "/my/second-project/",
 			expectedPath:        "some/path/to/project-2/",
@@ -139,7 +155,10 @@ func TestResolveLookupPathsOrderDoesNotMatter(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			request := httptest.NewRequest("GET", test.target, nil)
+			mockClient := NewMockClient(t, test.file, nil)
+			source := Gitlab{client: mockClient, enableDisk: true}
+
+			request := httptest.NewRequest(http.MethodGet, test.target, nil)
 
 			response, err := source.Resolve(request)
 			require.NoError(t, err)
@@ -151,3 +170,40 @@ func TestResolveLookupPathsOrderDoesNotMatter(t *testing.T) {
 		})
 	}
 }
+
+func NewMockClient(t *testing.T, file string, mockedLookup *api.Lookup) *mocks.MockClientStub {
+	mockCtrl := gomock.NewController(t)
+
+	mockClient := mocks.NewMockClientStub(mockCtrl)
+	mockClient.EXPECT().
+		Resolve(gomock.Any(), gomock.Any()).
+		DoAndReturn(func(ctx context.Context, domain string) *api.Lookup {
+			lookup := mockClient.GetLookup(ctx, domain)
+			return &lookup
+		}).
+		Times(1)
+
+	mockClient.EXPECT().
+		GetLookup(gomock.Any(), gomock.Any()).
+		DoAndReturn(func(ctx context.Context, domain string) api.Lookup {
+			if mockedLookup != nil {
+				return *mockedLookup
+			}
+
+			lookup := api.Lookup{Name: domain}
+
+			f, err := os.Open(file)
+			if err != nil {
+				lookup.Error = err
+				return lookup
+			}
+			defer f.Close()
+
+			lookup.Error = json.NewDecoder(f).Decode(&lookup.Domain)
+
+			return lookup
+		}).
+		Times(1)
+
+	return mockClient
+}