Provisioning: unit test repository/github package (#104310)

* Add unit tests IsAuthenticated

* Add unit tests RepoExists

* Add unit tests GetContents

* Add initial unit tests GetTree

* Add unit tests for CreateFile

* Add unit test UpdateFile

* Add unit tests DeleteFile

* Add unit tests for Commits

* Add unit tests for helpers

* Add unit test CompareCommits

* Add GetBranch tests

* Add unit tests BranchExists and CreateBranch

* Add unit tests Webhooks

* Remove unused code

* Add unit tests CommentPullRequest

* Add more cases for GetTree

* Complete coverage

* Fix linting
This commit is contained in:
Roberto Jiménez Sánchez
2025-04-22 18:20:29 +01:00
committed by GitHub
parent 8ef8471b23
commit a438b192d3
8 changed files with 3877 additions and 279 deletions
@@ -24,53 +24,6 @@ func (_m *MockPullRequestRepo) EXPECT() *MockPullRequestRepo_Expecter {
return &MockPullRequestRepo_Expecter{mock: &_m.Mock}
}
// ClearAllPullRequestFileComments provides a mock function with given fields: ctx, pr
func (_m *MockPullRequestRepo) ClearAllPullRequestFileComments(ctx context.Context, pr int) error {
ret := _m.Called(ctx, pr)
if len(ret) == 0 {
panic("no return value specified for ClearAllPullRequestFileComments")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, int) error); ok {
r0 = rf(ctx, pr)
} else {
r0 = ret.Error(0)
}
return r0
}
// MockPullRequestRepo_ClearAllPullRequestFileComments_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ClearAllPullRequestFileComments'
type MockPullRequestRepo_ClearAllPullRequestFileComments_Call struct {
*mock.Call
}
// ClearAllPullRequestFileComments is a helper method to define mock.On call
// - ctx context.Context
// - pr int
func (_e *MockPullRequestRepo_Expecter) ClearAllPullRequestFileComments(ctx interface{}, pr interface{}) *MockPullRequestRepo_ClearAllPullRequestFileComments_Call {
return &MockPullRequestRepo_ClearAllPullRequestFileComments_Call{Call: _e.mock.On("ClearAllPullRequestFileComments", ctx, pr)}
}
func (_c *MockPullRequestRepo_ClearAllPullRequestFileComments_Call) Run(run func(ctx context.Context, pr int)) *MockPullRequestRepo_ClearAllPullRequestFileComments_Call {
_c.Call.Run(func(args mock.Arguments) {
run(args[0].(context.Context), args[1].(int))
})
return _c
}
func (_c *MockPullRequestRepo_ClearAllPullRequestFileComments_Call) Return(_a0 error) *MockPullRequestRepo_ClearAllPullRequestFileComments_Call {
_c.Call.Return(_a0)
return _c
}
func (_c *MockPullRequestRepo_ClearAllPullRequestFileComments_Call) RunAndReturn(run func(context.Context, int) error) *MockPullRequestRepo_ClearAllPullRequestFileComments_Call {
_c.Call.Return(run)
return _c
}
// CommentPullRequest provides a mock function with given fields: ctx, pr, comment
func (_m *MockPullRequestRepo) CommentPullRequest(ctx context.Context, pr int, comment string) error {
ret := _m.Called(ctx, pr, comment)
@@ -119,56 +72,6 @@ func (_c *MockPullRequestRepo_CommentPullRequest_Call) RunAndReturn(run func(con
return _c
}
// CommentPullRequestFile provides a mock function with given fields: ctx, pr, path, ref, comment
func (_m *MockPullRequestRepo) CommentPullRequestFile(ctx context.Context, pr int, path string, ref string, comment string) error {
ret := _m.Called(ctx, pr, path, ref, comment)
if len(ret) == 0 {
panic("no return value specified for CommentPullRequestFile")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, int, string, string, string) error); ok {
r0 = rf(ctx, pr, path, ref, comment)
} else {
r0 = ret.Error(0)
}
return r0
}
// MockPullRequestRepo_CommentPullRequestFile_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CommentPullRequestFile'
type MockPullRequestRepo_CommentPullRequestFile_Call struct {
*mock.Call
}
// CommentPullRequestFile is a helper method to define mock.On call
// - ctx context.Context
// - pr int
// - path string
// - ref string
// - comment string
func (_e *MockPullRequestRepo_Expecter) CommentPullRequestFile(ctx interface{}, pr interface{}, path interface{}, ref interface{}, comment interface{}) *MockPullRequestRepo_CommentPullRequestFile_Call {
return &MockPullRequestRepo_CommentPullRequestFile_Call{Call: _e.mock.On("CommentPullRequestFile", ctx, pr, path, ref, comment)}
}
func (_c *MockPullRequestRepo_CommentPullRequestFile_Call) Run(run func(ctx context.Context, pr int, path string, ref string, comment string)) *MockPullRequestRepo_CommentPullRequestFile_Call {
_c.Call.Run(func(args mock.Arguments) {
run(args[0].(context.Context), args[1].(int), args[2].(string), args[3].(string), args[4].(string))
})
return _c
}
func (_c *MockPullRequestRepo_CommentPullRequestFile_Call) Return(_a0 error) *MockPullRequestRepo_CommentPullRequestFile_Call {
_c.Call.Return(_a0)
return _c
}
func (_c *MockPullRequestRepo_CommentPullRequestFile_Call) RunAndReturn(run func(context.Context, int, string, string, string) error) *MockPullRequestRepo_CommentPullRequestFile_Call {
_c.Call.Return(run)
return _c
}
// CompareFiles provides a mock function with given fields: ctx, base, ref
func (_m *MockPullRequestRepo) CompareFiles(ctx context.Context, base string, ref string) ([]repository.VersionedFileChange, error) {
ret := _m.Called(ctx, base, ref)
@@ -19,8 +19,6 @@ type PullRequestRepo interface {
Config() *provisioning.Repository
Read(ctx context.Context, path, ref string) (*repository.FileInfo, error)
CompareFiles(ctx context.Context, base, ref string) ([]repository.VersionedFileChange, error)
ClearAllPullRequestFileComments(ctx context.Context, pr int) error
CommentPullRequestFile(ctx context.Context, pr int, path string, ref string, comment string) error
CommentPullRequest(ctx context.Context, pr int, comment string) error
}
@@ -740,33 +740,12 @@ func (r *githubRepository) CompareFiles(ctx context.Context, base, ref string) (
return changes, nil
}
// ClearAllPullRequestFileComments clears all comments on a pull request
func (r *githubRepository) ClearAllPullRequestFileComments(ctx context.Context, prNumber int) error {
ctx, _ = r.logger(ctx, "")
return r.gh.ClearAllPullRequestFileComments(ctx, r.owner, r.repo, prNumber)
}
// CommentPullRequest adds a comment to a pull request.
func (r *githubRepository) CommentPullRequest(ctx context.Context, prNumber int, comment string) error {
ctx, _ = r.logger(ctx, "")
return r.gh.CreatePullRequestComment(ctx, r.owner, r.repo, prNumber, comment)
}
// CommentPullRequestFile lints a file and comments the issues found.
func (r *githubRepository) CommentPullRequestFile(ctx context.Context, prNumber int, path, ref, comment string) error {
ctx, _ = r.logger(ctx, ref)
fileComment := pgh.FileComment{
Content: comment,
Path: path,
Position: 1, // create a top-level comment
Ref: ref,
}
// FIXME: comment with Grafana Logo
// FIXME: comment author should be written by Grafana and not the user
return r.gh.CreatePullRequestFileComment(ctx, r.owner, r.repo, prNumber, fileComment)
}
// ResourceURLs implements RepositoryWithURLs.
func (r *githubRepository) ResourceURLs(ctx context.Context, file *FileInfo) (*provisioning.ResourceURLs, error) {
cfg := r.config.Spec.GitHub
@@ -100,8 +100,6 @@ type Client interface {
ListPullRequestFiles(ctx context.Context, owner, repository string, number int) ([]CommitFile, error)
CreatePullRequestComment(ctx context.Context, owner, repository string, number int, body string) error
CreatePullRequestFileComment(ctx context.Context, owner, repository string, number int, comment FileComment) error
ClearAllPullRequestFileComments(ctx context.Context, owner, repository string, number int) error
}
//go:generate mockery --name RepositoryContent --structname MockRepositoryContent --inpackage --filename mock_repository_content.go --with-expecter
@@ -18,9 +18,7 @@ type githubClient struct {
gh *github.Client
}
var _ Client = (*githubClient)(nil)
func NewClient(client *github.Client) *githubClient {
func NewClient(client *github.Client) Client {
return &githubClient{client}
}
@@ -141,6 +139,7 @@ func (r *githubClient) GetTree(ctx context.Context, owner, repository, basePath,
if currentRef != ref {
// We're operating with a subpath which doesn't exist yet.
// Pretend as if there is simply no files.
// FIXME: why should we pretend this?
return nil, false, nil
}
// currentRef == ref
@@ -309,6 +308,8 @@ func (r *githubClient) Commits(ctx context.Context, owner, repository, path, bra
ret := make([]Commit, 0, len(commits))
for _, c := range commits {
// FIXME: This code is a mess. I am pretty sure that we have issue in
// some situations
var createdAt time.Time
var author *CommitAuthor
if c.GetCommit().GetAuthor() != nil {
@@ -373,12 +374,22 @@ func (r *githubClient) CompareCommits(ctx context.Context, owner, repository, ba
}
func (r *githubClient) GetBranch(ctx context.Context, owner, repository, branchName string) (Branch, error) {
branch, _, err := r.gh.Repositories.GetBranch(ctx, owner, repository, branchName, 0)
branch, resp, err := r.gh.Repositories.GetBranch(ctx, owner, repository, branchName, 0)
if err != nil {
// For some reason, GitHub client handles this case differently by failing with a wrapped error
if resp != nil && resp.StatusCode == http.StatusNotFound {
return Branch{}, ErrResourceNotFound
}
if resp != nil && resp.StatusCode == http.StatusServiceUnavailable {
return Branch{}, ErrServiceUnavailable
}
var ghErr *github.ErrorResponse
if !errors.As(err, &ghErr) {
return Branch{}, err
}
// Leaving these just in case
if ghErr.Response.StatusCode == http.StatusServiceUnavailable {
return Branch{}, ErrServiceUnavailable
}
@@ -520,6 +531,8 @@ func (r *githubClient) GetWebhook(ctx context.Context, owner, repository string,
contentType := hook.GetConfig().GetContentType()
if contentType == "" {
// FIXME: Not sure about the value of the contentType
// we default to form in the other ones but to JSON here
contentType = "json"
}
@@ -613,60 +626,6 @@ func (r *githubClient) CreatePullRequestComment(ctx context.Context, owner, repo
return nil
}
func (r *githubClient) CreatePullRequestFileComment(ctx context.Context, owner, repository string, number int, comment FileComment) error {
commentRequest := &github.PullRequestComment{
Body: &comment.Content,
CommitID: &comment.Ref,
Path: &comment.Path,
Position: &comment.Position,
}
if _, _, err := r.gh.PullRequests.CreateComment(ctx, owner, repository, number, commentRequest); err != nil {
var ghErr *github.ErrorResponse
if errors.As(err, &ghErr) && ghErr.Response.StatusCode == http.StatusServiceUnavailable {
return ErrServiceUnavailable
}
return err
}
return nil
}
func (r *githubClient) ClearAllPullRequestFileComments(ctx context.Context, owner, repository string, number int) error {
listFn := func(ctx context.Context, opts *github.ListOptions) ([]*github.PullRequestComment, *github.Response, error) {
return r.gh.PullRequests.ListComments(ctx, owner, repository, number, &github.PullRequestListCommentsOptions{
ListOptions: *opts,
})
}
comments, err := paginatedList(ctx, listFn, defaultListOptions(maxPullRequestsFileComments))
if errors.Is(err, ErrTooManyItems) {
return fmt.Errorf("too many comments to process (more than %d)", maxPullRequestsFileComments)
}
if err != nil {
return err
}
userLogin, _, err := r.gh.Users.Get(ctx, "")
if err != nil {
return fmt.Errorf("get user: %w", err)
}
for _, c := range comments {
// skip if comments were not created by us
if c.User.GetLogin() != userLogin.GetLogin() {
continue
}
if _, err := r.gh.PullRequests.DeleteComment(ctx, owner, repository, c.GetID()); err != nil {
return fmt.Errorf("delete comment: %w", err)
}
}
return nil
}
type realRepositoryContent struct {
real *github.RepositoryContent
}
File diff suppressed because it is too large Load Diff
@@ -80,55 +80,6 @@ func (_c *MockClient_BranchExists_Call) RunAndReturn(run func(context.Context, s
return _c
}
// ClearAllPullRequestFileComments provides a mock function with given fields: ctx, owner, repository, number
func (_m *MockClient) ClearAllPullRequestFileComments(ctx context.Context, owner string, repository string, number int) error {
ret := _m.Called(ctx, owner, repository, number)
if len(ret) == 0 {
panic("no return value specified for ClearAllPullRequestFileComments")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, string, string, int) error); ok {
r0 = rf(ctx, owner, repository, number)
} else {
r0 = ret.Error(0)
}
return r0
}
// MockClient_ClearAllPullRequestFileComments_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ClearAllPullRequestFileComments'
type MockClient_ClearAllPullRequestFileComments_Call struct {
*mock.Call
}
// ClearAllPullRequestFileComments is a helper method to define mock.On call
// - ctx context.Context
// - owner string
// - repository string
// - number int
func (_e *MockClient_Expecter) ClearAllPullRequestFileComments(ctx interface{}, owner interface{}, repository interface{}, number interface{}) *MockClient_ClearAllPullRequestFileComments_Call {
return &MockClient_ClearAllPullRequestFileComments_Call{Call: _e.mock.On("ClearAllPullRequestFileComments", ctx, owner, repository, number)}
}
func (_c *MockClient_ClearAllPullRequestFileComments_Call) Run(run func(ctx context.Context, owner string, repository string, number int)) *MockClient_ClearAllPullRequestFileComments_Call {
_c.Call.Run(func(args mock.Arguments) {
run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(int))
})
return _c
}
func (_c *MockClient_ClearAllPullRequestFileComments_Call) Return(_a0 error) *MockClient_ClearAllPullRequestFileComments_Call {
_c.Call.Return(_a0)
return _c
}
func (_c *MockClient_ClearAllPullRequestFileComments_Call) RunAndReturn(run func(context.Context, string, string, int) error) *MockClient_ClearAllPullRequestFileComments_Call {
_c.Call.Return(run)
return _c
}
// Commits provides a mock function with given fields: ctx, owner, repository, path, branch
func (_m *MockClient) Commits(ctx context.Context, owner string, repository string, path string, branch string) ([]Commit, error) {
ret := _m.Called(ctx, owner, repository, path, branch)
@@ -405,56 +356,6 @@ func (_c *MockClient_CreatePullRequestComment_Call) RunAndReturn(run func(contex
return _c
}
// CreatePullRequestFileComment provides a mock function with given fields: ctx, owner, repository, number, comment
func (_m *MockClient) CreatePullRequestFileComment(ctx context.Context, owner string, repository string, number int, comment FileComment) error {
ret := _m.Called(ctx, owner, repository, number, comment)
if len(ret) == 0 {
panic("no return value specified for CreatePullRequestFileComment")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, string, string, int, FileComment) error); ok {
r0 = rf(ctx, owner, repository, number, comment)
} else {
r0 = ret.Error(0)
}
return r0
}
// MockClient_CreatePullRequestFileComment_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CreatePullRequestFileComment'
type MockClient_CreatePullRequestFileComment_Call struct {
*mock.Call
}
// CreatePullRequestFileComment is a helper method to define mock.On call
// - ctx context.Context
// - owner string
// - repository string
// - number int
// - comment FileComment
func (_e *MockClient_Expecter) CreatePullRequestFileComment(ctx interface{}, owner interface{}, repository interface{}, number interface{}, comment interface{}) *MockClient_CreatePullRequestFileComment_Call {
return &MockClient_CreatePullRequestFileComment_Call{Call: _e.mock.On("CreatePullRequestFileComment", ctx, owner, repository, number, comment)}
}
func (_c *MockClient_CreatePullRequestFileComment_Call) Run(run func(ctx context.Context, owner string, repository string, number int, comment FileComment)) *MockClient_CreatePullRequestFileComment_Call {
_c.Call.Run(func(args mock.Arguments) {
run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(int), args[4].(FileComment))
})
return _c
}
func (_c *MockClient_CreatePullRequestFileComment_Call) Return(_a0 error) *MockClient_CreatePullRequestFileComment_Call {
_c.Call.Return(_a0)
return _c
}
func (_c *MockClient_CreatePullRequestFileComment_Call) RunAndReturn(run func(context.Context, string, string, int, FileComment) error) *MockClient_CreatePullRequestFileComment_Call {
_c.Call.Return(run)
return _c
}
// CreateWebhook provides a mock function with given fields: ctx, owner, repository, cfg
func (_m *MockClient) CreateWebhook(ctx context.Context, owner string, repository string, cfg WebhookConfig) (WebhookConfig, error) {
ret := _m.Called(ctx, owner, repository, cfg)
@@ -56,6 +56,7 @@ func TestLocalResolver(t *testing.T) {
"client.go",
"factory.go",
"impl.go",
"impl_test.go",
"mock_client.go",
"mock_commit_file.go",
"mock_repository_content.go",