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 14, 2024
1 parent 993d79c commit cc763b2
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 62 deletions.
56 changes: 22 additions & 34 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)
require.ErrorIs(t, err, ErrCacheMiss)
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)
require.ErrorIs(t, err, ErrCacheMiss)
// cache miss
_, err = cache.GetRevisionMetadata("my-repo-url", "other-revision")
assert.Equal(t, ErrCacheMiss, err)
require.ErrorIs(t, err, ErrCacheMiss)
// 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)
require.ErrorIs(t, err, ErrCacheMiss)
// 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)
require.ErrorIs(t, err, ErrCacheMiss)
// cache miss
_, err = cache.ListApps("my-repo-url", "other-revision")
assert.Equal(t, ErrCacheMiss, err)
require.ErrorIs(t, err, ErrCacheMiss)
// cache hit
value, err := cache.ListApps("my-repo-url", "my-revision")
require.NoError(t, err)
Expand All @@ -95,34 +95,34 @@ func TestCache_GetManifests(t *testing.T) {
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)
require.ErrorIs(t, err, ErrCacheMiss)
// 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)
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)
require.ErrorIs(t, err, 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)
require.ErrorIs(t, err, 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)
require.ErrorIs(t, err, 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)
require.ErrorIs(t, err, 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)
require.ErrorIs(t, err, 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)
require.ErrorIs(t, err, ErrCacheMiss)
})
t.Run("expect cache hit", func(t *testing.T) {
err = cache.SetManifests(
Expand All @@ -148,16 +148,16 @@ func TestCache_GetAppDetails(t *testing.T) {
value := &apiclient.RepoAppDetailsResponse{}
emptyRefSources := map[string]*RefTarget{}
err := cache.GetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, value, "", nil)
assert.Equal(t, ErrCacheMiss, err)
require.ErrorIs(t, err, ErrCacheMiss)
res := &apiclient.RepoAppDetailsResponse{Type: "my-type"}
err = cache.SetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, res, "", nil)
require.NoError(t, err)
// cache miss
err = cache.GetAppDetails("other-revision", &ApplicationSource{}, emptyRefSources, value, "", nil)
assert.Equal(t, ErrCacheMiss, err)
require.ErrorIs(t, err, ErrCacheMiss)
// cache miss
err = cache.GetAppDetails("my-revision", &ApplicationSource{Path: "other-path"}, emptyRefSources, value, "", nil)
assert.Equal(t, ErrCacheMiss, err)
require.ErrorIs(t, err, ErrCacheMiss)
// cache hit
err = cache.GetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, value, "", nil)
require.NoError(t, err)
Expand Down Expand Up @@ -200,9 +200,7 @@ func TestCachedManifestResponse_HashBehavior(t *testing.T) {
}
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, err)

// Get the cache entry of the set value directly from the in memory cache, and check the values
var cacheKey string
Expand All @@ -221,18 +219,14 @@ 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, err)
assert.Equal(t, retrievedVal, store)

// Corrupt the hash so that it doesn't match
Expand All @@ -244,9 +238,7 @@ func TestCachedManifestResponse_HashBehavior(t *testing.T) {
Key: cacheKey,
Object: &newCmr,
})
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
}

// Retrieve the value using GetManifests and confirm it returns a cache miss
Expand All @@ -263,16 +255,12 @@ func TestCachedManifestResponse_HashBehavior(t *testing.T) {
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 Down
37 changes: 9 additions & 28 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 @@ -4099,7 +4087,6 @@ func TestVerifyCommitSignature(t *testing.T) {
mockGitClient := &gitmocks.Client{}
mockGitClient.On("VerifyCommitSignature", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(testSignature, nil)

err := verifyCommitSignature(true, mockGitClient, "abcd1234", repo)
require.NoError(t, err)
})
Expand All @@ -4109,32 +4096,26 @@ func TestVerifyCommitSignature(t *testing.T) {
mockGitClient := &gitmocks.Client{}
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, err, "revision abcd1234 is not signed")
})

t.Run("VerifyCommitSignature with unknown signature", func(t *testing.T) {
t.Setenv("ARGOCD_GPG_ENABLED", "true")
mockGitClient := &gitmocks.Client{}
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, err, "UNKNOWN signature: gpg: Unknown signature from ABCDEFGH")
})

t.Run("VerifyCommitSignature with error verifying signature", func(t *testing.T) {
t.Setenv("ARGOCD_GPG_ENABLED", "true")
mockGitClient := &gitmocks.Client{}
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, err, "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) {
Expand Down

0 comments on commit cc763b2

Please sign in to comment.