Improve tests and simplify cache retriever type

6 files changed, 49 insertions, 51 deletions

diff --git a/internal/source/gitlab/cache/cache_test.go b/internal/source/gitlab/cache/cache_test.go
index 75b673e0..7abcb09d 100644
--- a/internal/source/gitlab/cache/cache_test.go
+++ b/internal/source/gitlab/cache/cache_test.go
@@ -8,7 +8,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
 )
@@ -103,9 +103,9 @@ func TestResolve(t *testing.T) {
 
 			lookup := cache.Resolve(context.Background(), "my.gitlab.com")
 
-			assert.NoError(t, lookup.Error)
-			assert.Equal(t, "my.gitlab.com", lookup.Name)
-			assert.Equal(t, uint64(1), resolver.lookups)
+			require.NoError(t, lookup.Error)
+			require.Equal(t, "my.gitlab.com", lookup.Name)
+			require.Equal(t, uint64(1), resolver.lookups)
 		})
 	})
 
@@ -124,12 +124,12 @@ func TestResolve(t *testing.T) {
 			go receiver()
 			go receiver()
 
-			assert.Equal(t, uint64(0), resolver.lookups)
+			require.Equal(t, uint64(0), resolver.lookups)
 
 			resolver.domain <- "my.gitlab.com"
 			wg.Wait()
 
-			assert.Equal(t, uint64(1), resolver.lookups)
+			require.Equal(t, uint64(1), resolver.lookups)
 		})
 	})
 
@@ -138,8 +138,8 @@ func TestResolve(t *testing.T) {
 			cache.withTestEntry(entryConfig{expired: false, retrieved: true}, func(*Entry) {
 				lookup := cache.Resolve(context.Background(), "my.gitlab.com")
 
-				assert.Equal(t, "my.gitlab.com", lookup.Name)
-				assert.Equal(t, uint64(0), resolver.lookups)
+				require.Equal(t, "my.gitlab.com", lookup.Name)
+				require.Equal(t, uint64(0), resolver.lookups)
 			})
 		})
 	})
@@ -155,14 +155,14 @@ func TestResolve(t *testing.T) {
 
 				<-resolver.bootup
 
-				assert.Equal(t, uint64(1), resolver.started)
-				assert.Equal(t, uint64(0), resolver.lookups)
+				require.Equal(t, uint64(1), resolver.started)
+				require.Equal(t, uint64(0), resolver.lookups)
 
 				resolver.domain <- "my.gitlab.com"
 				<-lookup
 
-				assert.Equal(t, uint64(1), resolver.started)
-				assert.Equal(t, uint64(1), resolver.lookups)
+				require.Equal(t, uint64(1), resolver.started)
+				require.Equal(t, uint64(1), resolver.lookups)
 			})
 		})
 	})
@@ -172,11 +172,11 @@ func TestResolve(t *testing.T) {
 			cache.withTestEntry(entryConfig{expired: true, retrieved: true}, func(*Entry) {
 				lookup := cache.Resolve(context.Background(), "my.gitlab.com")
 
-				assert.Equal(t, "my.gitlab.com", lookup.Name)
-				assert.Equal(t, uint64(0), resolver.lookups)
+				require.Equal(t, "my.gitlab.com", lookup.Name)
+				require.Equal(t, uint64(0), resolver.lookups)
 
 				resolver.domain <- "my.gitlab.com"
-				assert.Equal(t, uint64(1), resolver.lookups)
+				require.Equal(t, uint64(1), resolver.lookups)
 			})
 		})
 	})
@@ -188,10 +188,10 @@ func TestResolve(t *testing.T) {
 				cache.Resolve(context.Background(), "my.gitlab.com")
 				cache.Resolve(context.Background(), "my.gitlab.com")
 
-				assert.Equal(t, uint64(0), resolver.lookups)
+				require.Equal(t, uint64(0), resolver.lookups)
 
 				resolver.domain <- "my.gitlab.com"
-				assert.Equal(t, uint64(1), resolver.lookups)
+				require.Equal(t, uint64(1), resolver.lookups)
 			})
 		})
 	})
@@ -202,8 +202,8 @@ func TestResolve(t *testing.T) {
 
 			lookup := cache.Resolve(context.Background(), "my.gitlab.com")
 
-			assert.Equal(t, uint64(3), resolver.lookups)
-			assert.EqualError(t, lookup.Error, "500 err")
+			require.Equal(t, uint64(3), resolver.lookups)
+			require.EqualError(t, lookup.Error, "500 err")
 		})
 	})
 
@@ -214,8 +214,8 @@ func TestResolve(t *testing.T) {
 		withTestCache(resolverConfig{}, func(cache *Cache, resolver *client) {
 			lookup := cache.Resolve(ctx, "my.gitlab.com")
 
-			assert.Equal(t, uint64(0), resolver.lookups)
-			assert.EqualError(t, lookup.Error, "context done")
+			require.Equal(t, uint64(0), resolver.lookups)
+			require.EqualError(t, lookup.Error, "context done")
 		})
 	})
 
@@ -226,8 +226,8 @@ func TestResolve(t *testing.T) {
 		withTestCache(resolverConfig{}, func(cache *Cache, resolver *client) {
 			lookup := cache.Resolve(context.Background(), "my.gitlab.com")
 
-			assert.Equal(t, uint64(0), resolver.lookups)
-			assert.EqualError(t, lookup.Error, "context done")
+			require.Equal(t, uint64(0), resolver.lookups)
+			require.EqualError(t, lookup.Error, "context done")
 		})
 	})
 
@@ -244,7 +244,7 @@ func TestResolve(t *testing.T) {
 				resolver.domain <- "my.gitlab.com"
 				lookup := <-response
 
-				assert.EqualError(t, lookup.Error, "context done")
+				require.EqualError(t, lookup.Error, "context done")
 			})
 		})
 	})
diff --git a/internal/source/gitlab/cache/const.go b/internal/source/gitlab/cache/const.go
index 5b11dc23..90c5bddc 100644
--- a/internal/source/gitlab/cache/const.go
+++ b/internal/source/gitlab/cache/const.go
@@ -6,6 +6,6 @@ var (
 	shortCacheExpiry     = 30 * time.Second
 	longCacheExpiry      = 10 * time.Minute
 	retrievalTimeout     = 5 * time.Second
-	maxRetrievalRetries  = 3
 	maxRetrievalInterval = time.Second
+	maxRetrievalRetries  = 3
 )
diff --git a/internal/source/gitlab/cache/entry.go b/internal/source/gitlab/cache/entry.go
index 9d81cc85..1a40e203 100644
--- a/internal/source/gitlab/cache/entry.go
+++ b/internal/source/gitlab/cache/entry.go
@@ -90,7 +90,7 @@ func (e *Entry) Refresh(client api.Client, store Store) {
 }
 
 func (e *Entry) retrieveWithClient(client api.Client) {
-	retriever := Retriever{client: client, timeout: retrievalTimeout}
+	retriever := Retriever{client: client}
 
 	e.setResponse(retriever.Retrieve(e.domain))
 }
diff --git a/internal/source/gitlab/cache/entry_test.go b/internal/source/gitlab/cache/entry_test.go
index ffa0d195..118981d0 100644
--- a/internal/source/gitlab/cache/entry_test.go
+++ b/internal/source/gitlab/cache/entry_test.go
@@ -4,7 +4,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
 )
@@ -14,8 +14,8 @@ func TestIsUpToDateAndNeedsRefresh(t *testing.T) {
 		entry := newCacheEntry("my.gitlab.com")
 		entry.response = &api.Lookup{}
 
-		assert.True(t, entry.IsUpToDate())
-		assert.False(t, entry.NeedsRefresh())
+		require.True(t, entry.IsUpToDate())
+		require.False(t, entry.NeedsRefresh())
 	})
 
 	t.Run("when is resolved and is expired", func(t *testing.T) {
@@ -23,22 +23,22 @@ func TestIsUpToDateAndNeedsRefresh(t *testing.T) {
 		entry.response = &api.Lookup{}
 		entry.created = time.Now().Add(-time.Hour)
 
-		assert.False(t, entry.IsUpToDate())
-		assert.True(t, entry.NeedsRefresh())
+		require.False(t, entry.IsUpToDate())
+		require.True(t, entry.NeedsRefresh())
 	})
 
 	t.Run("when is not resolved and not expired", func(t *testing.T) {
 		entry := newCacheEntry("my.gitlab.com")
 
-		assert.False(t, entry.IsUpToDate())
-		assert.False(t, entry.NeedsRefresh())
+		require.False(t, entry.IsUpToDate())
+		require.False(t, entry.NeedsRefresh())
 	})
 
 	t.Run("when is not resolved and is expired", func(t *testing.T) {
 		entry := newCacheEntry("my.gitlab.com")
 		entry.created = time.Now().Add(-time.Hour)
 
-		assert.False(t, entry.IsUpToDate())
-		assert.False(t, entry.NeedsRefresh())
+		require.False(t, entry.IsUpToDate())
+		require.False(t, entry.NeedsRefresh())
 	})
 }
diff --git a/internal/source/gitlab/cache/retriever.go b/internal/source/gitlab/cache/retriever.go
index 2db2eca8..1284b021 100644
--- a/internal/source/gitlab/cache/retriever.go
+++ b/internal/source/gitlab/cache/retriever.go
@@ -12,14 +12,13 @@ import (
 // Retriever is an utility type that performs an HTTP request with backoff in
 // case of errors
 type Retriever struct {
-	client  api.Client
-	timeout time.Duration
+	client api.Client
 }
 
 // Retrieve retrieves a lookup response from external source with timeout and
 // backoff. It has its own context with timeout.
 func (r *Retriever) Retrieve(domain string) api.Lookup {
-	ctx, cancel := context.WithTimeout(context.Background(), r.timeout)
+	ctx, cancel := context.WithTimeout(context.Background(), retrievalTimeout)
 	defer cancel()
 
 	var lookup api.Lookup
diff --git a/internal/source/gitlab/gitlab_test.go b/internal/source/gitlab/gitlab_test.go
index 02751eea..ad5144f5 100644
--- a/internal/source/gitlab/gitlab_test.go
+++ b/internal/source/gitlab/gitlab_test.go
@@ -4,7 +4,6 @@ import (
 	"net/http/httptest"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/cache"
@@ -19,7 +18,7 @@ func TestGetDomain(t *testing.T) {
 		domain, err := source.GetDomain("test.gitlab.io")
 		require.NoError(t, err)
 
-		assert.Equal(t, "test.gitlab.io", domain.Name)
+		require.Equal(t, "test.gitlab.io", domain.Name)
 	})
 
 	t.Run("when the response is not valid", func(t *testing.T) {
@@ -28,8 +27,8 @@ func TestGetDomain(t *testing.T) {
 
 		domain, err := source.GetDomain("test.gitlab.io")
 
-		assert.NotNil(t, err)
-		assert.Nil(t, domain)
+		require.NotNil(t, err)
+		require.Nil(t, domain)
 	})
 }
 
@@ -44,9 +43,9 @@ func TestResolve(t *testing.T) {
 		lookup, subpath, err := source.Resolve(request)
 		require.NoError(t, err)
 
-		assert.Equal(t, "/my/pages/project", lookup.Prefix)
-		assert.Equal(t, "path/index.html", subpath)
-		assert.False(t, lookup.IsNamespaceProject)
+		require.Equal(t, "/my/pages/project", lookup.Prefix)
+		require.Equal(t, "path/index.html", subpath)
+		require.False(t, lookup.IsNamespaceProject)
 	})
 
 	t.Run("when request a nested group project", func(t *testing.T) {
@@ -56,10 +55,10 @@ func TestResolve(t *testing.T) {
 		lookup, subpath, err := source.Resolve(request)
 		require.NoError(t, err)
 
-		assert.Equal(t, "/", lookup.Prefix)
-		assert.Equal(t, "path/to/index.html", subpath)
-		assert.Equal(t, "some/path/to/project-3/", lookup.Path)
-		assert.True(t, lookup.IsNamespaceProject)
+		require.Equal(t, "/", lookup.Prefix)
+		require.Equal(t, "path/to/index.html", subpath)
+		require.Equal(t, "some/path/to/project-3/", lookup.Path)
+		require.True(t, lookup.IsNamespaceProject)
 	})
 
 	t.Run("when request path has not been sanitized", func(t *testing.T) {
@@ -69,7 +68,7 @@ func TestResolve(t *testing.T) {
 		lookup, subpath, err := source.Resolve(request)
 		require.NoError(t, err)
 
-		assert.Equal(t, "/my/pages/project", lookup.Prefix)
-		assert.Equal(t, "index.html", subpath)
+		require.Equal(t, "/my/pages/project", lookup.Prefix)
+		require.Equal(t, "index.html", subpath)
 	})
 }