add comments for exported method and fix fmt

move mock struct to test file

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 {

pkg/mock/clients/github.go

@@ -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
-}

pkg/mock/clients/qiniu.go

@@ -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
-}

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, fmt.Errorf("Get pull request changed file failed: %s", err.Error())
-			return nil, nil, err
 		}
 		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
 }

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)
 }

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

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.

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
 }

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) {