Skip to content

Commit

Permalink
chore: use testify instead of testing.Fatal or testing.Error in repos…
Browse files Browse the repository at this point in the history 
…erver

Signed-off-by: Matthieu MOREL <[email protected]>
  • Loading branch information
@mmorel-35
mmorel-35 committed Nov 12, 2024
1 parent 993d79c commit 47e5cba
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 100 deletions.
103 changes: 34 additions & 69 deletions reposerver/cache/cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,17 @@ func TestCache_GetRevisionMetadata(t *testing.T) {
mockCache := fixtures.mockCache
// cache miss
_, err := cache.GetRevisionMetadata("my-repo-url", "my-revision")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, err, ErrCacheMiss)

Check failure on line 47 in reposerver/cache/cache_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code 

require-error: for error assertions use require (testifylint)
mockCache.RedisClient.AssertCalled(t, "Get", mock.Anything, mock.Anything)
// populate cache
err = cache.SetRevisionMetadata("my-repo-url", "my-revision", &RevisionMetadata{Message: "my-message"})
require.NoError(t, err)
// cache miss
_, err = cache.GetRevisionMetadata("other-repo-url", "my-revision")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, err, ErrCacheMiss)

Check failure on line 54 in reposerver/cache/cache_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code 

require-error: for error assertions use require (testifylint)
// cache miss
_, err = cache.GetRevisionMetadata("my-repo-url", "other-revision")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, err, ErrCacheMiss)

Check failure on line 57 in reposerver/cache/cache_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code 

require-error: for error assertions use require (testifylint)
// cache hit
value, err := cache.GetRevisionMetadata("my-repo-url", "my-revision")
require.NoError(t, err)
Expand All @@ -69,16 +69,16 @@ func TestCache_ListApps(t *testing.T) {
mockCache := fixtures.mockCache
// cache miss
_, err := cache.ListApps("my-repo-url", "my-revision")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, err, ErrCacheMiss)

Check failure on line 72 in reposerver/cache/cache_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code 

require-error: for error assertions use require (testifylint)
// populate cache
err = cache.SetApps("my-repo-url", "my-revision", map[string]string{"foo": "bar"})
require.NoError(t, err)
// cache miss
_, err = cache.ListApps("other-repo-url", "my-revision")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, err, ErrCacheMiss)
// cache miss
_, err = cache.ListApps("my-repo-url", "other-revision")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, err, ErrCacheMiss)
// cache hit
value, err := cache.ListApps("my-repo-url", "my-revision")
require.NoError(t, err)
Expand All @@ -94,44 +94,34 @@ func TestCache_GetManifests(t *testing.T) {
// cache miss
q := &apiclient.ManifestRequest{}
value := &CachedManifestResponse{}
err := cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil, "")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil, ""), ErrCacheMiss)

Check failure on line 97 in reposerver/cache/cache_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code 

require-error: for error assertions use require (testifylint)
// populate cache
res := &CachedManifestResponse{ManifestResponse: &apiclient.ManifestResponse{SourceType: "my-source-type"}}
err = cache.SetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", res, nil, "")
require.NoError(t, err)
require.NoError(t, cache.SetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", res, nil, ""))
t.Run("expect cache miss because of changed revision", func(t *testing.T) {
err = cache.GetManifests("other-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil, "")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, cache.GetManifests("other-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil, ""), ErrCacheMiss)
})
t.Run("expect cache miss because of changed path", func(t *testing.T) {
err = cache.GetManifests("my-revision", &ApplicationSource{Path: "other-path"}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil, "")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, cache.GetManifests("my-revision", &ApplicationSource{Path: "other-path"}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil, ""), ErrCacheMiss)
})
t.Run("expect cache miss because of changed namespace", func(t *testing.T) {
err = cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "other-namespace", "", "my-app-label-key", "my-app-label-value", value, nil, "")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "other-namespace", "", "my-app-label-key", "my-app-label-value", value, nil, ""), ErrCacheMiss)
})
t.Run("expect cache miss because of changed app label key", func(t *testing.T) {
err = cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "other-app-label-key", "my-app-label-value", value, nil, "")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "other-app-label-key", "my-app-label-value", value, nil, ""), ErrCacheMiss)
})
t.Run("expect cache miss because of changed app label value", func(t *testing.T) {
err = cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "other-app-label-value", value, nil, "")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "other-app-label-value", value, nil, ""), ErrCacheMiss)
})
t.Run("expect cache miss because of changed referenced source", func(t *testing.T) {
err = cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "other-app-label-value", value, map[string]string{"my-referenced-source": "my-referenced-revision"}, "")
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "other-app-label-value", value, map[string]string{"my-referenced-source": "my-referenced-revision"}, ""), ErrCacheMiss)
})
t.Run("expect cache hit", func(t *testing.T) {
err = cache.SetManifests(
require.NoError(t, cache.SetManifests(
"my-revision1", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value",
&CachedManifestResponse{ManifestResponse: &apiclient.ManifestResponse{SourceType: "my-source-type", Revision: "my-revision2"}}, nil, "")
require.NoError(t, err)
&CachedManifestResponse{ManifestResponse: &apiclient.ManifestResponse{SourceType: "my-source-type", Revision: "my-revision2"}}, nil, ""))

err = cache.GetManifests("my-revision1", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil, "")
require.NoError(t, err)
require.NoError(t, cache.GetManifests("my-revision1", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil, ""))

assert.Equal(t, "my-source-type", value.ManifestResponse.SourceType)
assert.Equal(t, "my-revision1", value.ManifestResponse.Revision)
Expand All @@ -147,20 +137,15 @@ func TestCache_GetAppDetails(t *testing.T) {
// cache miss
value := &apiclient.RepoAppDetailsResponse{}
emptyRefSources := map[string]*RefTarget{}
err := cache.GetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, value, "", nil)
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, cache.GetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, value, "", nil), ErrCacheMiss)

Check failure on line 140 in reposerver/cache/cache_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code 

require-error: for error assertions use require (testifylint)
res := &apiclient.RepoAppDetailsResponse{Type: "my-type"}
err = cache.SetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, res, "", nil)
require.NoError(t, err)
require.NoError(t, cache.SetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, res, "", nil))
// cache miss
err = cache.GetAppDetails("other-revision", &ApplicationSource{}, emptyRefSources, value, "", nil)
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, cache.GetAppDetails("other-revision", &ApplicationSource{}, emptyRefSources, value, "", nil), ErrCacheMiss)

Check failure on line 144 in reposerver/cache/cache_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code 

require-error: for error assertions use require (testifylint)
// cache miss
err = cache.GetAppDetails("my-revision", &ApplicationSource{Path: "other-path"}, emptyRefSources, value, "", nil)
assert.Equal(t, ErrCacheMiss, err)
assert.ErrorIs(t, cache.GetAppDetails("my-revision", &ApplicationSource{Path: "other-path"}, emptyRefSources, value, "", nil), ErrCacheMiss)

Check failure on line 146 in reposerver/cache/cache_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code 

require-error: for error assertions use require (testifylint)
// cache hit
err = cache.GetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, value, "", nil)
require.NoError(t, err)
require.NoError(t, cache.GetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, value, "", nil))
assert.Equal(t, &apiclient.RepoAppDetailsResponse{Type: "my-type"}, value)
mockCache.AssertCacheCalledTimes(t, &mocks.CacheCallCounts{ExternalSets: 1, ExternalGets: 4})
}
Expand Down Expand Up @@ -199,10 +184,7 @@ func TestCachedManifestResponse_HashBehavior(t *testing.T) {
NumberOfConsecutiveFailures: 0,
}
q := &apiclient.ManifestRequest{}
err := repoCache.SetManifests(response.Revision, appSrc, q.RefSources, q, response.Namespace, "", appKey, appValue, store, nil, "")
if err != nil {
t.Fatal(err)
}
require.NoError(t, repoCache.SetManifests(response.Revision, appSrc, q.RefSources, q, response.Namespace, "", appKey, appValue, store, nil, ""))

// Get the cache entry of the set value directly from the in memory cache, and check the values
var cacheKey string
Expand All @@ -221,58 +203,43 @@ func TestCachedManifestResponse_HashBehavior(t *testing.T) {
assert.Equal(t, cmr.ManifestResponse, store.ManifestResponse)

regeneratedHash, err := cmr.generateCacheEntryHash()
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
assert.Equal(t, cmr.CacheEntryHash, regeneratedHash)
}

// Retrieve the value using 'GetManifests' and confirm it works
retrievedVal := &CachedManifestResponse{}
err = repoCache.GetManifests(response.Revision, appSrc, q.RefSources, q, response.Namespace, "", appKey, appValue, retrievedVal, nil, "")
if err != nil {
t.Fatal(err)
}
require.NoError(t, repoCache.GetManifests(response.Revision, appSrc, q.RefSources, q, response.Namespace, "", appKey, appValue, retrievedVal, nil, ""))
assert.Equal(t, retrievedVal, store)

// Corrupt the hash so that it doesn't match
{
newCmr := cmr.shallowCopy()
newCmr.CacheEntryHash = "!bad-hash!"

err := inMemCache.Set(&cacheutil.Item{
require.NoError(t, inMemCache.Set(&cacheutil.Item{
Key: cacheKey,
Object: &newCmr,
})
if err != nil {
t.Fatal(err)
}
}))
}

// Retrieve the value using GetManifests and confirm it returns a cache miss
retrievedVal = &CachedManifestResponse{}
err = repoCache.GetManifests(response.Revision, appSrc, q.RefSources, q, response.Namespace, "", appKey, appValue, retrievedVal, nil, "")

assert.Equal(t, err, cacheutil.ErrCacheMiss)
assert.ErrorIs(t, repoCache.GetManifests(response.Revision, appSrc, q.RefSources, q, response.Namespace, "", appKey, appValue, retrievedVal, nil, ""), cacheutil.ErrCacheMiss)

Check failure on line 228 in reposerver/cache/cache_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code 

require-error: for error assertions use require (testifylint)

// Verify that the hash mismatch item has been deleted
items := getInMemoryCacheContents(t, inMemCache)
assert.Empty(t, items)
assert.Empty(t, getInMemoryCacheContents(t, inMemCache))
}

func getInMemoryCacheContents(t *testing.T, inMemCache *cacheutil.InMemoryCache) map[string]*CachedManifestResponse {
t.Helper()
items, err := inMemCache.Items(func() interface{} { return &CachedManifestResponse{} })
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)

result := map[string]*CachedManifestResponse{}
for key, val := range items {
obj, ok := val.(*CachedManifestResponse)
if !ok {
t.Fatal(errors.New("Unexpected type in cache"))
}
require.True(t, ok, "Unexpected type in cache")

result[key] = obj
}
Expand All @@ -292,10 +259,9 @@ func TestCachedManifestResponse_ShallowCopy(t *testing.T) {
NumberOfConsecutiveFailures: 3,
}

post := pre.shallowCopy()
assert.Equal(t, pre, post)
assert.Equal(t, pre, pre.shallowCopy())

unequal := &CachedManifestResponse{
assert.NotEqual(t, pre, &CachedManifestResponse{

Check failure on line 264 in reposerver/cache/cache_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code 

expected-actual: need to reverse actual and expected values (testifylint)
CacheEntryHash: "diff-value",
FirstFailureTimestamp: 1,
ManifestResponse: &apiclient.ManifestResponse{
Expand All @@ -304,8 +270,7 @@ func TestCachedManifestResponse_ShallowCopy(t *testing.T) {
MostRecentError: "error",
NumberOfCachedResponsesReturned: 2,
NumberOfConsecutiveFailures: 3,
}
assert.NotEqual(t, pre, unequal)
})
}

func TestCachedManifestResponse_ShallowCopyExpectedFields(t *testing.T) {
Expand Down
42 changes: 11 additions & 31 deletions reposerver/repository/repository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -429,9 +429,7 @@ func TestGenerateManifestsHelmWithRefs_CachedNoLsRemote(t *testing.T) {
}
return err
})
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
})
service := NewService(metrics.NewMetricsServer(), cacheMocks.cache, RepoServerInitConstants{ParallelismLimit: 1}, argo.NewResourceTracking(), &git.NoopCredsStore{}, repopath)
var gitClient git.Client
Expand Down Expand Up @@ -2196,15 +2194,10 @@ func TestGenerateManifestWithAnnotatedAndRegularGitTagHashes(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
manifestResponse, err := tt.service.GenerateManifest(tt.ctx, tt.manifestRequest)
if !tt.wantError {
if err == nil {
assert.Equal(t, manifestResponse.Revision, actualCommitSHA)
} else {
t.Errorf("unexpected error")
}
require.NoError(t, err)
assert.Equal(t, manifestResponse.Revision, actualCommitSHA)
} else {
if err == nil {
t.Errorf("expected an error but did not throw one")
}
assert.Errorf(t, err, "expected an error but did not throw one")
}
})
}
Expand Down Expand Up @@ -2239,13 +2232,8 @@ func TestGenerateManifestWithAnnotatedTagsAndMultiSourceApp(t *testing.T) {
}

response, err := service.GenerateManifest(context.Background(), manifestRequest)
if err != nil {
t.Errorf("unexpected %s", err)
}

if response.Revision != annotatedGitTaghash {
t.Errorf("returned SHA %s is different from expected annotated tag %s", response.Revision, annotatedGitTaghash)
}
require.NoError(t, err)
assert.Equalf(t, response.Revision, annotatedGitTaghash, "returned SHA %s is different from expected annotated tag %s", response.Revision, annotatedGitTaghash)
}

func TestGenerateMultiSourceHelmWithFileParameter(t *testing.T) {
Expand Down Expand Up @@ -4100,8 +4088,7 @@ func TestVerifyCommitSignature(t *testing.T) {
mockGitClient.On("VerifyCommitSignature", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(testSignature, nil)

err := verifyCommitSignature(true, mockGitClient, "abcd1234", repo)
require.NoError(t, err)
require.NoError(t, verifyCommitSignature(true, mockGitClient, "abcd1234", repo))
})

t.Run("VerifyCommitSignature with invalid signature", func(t *testing.T) {
Expand All @@ -4110,9 +4097,7 @@ func TestVerifyCommitSignature(t *testing.T) {
mockGitClient.On("VerifyCommitSignature", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return("", nil)

err := verifyCommitSignature(true, mockGitClient, "abcd1234", repo)
require.Error(t, err)
assert.Equal(t, "revision abcd1234 is not signed", err.Error())
assert.EqualError(t, verifyCommitSignature(true, mockGitClient, "abcd1234", repo), "revision abcd1234 is not signed")
})

t.Run("VerifyCommitSignature with unknown signature", func(t *testing.T) {
Expand All @@ -4121,9 +4106,7 @@ func TestVerifyCommitSignature(t *testing.T) {
mockGitClient.On("VerifyCommitSignature", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return("", fmt.Errorf("UNKNOWN signature: gpg: Unknown signature from ABCDEFGH"))

err := verifyCommitSignature(true, mockGitClient, "abcd1234", repo)
require.Error(t, err)
assert.Equal(t, "UNKNOWN signature: gpg: Unknown signature from ABCDEFGH", err.Error())
assert.EqualError(t, verifyCommitSignature(true, mockGitClient, "abcd1234", repo), "UNKNOWN signature: gpg: Unknown signature from ABCDEFGH")
})

t.Run("VerifyCommitSignature with error verifying signature", func(t *testing.T) {
Expand All @@ -4132,16 +4115,13 @@ func TestVerifyCommitSignature(t *testing.T) {
mockGitClient.On("VerifyCommitSignature", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return("", fmt.Errorf("error verifying signature of commit 'abcd1234' in repo 'https://github.com/example/repo.git': failed to verify signature"))

err := verifyCommitSignature(true, mockGitClient, "abcd1234", repo)
require.Error(t, err)
assert.Equal(t, "error verifying signature of commit 'abcd1234' in repo 'https://github.com/example/repo.git': failed to verify signature", err.Error())
assert.EqualError(t, verifyCommitSignature(true, mockGitClient, "abcd1234", repo), "error verifying signature of commit 'abcd1234' in repo 'https://github.com/example/repo.git': failed to verify signature")
})

t.Run("VerifyCommitSignature with signature verification disabled", func(t *testing.T) {
t.Setenv("ARGOCD_GPG_ENABLED", "false")
mockGitClient := &gitmocks.Client{}
err := verifyCommitSignature(false, mockGitClient, "abcd1234", repo)
require.NoError(t, err)
require.NoError(t, verifyCommitSignature(false, mockGitClient, "abcd1234", repo))
})
}

Expand Down

0 comments on commit 47e5cba

Please sign in to comment.