Provisioning: remaining unit test coverage github repository (#104382)

* Add test for GitHubRepository_LatestRef

* Add test for GitHubRepository_LatestRef

* Add test for GitHubRepository_CommentPullRequest

* Add remaining tests for GitHubRepository

* Add remaining tests for GitHubRepository

* Fix linting

pkg/registry/apis/provisioning/register.go

@@ -1152,7 +1152,7 @@ func (b *APIBuilder) AsRepository(ctx context.Context, r *provisioning.Repositor
 			return gogit.Clone(ctx, b.clonedir, r, opts, b.secrets)
 		}
 
-		return repository.NewGitHub(ctx, r, b.ghFactory, b.secrets, webhookURL, cloneFn), nil
+		return repository.NewGitHub(ctx, r, b.ghFactory, b.secrets, webhookURL, cloneFn)
 	default:
 		return nil, fmt.Errorf("unknown repository type (%s)", r.Spec.Type)
 	}

pkg/registry/apis/provisioning/repository/clone_fn_mock.go

@@ -0,0 +1,95 @@
+// Code generated by mockery v2.52.4. DO NOT EDIT.
+
+package repository
+
+import (
+	context "context"
+
+	mock "github.com/stretchr/testify/mock"
+)
+
+// MockCloneFn is an autogenerated mock type for the CloneFn type
+type MockCloneFn struct {
+	mock.Mock
+}
+
+type MockCloneFn_Expecter struct {
+	mock *mock.Mock
+}
+
+func (_m *MockCloneFn) EXPECT() *MockCloneFn_Expecter {
+	return &MockCloneFn_Expecter{mock: &_m.Mock}
+}
+
+// Execute provides a mock function with given fields: ctx, opts
+func (_m *MockCloneFn) Execute(ctx context.Context, opts CloneOptions) (ClonedRepository, error) {
+	ret := _m.Called(ctx, opts)
+
+	if len(ret) == 0 {
+		panic("no return value specified for Execute")
+	}
+
+	var r0 ClonedRepository
+	var r1 error
+	if rf, ok := ret.Get(0).(func(context.Context, CloneOptions) (ClonedRepository, error)); ok {
+		return rf(ctx, opts)
+	}
+	if rf, ok := ret.Get(0).(func(context.Context, CloneOptions) ClonedRepository); ok {
+		r0 = rf(ctx, opts)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(ClonedRepository)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func(context.Context, CloneOptions) error); ok {
+		r1 = rf(ctx, opts)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// MockCloneFn_Execute_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Execute'
+type MockCloneFn_Execute_Call struct {
+	*mock.Call
+}
+
+// Execute is a helper method to define mock.On call
+//   - ctx context.Context
+//   - opts CloneOptions
+func (_e *MockCloneFn_Expecter) Execute(ctx interface{}, opts interface{}) *MockCloneFn_Execute_Call {
+	return &MockCloneFn_Execute_Call{Call: _e.mock.On("Execute", ctx, opts)}
+}
+
+func (_c *MockCloneFn_Execute_Call) Run(run func(ctx context.Context, opts CloneOptions)) *MockCloneFn_Execute_Call {
+	_c.Call.Run(func(args mock.Arguments) {
+		run(args[0].(context.Context), args[1].(CloneOptions))
+	})
+	return _c
+}
+
+func (_c *MockCloneFn_Execute_Call) Return(_a0 ClonedRepository, _a1 error) *MockCloneFn_Execute_Call {
+	_c.Call.Return(_a0, _a1)
+	return _c
+}
+
+func (_c *MockCloneFn_Execute_Call) RunAndReturn(run func(context.Context, CloneOptions) (ClonedRepository, error)) *MockCloneFn_Execute_Call {
+	_c.Call.Return(run)
+	return _c
+}
+
+// NewMockCloneFn creates a new instance of MockCloneFn. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
+// The first argument is typically a *testing.T value.
+func NewMockCloneFn(t interface {
+	mock.TestingT
+	Cleanup(func())
+}) *MockCloneFn {
+	mock := &MockCloneFn{}
+	mock.Mock.Test(t)
+
+	t.Cleanup(func() { mock.AssertExpectations(t) })
+
+	return mock
+}

pkg/registry/apis/provisioning/repository/github.go

@@ -56,15 +56,21 @@ func NewGitHub(
 	secrets secrets.Service,
 	webhookURL string,
 	cloneFn CloneFn,
-) *githubRepository {
-	owner, repo, _ := parseOwnerRepo(config.Spec.GitHub.URL)
+) (*githubRepository, error) {
+	owner, repo, err := parseOwnerRepo(config.Spec.GitHub.URL)
+	if err != nil {
+		return nil, fmt.Errorf("parse owner and repo: %w", err)
+	}
+
 	token := config.Spec.GitHub.Token
 	if token == "" {
 		decrypted, err := secrets.Decrypt(ctx, config.Spec.GitHub.EncryptedToken)
-		if err == nil {
-			token = string(decrypted)
+		if err != nil {
+			return nil, fmt.Errorf("decrypt token: %w", err)
 		}
+		token = string(decrypted)
 	}
+
 	return &githubRepository{
 		config:     config,
 		gh:         factory.New(ctx, token), // TODO, baseURL from config
@@ -73,7 +79,7 @@ func NewGitHub(
 		owner:      owner,
 		repo:       repo,
 		cloneFn:    cloneFn,
-	}
+	}, nil
 }
 
 func (r *githubRepository) Config() *provisioning.Repository {
@@ -705,8 +711,8 @@ func (r *githubRepository) CompareFiles(ctx context.Context, base, ref string) (
 				})
 			case previousErr == nil && currentErr != nil:
 				changes = append(changes, VersionedFileChange{
-					Path:   currentPath,
-					Ref:    ref,
+					Path:   previousPath,
+					Ref:    base,
 					Action: FileActionDeleted,
 				})
 			case previousErr != nil && currentErr == nil:
@@ -826,7 +832,7 @@ func (r *githubRepository) updateWebhook(ctx context.Context) (pgh.WebhookConfig
 
 	var mustUpdate bool
 
-	if hook.URL != r.config.Status.Webhook.URL {
+	if hook.URL != r.webhookURL {
 		mustUpdate = true
 		hook.URL = r.webhookURL
 	}

pkg/registry/apis/provisioning/repository/repository.go

@@ -53,6 +53,7 @@ type FileInfo struct {
 	Modified *metav1.Time
 }
 
+//go:generate mockery --name CloneFn --structname MockCloneFn --inpackage --filename clone_fn_mock.go --with-expecter
 type CloneFn func(ctx context.Context, opts CloneOptions) (ClonedRepository, error)
 
 type CloneOptions struct {