From 68a3ea2e4d75fe51d13d070d55b8d314df642b23 Mon Sep 17 00:00:00 2001 From: Sunny Date: Thu, 4 Nov 2021 02:33:08 +0530 Subject: [PATCH] Add tests for libgit2 remote callbacks - Adds tests for the libgit2 remote callbacks - Adds tests for CheckoutStrategyForImplementation with context timeout and verify timeout is respected by both the git implementations. Signed-off-by: Sunny --- go.mod | 2 +- go.sum | 4 +- pkg/git/libgit2/transport_test.go | 150 ++++++++++++++++++++++++++++++ pkg/git/strategy/strategy_test.go | 97 ++++++++++++++++++- 4 files changed, 248 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 3b89c28a..04c6cb03 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/ProtonMail/go-crypto v0.0.0-20210428141323-04723f9f07d7 github.com/cyphar/filepath-securejoin v0.2.2 github.com/fluxcd/pkg/apis/meta v0.10.0 - github.com/fluxcd/pkg/gittestserver v0.4.1 + github.com/fluxcd/pkg/gittestserver v0.4.2 github.com/fluxcd/pkg/gitutil v0.1.0 github.com/fluxcd/pkg/helmtestserver v0.2.0 github.com/fluxcd/pkg/lockedfile v0.1.0 diff --git a/go.sum b/go.sum index 9ebb6018..4702f7e6 100644 --- a/go.sum +++ b/go.sum @@ -266,8 +266,8 @@ github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fluxcd/pkg/apis/meta v0.10.0 h1:N7wVGHC1cyPdT87hrDC7UwCwRwnZdQM46PBSLjG2rlE= github.com/fluxcd/pkg/apis/meta v0.10.0/go.mod h1:CW9X9ijMTpNe7BwnokiUOrLl/h13miwVr/3abEQLbKE= -github.com/fluxcd/pkg/gittestserver v0.4.1 h1:knghRrVEEPnpO0VJYjoz0H2YMc4fnKAVt5hDGsB1IHc= -github.com/fluxcd/pkg/gittestserver v0.4.1/go.mod h1:hUPx21fe/6oox336Wih/XF1fnmzLmptNMOvATbTZXNY= +github.com/fluxcd/pkg/gittestserver v0.4.2 h1:XqoiemTnnUNldnOw8N7OTdalu2iZp1FTRhp9uUauDJQ= +github.com/fluxcd/pkg/gittestserver v0.4.2/go.mod h1:hUPx21fe/6oox336Wih/XF1fnmzLmptNMOvATbTZXNY= github.com/fluxcd/pkg/gitutil v0.1.0 h1:VO3kJY/CKOCO4ysDNqfdpTg04icAKBOSb3lbR5uE/IE= github.com/fluxcd/pkg/gitutil v0.1.0/go.mod h1:Ybz50Ck5gkcnvF0TagaMwtlRy3X3wXuiri1HVsK5id4= github.com/fluxcd/pkg/helmtestserver v0.2.0 h1:cE7YHDmrWI0hr9QpaaeQ0vQ16Z0IiqZKiINDpqdY610= diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index 15eb6400..4a14b3af 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -18,6 +18,7 @@ package libgit2 import ( "bytes" + "context" "crypto/x509" "encoding/base64" "encoding/pem" @@ -346,6 +347,155 @@ gitlab.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAfuCHKVTjquxvt6CM6tdG4SLp1Btn/nO } } +func Test_transferProgressCallback(t *testing.T) { + tests := []struct { + name string + progress git2go.TransferProgress + cancelFunc func(context.CancelFunc) + wantErr git2go.ErrorCode + }{ + { + name: "ok - in progress", + progress: git2go.TransferProgress{ + TotalObjects: 30, + ReceivedObjects: 21, + }, + cancelFunc: func(cf context.CancelFunc) {}, + wantErr: git2go.ErrorCodeOK, + }, + { + name: "ok - transfer complete", + progress: git2go.TransferProgress{ + TotalObjects: 30, + ReceivedObjects: 30, + }, + cancelFunc: func(cf context.CancelFunc) {}, + wantErr: git2go.ErrorCodeOK, + }, + { + name: "ok - transfer complete, context cancelled", + progress: git2go.TransferProgress{ + TotalObjects: 30, + ReceivedObjects: 30, + }, + cancelFunc: func(cf context.CancelFunc) { cf() }, + wantErr: git2go.ErrorCodeOK, + }, + { + name: "error - context cancelled", + progress: git2go.TransferProgress{ + TotalObjects: 30, + ReceivedObjects: 21, + }, + cancelFunc: func(cf context.CancelFunc) { cf() }, + wantErr: git2go.ErrorCodeUser, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + tpcb := transferProgressCallback(ctx) + + tt.cancelFunc(cancel) + + g.Expect(tpcb(tt.progress)).To(Equal(tt.wantErr)) + }) + } +} + +func Test_transportMessageCallback(t *testing.T) { + tests := []struct { + name string + cancelFunc func(context.CancelFunc) + wantErr git2go.ErrorCode + }{ + { + name: "ok - transport open", + cancelFunc: func(cf context.CancelFunc) {}, + wantErr: git2go.ErrorCodeOK, + }, + { + name: "error - transport closed", + cancelFunc: func(cf context.CancelFunc) { cf() }, + wantErr: git2go.ErrorCodeUser, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + tmcb := transportMessageCallback(ctx) + + tt.cancelFunc(cancel) + + g.Expect(tmcb("")).To(Equal(tt.wantErr)) + }) + } +} + +func Test_pushTransferProgressCallback(t *testing.T) { + type pushProgress struct { + current uint32 + total uint32 + bytes uint + } + tests := []struct { + name string + progress pushProgress + cancelFunc func(context.CancelFunc) + wantErr git2go.ErrorCode + }{ + { + name: "ok - in progress", + progress: pushProgress{current: 20, total: 25}, + cancelFunc: func(cf context.CancelFunc) {}, + wantErr: git2go.ErrorCodeOK, + }, + { + name: "ok - transfer complete", + progress: pushProgress{current: 25, total: 25}, + cancelFunc: func(cf context.CancelFunc) {}, + wantErr: git2go.ErrorCodeOK, + }, + { + name: "ok - transfer complete, context cancelled", + progress: pushProgress{current: 25, total: 25}, + cancelFunc: func(cf context.CancelFunc) { cf() }, + wantErr: git2go.ErrorCodeOK, + }, + { + name: "error - context cancelled", + progress: pushProgress{current: 20, total: 25}, + cancelFunc: func(cf context.CancelFunc) { cf() }, + wantErr: git2go.ErrorCodeUser, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + ptpcb := pushTransferProgressCallback(ctx) + + tt.cancelFunc(cancel) + + g.Expect(ptpcb(tt.progress.current, tt.progress.total, tt.progress.bytes)).To(Equal(tt.wantErr)) + }) + } +} + func md5Fingerprint(in string) [16]byte { var out [16]byte copy(out[:], in) diff --git a/pkg/git/strategy/strategy_test.go b/pkg/git/strategy/strategy_test.go index f88563a9..32f2741a 100644 --- a/pkg/git/strategy/strategy_test.go +++ b/pkg/git/strategy/strategy_test.go @@ -19,6 +19,8 @@ package strategy import ( "context" "errors" + "fmt" + "net/http" "net/url" "os" "path/filepath" @@ -189,7 +191,7 @@ func TestCheckoutStrategyForImplementation_Auth(t *testing.T) { // Run the test cases against the git implementations. for _, gitImpl := range gitImpls { for _, tt := range cases { - t.Run(string(gitImpl)+"_"+tt.name, testFunc(tt, gitImpl)) + t.Run(fmt.Sprintf("%s_%s", gitImpl, tt.name), testFunc(tt, gitImpl)) } } } @@ -351,7 +353,98 @@ func TestCheckoutStrategyForImplementation_SemVerCheckout(t *testing.T) { // Run the test cases against the git implementations. for _, gitImpl := range gitImpls { for _, tt := range tests { - t.Run(string(gitImpl)+"_"+tt.name, testFunc(tt, gitImpl)) + t.Run(fmt.Sprintf("%s_%s", gitImpl, tt.name), testFunc(tt, gitImpl)) + } + } +} + +func TestCheckoutStrategyForImplementation_WithCtxTimeout(t *testing.T) { + gitImpls := []git.Implementation{gogit.Implementation, libgit2.Implementation} + + type testCase struct { + name string + timeout time.Duration + wantErr bool + } + + cases := []testCase{ + { + name: "fails with short timeout", + timeout: 100 * time.Millisecond, + wantErr: true, + }, + { + name: "succeeds with sufficient timeout", + timeout: 5 * time.Second, + wantErr: false, + }, + } + + // Keeping it low to keep the test run time low. + serverDelay := 500 * time.Millisecond + + testFunc := func(tt testCase, impl git.Implementation) func(t *testing.T) { + return func(*testing.T) { + g := NewWithT(t) + + gitServer, err := gittestserver.NewTempGitServer() + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(gitServer.Root()) + username := "test-user" + password := "test-password" + gitServer.Auth(username, password) + gitServer.KeyDir(gitServer.Root()) + + middleware := func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(serverDelay) + next.ServeHTTP(w, r) + }) + } + gitServer.AddHTTPMiddlewares(middleware) + + g.Expect(gitServer.StartHTTP()).ToNot(HaveOccurred()) + defer gitServer.StopHTTP() + + branch := "main" + repoPath := "bar/test-reponame" + err = gitServer.InitRepo("testdata/repo1", branch, repoPath) + g.Expect(err).ToNot(HaveOccurred()) + + repoURL := gitServer.HTTPAddressWithCredentials() + "/" + repoPath + + authOpts := &git.AuthOptions{ + Transport: git.HTTP, + Username: username, + Password: password, + } + + checkoutOpts := git.CheckoutOptions{ + Branch: branch, + } + checkoutStrategy, err := CheckoutStrategyForImplementation(context.TODO(), impl, checkoutOpts) + g.Expect(err).ToNot(HaveOccurred()) + + tmpDir, err := os.MkdirTemp("", "test-checkout") + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(tmpDir) + + checkoutCtx, cancel := context.WithTimeout(context.TODO(), tt.timeout) + defer cancel() + + _, gotErr := checkoutStrategy.Checkout(checkoutCtx, tmpDir, repoURL, authOpts) + if tt.wantErr { + g.Expect(gotErr).To(HaveOccurred()) + } else { + g.Expect(gotErr).ToNot(HaveOccurred()) + } + } + } + + // Run the test cases against the git implementations. + for _, gitImpl := range gitImpls { + for _, tt := range cases { + t.Run(fmt.Sprintf("%s_%s", gitImpl, tt.name), testFunc(tt, gitImpl)) } } }