From 4536d67c83bb6cd4d97509e9ffcdca90f8274f23 Mon Sep 17 00:00:00 2001 From: tongjingran Date: Wed, 29 Jul 2020 11:11:58 +0800 Subject: [PATCH] add comments for exported method and fix fmt move mock struct to test file --- pkg/github/github.go | 3 +- pkg/mock/clients/github.go | 34 ----------- pkg/mock/clients/qiniu.go | 61 -------------------- pkg/prow/job.go | 8 +-- pkg/prow/job_test.go | 103 ++++++++++++++++++++++++++++------ pkg/qiniu/client.go | 3 +- pkg/qiniu/object.go | 8 +-- pkg/qiniu/qnPresubmit.go | 4 +- pkg/qiniu/qnPresubmit_test.go | 6 +- 9 files changed, 101 insertions(+), 129 deletions(-) delete mode 100644 pkg/mock/clients/github.go delete mode 100644 pkg/mock/clients/qiniu.go diff --git a/pkg/github/github.go b/pkg/github/github.go index db70fcf..cf965d0 100644 --- a/pkg/github/github.go +++ b/pkg/github/github.go @@ -38,6 +38,7 @@ import ( // It is also the flag when checking whether the target comment exists or not to avoid duplicate const CommentsPrefix = "The following is the coverage report on the affected files." +// PrComment is the interface of the entry which is able to comment on Github Pull Requests type PrComment interface { CreateGithubComment(commentPrefix string, diffCovList cover.DeltaCovList) (err error) PostComment(content, commentPrefix string) error @@ -150,7 +151,7 @@ func (c *GithubPrComment) EraseHistoryComment(commentPrefix string) error { return nil } -//GetPrChangedFiles get github pull request changes file list +// GetPrChangedFiles get github pull request changes file list func (c *GithubPrComment) GetPrChangedFiles() (files []string, err error) { var commitFiles []*github.CommitFile for { diff --git a/pkg/mock/clients/github.go b/pkg/mock/clients/github.go deleted file mode 100644 index 2b77786..0000000 --- a/pkg/mock/clients/github.go +++ /dev/null @@ -1,34 +0,0 @@ -package clients - -import ( - "github.com/qiniu/goc/pkg/cover" -) - -type MockPrComment struct { - GetPrChangedFilesRes []string - GetPrChangedFilesErr error - PostCommentErr error - EraseHistoryCommentErr error - CreateGithubCommentErr error - CommentFlag string -} - -func (s *MockPrComment) GetPrChangedFiles() (files []string, err error) { - return s.GetPrChangedFilesRes, s.GetPrChangedFilesErr -} - -func (s *MockPrComment) PostComment(content, commentPrefix string) error { - return s.PostCommentErr -} - -func (s *MockPrComment) EraseHistoryComment(commentPrefix string) error { - return s.EraseHistoryCommentErr -} - -func (s *MockPrComment) CreateGithubComment(commentPrefix string, diffCovList cover.DeltaCovList) (err error) { - return s.CreateGithubCommentErr -} - -func (s *MockPrComment) GetCommentFlag() string { - return s.CommentFlag -} diff --git a/pkg/mock/clients/qiniu.go b/pkg/mock/clients/qiniu.go deleted file mode 100644 index dcdda29..0000000 --- a/pkg/mock/clients/qiniu.go +++ /dev/null @@ -1,61 +0,0 @@ -package clients - -import ( - "context" - "github.com/qiniu/goc/pkg/qiniu" - "os" - "time" -) - -type MockQnClient struct { - QiniuObjectHandleRes qiniu.ObjectHandle - ReadObjectRes []byte - ReadObjectErr error - ListAllRes []string - ListAllErr error - GetAccessURLRes string - GetArtifactDetailsRes *qiniu.LogHistoryTemplate - GetArtifactDetailsErr error - ListSubDirsRes []string - ListSubDirsErr error -} - -func (s *MockQnClient) QiniuObjectHandle(key string) qiniu.ObjectHandle { - return s.QiniuObjectHandleRes -} - -func (s *MockQnClient) ReadObject(key string) ([]byte, error) { - return s.ReadObjectRes, s.ReadObjectErr -} - -func (s *MockQnClient) ListAll(ctx context.Context, prefix string, delimiter string) ([]string, error) { - return s.ListAllRes, s.ListAllErr -} - -func (s *MockQnClient) GetAccessURL(key string, timeout time.Duration) string { - return s.GetAccessURLRes -} - -func (s *MockQnClient) GetArtifactDetails(key string) (*qiniu.LogHistoryTemplate, error) { - return s.GetArtifactDetailsRes, s.GetArtifactDetailsErr -} - -func (s *MockQnClient) ListSubDirs(prefix string) ([]string, error) { - return s.ListSubDirsRes, s.ListSubDirsErr -} - -type MockArtifacts struct { - ProfilePathRes string - CreateChangedProfileRes *os.File - GetChangedProfileNameRes string -} - -func (s *MockArtifacts) ProfilePath() string { - return s.ProfilePathRes -} -func (s *MockArtifacts) CreateChangedProfile() *os.File { - return s.CreateChangedProfileRes -} -func (s *MockArtifacts) GetChangedProfileName() string { - return s.GetChangedProfileNameRes -} diff --git a/pkg/prow/job.go b/pkg/prow/job.go index e1d3f69..5d73d53 100644 --- a/pkg/prow/job.go +++ b/pkg/prow/job.go @@ -215,8 +215,7 @@ func getFilesAndCovList(fullDiff bool, prComment github.PrComment, localP, baseP // get github pull request changed files' name var ghChangedFiles, err = prComment.GetPrChangedFiles() if err != nil { - logrus.Errorf("Get pull request changed file failed.") - return nil, nil, err + return nil, nil, fmt.Errorf("Get pull request changed file failed: %s", err.Error()) } if len(ghChangedFiles) == 0 { logrus.Printf("0 files changed in github pull request, don't need to run coverage profile in presubmit.\n") @@ -226,14 +225,13 @@ func getFilesAndCovList(fullDiff bool, prComment github.PrComment, localP, baseP // calculate diff cov between local and remote profile deltaCovList = cover.GetChFileDeltaCov(localP, baseP, changedFiles) + logrus.Printf("Get changed files and delta cover list success. ChangedFiles: [%+v], DeltaCovList: [%+v]", changedFiles, deltaCovList) return changedFiles, deltaCovList, nil } deltaCovList = cover.GetDeltaCov(localP, baseP) - logrus.Infof("get delta file name is:") for _, d := range deltaCovList { - logrus.Infof("%s", d.FileName) changedFiles = append(changedFiles, d.FileName) } - + logrus.Printf("Get all files and delta cover list success. Files: [%+v], DeltaCovList: [%+v]", changedFiles, deltaCovList) return changedFiles, deltaCovList, nil } diff --git a/pkg/prow/job_test.go b/pkg/prow/job_test.go index 633d6ce..b1667ca 100644 --- a/pkg/prow/job_test.go +++ b/pkg/prow/job_test.go @@ -17,18 +17,21 @@ package prow import ( + "context" "errors" "fmt" - "github.com/qiniu/goc/pkg/cover" - "github.com/qiniu/goc/pkg/github" - "github.com/qiniu/goc/pkg/mock/clients" - "github.com/qiniu/goc/pkg/qiniu" - "github.com/sirupsen/logrus" - "github.com/stretchr/testify/assert" "io/ioutil" "os" "path" "testing" + "time" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + + "github.com/qiniu/goc/pkg/cover" + "github.com/qiniu/goc/pkg/github" + "github.com/qiniu/goc/pkg/qiniu" ) var ( @@ -41,6 +44,72 @@ qiniu.com/kodo/bd/pfd/locker/bdlocker/locker.go:33.51,35.2 1 0` defaultChangedPath = "changed.cov" ) +type MockQnClient struct { + QiniuObjectHandleRes qiniu.ObjectHandle + ReadObjectRes []byte + ReadObjectErr error + ListAllRes []string + ListAllErr error + GetAccessURLRes string + GetArtifactDetailsRes *qiniu.LogHistoryTemplate + GetArtifactDetailsErr error + ListSubDirsRes []string + ListSubDirsErr error +} + +func (s *MockQnClient) QiniuObjectHandle(key string) qiniu.ObjectHandle { + return s.QiniuObjectHandleRes +} + +func (s *MockQnClient) ReadObject(key string) ([]byte, error) { + return s.ReadObjectRes, s.ReadObjectErr +} + +func (s *MockQnClient) ListAll(ctx context.Context, prefix string, delimiter string) ([]string, error) { + return s.ListAllRes, s.ListAllErr +} + +func (s *MockQnClient) GetAccessURL(key string, timeout time.Duration) string { + return s.GetAccessURLRes +} + +func (s *MockQnClient) GetArtifactDetails(key string) (*qiniu.LogHistoryTemplate, error) { + return s.GetArtifactDetailsRes, s.GetArtifactDetailsErr +} + +func (s *MockQnClient) ListSubDirs(prefix string) ([]string, error) { + return s.ListSubDirsRes, s.ListSubDirsErr +} + +type MockPrComment struct { + GetPrChangedFilesRes []string + GetPrChangedFilesErr error + PostCommentErr error + EraseHistoryCommentErr error + CreateGithubCommentErr error + CommentFlag string +} + +func (s *MockPrComment) GetPrChangedFiles() (files []string, err error) { + return s.GetPrChangedFilesRes, s.GetPrChangedFilesErr +} + +func (s *MockPrComment) PostComment(content, commentPrefix string) error { + return s.PostCommentErr +} + +func (s *MockPrComment) EraseHistoryComment(commentPrefix string) error { + return s.EraseHistoryCommentErr +} + +func (s *MockPrComment) CreateGithubComment(commentPrefix string, diffCovList cover.DeltaCovList) (err error) { + return s.CreateGithubCommentErr +} + +func (s *MockPrComment) GetCommentFlag() string { + return s.CommentFlag +} + func TestTrimGhFileToProfile(t *testing.T) { items := []struct { inputFiles []string @@ -149,7 +218,7 @@ func TestRunPresubmitFulldiff(t *testing.T) { func TestRunPresubmitError(t *testing.T) { items := []struct { - prepare bool // 是否需要准备本地cov + prepare bool // prepare local profile j Job err string }{ @@ -164,14 +233,14 @@ func TestRunPresubmitError(t *testing.T) { prepare: true, j: Job{ LocalProfilePath: defaultLocalPath, - QiniuClient: &clients.MockQnClient{}, + QiniuClient: &MockQnClient{}, }, }, { prepare: true, j: Job{ LocalProfilePath: defaultLocalPath, - QiniuClient: &clients.MockQnClient{ListSubDirsErr: errors.New("mock error")}, + QiniuClient: &MockQnClient{ListSubDirsErr: errors.New("mock error")}, }, err: "mock error", }, @@ -180,7 +249,7 @@ func TestRunPresubmitError(t *testing.T) { j: Job{ LocalProfilePath: defaultLocalPath, QiniuClient: &MockProfileQnClient{}, - GithubComment: &clients.MockPrComment{GetPrChangedFilesRes: []string{"qiniu.com/kodo/apiserver/server/main.go"}}, + GithubComment: &MockPrComment{GetPrChangedFilesRes: []string{"qiniu.com/kodo/apiserver/server/main.go"}}, FullDiff: true, LocalArtifacts: &qiniu.ProfileArtifacts{ChangedProfileName: defaultChangedPath}, }, @@ -204,7 +273,7 @@ func TestRunPresubmitError(t *testing.T) { } type MockProfileQnClient struct { - *clients.MockQnClient + *MockQnClient } func (s *MockProfileQnClient) ListSubDirs(prefix string) ([]string, error) { @@ -231,7 +300,7 @@ func TestGetFilesAndCovList(t *testing.T) { }{ { fullDiff: true, - prComment: &clients.MockPrComment{}, + prComment: &MockPrComment{}, localP: cover.CoverageList{ {FileName: "qiniu.com/kodo/apiserver/server/main.go", NCoveredStmts: 2, NAllStmts: 2}, {FileName: "qiniu.com/kodo/apiserver/server/test.go", NCoveredStmts: 2, NAllStmts: 2}, @@ -245,18 +314,18 @@ func TestGetFilesAndCovList(t *testing.T) { }, { fullDiff: false, - prComment: &clients.MockPrComment{GetPrChangedFilesErr: errors.New("mock error")}, + prComment: &MockPrComment{GetPrChangedFilesErr: errors.New("mock error")}, err: "mock error", }, { fullDiff: false, - prComment: &clients.MockPrComment{}, + prComment: &MockPrComment{}, lenFiles: 0, lenCovList: 0, }, { fullDiff: false, - prComment: &clients.MockPrComment{GetPrChangedFilesRes: []string{"qiniu.com/kodo/apiserver/server/main.go"}}, + prComment: &MockPrComment{GetPrChangedFilesRes: []string{"qiniu.com/kodo/apiserver/server/main.go"}}, localP: cover.CoverageList{ {FileName: "qiniu.com/kodo/apiserver/server/main.go", NCoveredStmts: 2, NAllStmts: 2}, {FileName: "qiniu.com/kodo/apiserver/server/test.go", NCoveredStmts: 2, NAllStmts: 2}, @@ -274,7 +343,7 @@ func TestGetFilesAndCovList(t *testing.T) { fmt.Println(i) files, covList, err := getFilesAndCovList(tc.fullDiff, tc.prComment, tc.localP, tc.baseP) if err != nil { - assert.Equal(t, err.Error(), tc.err) + assert.Contains(t, err.Error(), tc.err) } else { assert.Equal(t, len(files), tc.lenFiles) assert.Equal(t, len(covList), tc.lenCovList) @@ -285,7 +354,7 @@ func TestGetFilesAndCovList(t *testing.T) { func TestSetDeltaCovLinks(t *testing.T) { covList := cover.DeltaCovList{{FileName: "file1", BasePer: "5%", NewPer: "5%", DeltaPer: "0"}} j := &Job{ - QiniuClient: &clients.MockQnClient{}, + QiniuClient: &MockQnClient{}, } j.SetDeltaCovLinks(covList) } diff --git a/pkg/qiniu/client.go b/pkg/qiniu/client.go index 640730d..d3255b8 100644 --- a/pkg/qiniu/client.go +++ b/pkg/qiniu/client.go @@ -43,6 +43,7 @@ type Config struct { Domain string `json:"domain"` } +// Client is the interface contains the operation with qiniu cloud type Client interface { QiniuObjectHandle(key string) ObjectHandle ReadObject(key string) ([]byte, error) @@ -103,7 +104,7 @@ func (q *QnClient) ListAll(ctx context.Context, prefix string, delimiter string) return files, nil } -// ListAll to list all the entries with contains the expected prefix +// listEntries to list all the entries with contains the expected prefix func (q *QnClient) listEntries(prefix string, delimiter string) ([]storage.ListItem, error) { var marker string var artifacts []storage.ListItem diff --git a/pkg/qiniu/object.go b/pkg/qiniu/object.go index e4f6db0..81a8823 100644 --- a/pkg/qiniu/object.go +++ b/pkg/qiniu/object.go @@ -29,8 +29,8 @@ import ( "github.com/sirupsen/logrus" ) +// ObjectHandle is the interface contains the operations on an object in a qiniu cloud bucket type ObjectHandle interface { - Attrs(ctx context.Context) (storage.FileInfo, error) NewReader(ctx context.Context) (io.ReadCloser, error) NewRangeReader(ctx context.Context, offset, length int64) (io.ReadCloser, error) } @@ -44,12 +44,6 @@ type QnObjectHandle struct { client *client.Client } -// Attrs get the object's metainfo -func (o *QnObjectHandle) Attrs(ctx context.Context) (storage.FileInfo, error) { - //TODO(CarlJi): need retry when errors - return o.bm.Stat(o.cfg.Bucket, o.key) -} - // NewReader creates a reader to read the contents of the object. // ErrObjectNotExist will be returned if the object is not found. // The caller must call Close on the returned Reader when done reading. diff --git a/pkg/qiniu/qnPresubmit.go b/pkg/qiniu/qnPresubmit.go index 22ed770..4aedf9e 100644 --- a/pkg/qiniu/qnPresubmit.go +++ b/pkg/qiniu/qnPresubmit.go @@ -107,13 +107,14 @@ func FindBaseProfileFromQiniu(qc Client, prowJobName, covProfileName string) ([] return qc.ReadObject(profilePath) } +// Artifacts is the interface of the rule to store test artifacts in prow type Artifacts interface { ProfilePath() string CreateChangedProfile() *os.File GetChangedProfileName() string } -// ProfileArtifacts prepresents the rule to store test artifacts in prow +// ProfileArtifacts presents the rule to store test artifacts in prow type ProfileArtifacts struct { Directory string ProfileName string @@ -139,6 +140,7 @@ func (a *ProfileArtifacts) CreateChangedProfile() *os.File { return p } +// GetChangedProfileName get ChangedProfileName of the ProfileArtifacts func (a *ProfileArtifacts) GetChangedProfileName() string { return a.ChangedProfileName } diff --git a/pkg/qiniu/qnPresubmit_test.go b/pkg/qiniu/qnPresubmit_test.go index 759e151..a4b1680 100644 --- a/pkg/qiniu/qnPresubmit_test.go +++ b/pkg/qiniu/qnPresubmit_test.go @@ -17,10 +17,10 @@ package qiniu import ( + "os" "testing" "github.com/stretchr/testify/assert" - "os" ) func TestFindBaseProfileFromQiniu(t *testing.T) { @@ -55,8 +55,10 @@ func TestProfileArtifacts_CreateChangedProfile(t *testing.T) { ChangedProfileName: "test.cov", } file := p.CreateChangedProfile() - defer file.Close() + file.Close() defer os.Remove(p.ChangedProfileName) + _, err := os.Stat(p.ChangedProfileName) + assert.NoError(t, err) } func TestProfileArtifacts_GetChangedProfileName(t *testing.T) {