Merge pull request #82 from CarlJi/0726

fix golint error and add golanglint-ci to check code quality persistently

.github/workflows/golangci-lint.yml

@@ -0,0 +1,28 @@
+name: golangci-lint
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - master
+  pull_request:
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v1.2.1
+        with:
+          # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
+          version: v1.29
+
+          # Optional: working directory, useful for monorepos
+          # working-directory: somedir
+
+          # Optional: golangci-lint command line arguments.
+          # args: --issues-exit-code=0
+
+          # Optional: show only new issues if it's a pull request. The default value is `false`.
+          only-new-issues: true

README.md

@@ -57,7 +57,7 @@ Goc can collect code coverages at runtime for your long-run golang applications.
 ## RoadMap
 - [x] Support code coverage collection for system testing.
 - [x] Support code coverage counters clear for the services under test at runtime.
-- [ ] Support develop mode towards accurate testing.
+- [x] Support develop mode towards accurate testing.
 - [ ] Support code coverage diff based on Pull Request.
 - [ ] Optimize the performance costed by code coverage counters.
 

cmd/build_test.go

@@ -58,7 +58,7 @@ func TestGeneratedBinary(t *testing.T) {
 	assert.Equal(t, cnt > 0, true, "main.registerSelf function should be in the binary")
 
 	cnt = strings.Count(string(out), "GoCover")
-	assert.Equal(t, cnt > 0, true, "GoCover varibale should be in the binary")
+	assert.Equal(t, cnt > 0, true, "GoCover variable should be in the binary")
 }
 
 func TestBuildBinaryName(t *testing.T) {
@@ -86,7 +86,7 @@ func TestBuildBinaryName(t *testing.T) {
 	assert.Equal(t, cnt > 0, true, "main.registerSelf function should be in the binary")
 
 	cnt = strings.Count(string(out), "GoCover")
-	assert.Equal(t, cnt > 0, true, "GoCover varibale should be in the binary")
+	assert.Equal(t, cnt > 0, true, "GoCover variable should be in the binary")
 }
 
 // test if goc can get variables in internal package
@@ -115,5 +115,5 @@ func TestBuildBinaryForInternalPackage(t *testing.T) {
 	assert.Equal(t, cnt > 0, true, "GoCacheCover variable for internal package should be in the binary")
 
 	cnt = strings.Count(string(out), "internal.GoCover")
-	assert.Equal(t, cnt > 0, true, "internal.GoCover varibale should be in the binary")
+	assert.Equal(t, cnt > 0, true, "internal.GoCover variable should be in the binary")
 }

cmd/commonflags.go

@@ -70,7 +70,7 @@ func addRunFlags(cmdset *pflag.FlagSet) {
 	viper.BindPFlags(cmdset)
 }
 
-// add Cover Mode check
+// CoverMode represents the covermode when doing cover for source code
 type CoverMode struct {
 	mode string
 }
@@ -79,6 +79,7 @@ func (m *CoverMode) String() string {
 	return m.mode
 }
 
+// Set sets the value to the CoverMode struct, use 'count' as default if v is empty
 func (m *CoverMode) Set(v string) error {
 	if v == "" {
 		m.mode = "count"
@@ -91,11 +92,12 @@ func (m *CoverMode) Set(v string) error {
 	return nil
 }
 
+// Type returns the type of CoverMode
 func (m *CoverMode) Type() string {
 	return "string"
 }
 
-// add agentPort check
+// AgentPort is the struct to do agentPort check
 type AgentPort struct {
 	port string
 }
@@ -104,6 +106,7 @@ func (agent *AgentPort) String() string {
 	return agent.port
 }
 
+// Set sets the value to the AgentPort struct
 func (agent *AgentPort) Set(v string) error {
 	if v == "" {
 		agent.port = ""
@@ -117,6 +120,7 @@ func (agent *AgentPort) Set(v string) error {
 	return nil
 }
 
+// Type returns the type of AgentPort
 func (agent *AgentPort) Type() string {
 	return "string"
 }

cmd/install_test.go

@@ -52,7 +52,7 @@ func TestInstalledBinaryForMod(t *testing.T) {
 	assert.Equal(t, cnt > 0, true, "main.registerSelf function should be in the binary")
 
 	cnt = strings.Count(string(out), "GoCover")
-	assert.Equal(t, cnt > 0, true, "GoCover varibale should be in the binary")
+	assert.Equal(t, cnt > 0, true, "GoCover variable should be in the binary")
 }
 
 func TestInstalledBinaryForLegacy(t *testing.T) {
@@ -80,5 +80,5 @@ func TestInstalledBinaryForLegacy(t *testing.T) {
 	assert.Equal(t, cnt > 0, true, "main.registerSelf function should be in the binary")
 
 	cnt = strings.Count(string(out), "GoCover")
-	assert.Equal(t, cnt > 0, true, "GoCover varibale should be in the binary")
+	assert.Equal(t, cnt > 0, true, "GoCover variable should be in the binary")
 }

pkg/build/build.go

@@ -79,6 +79,7 @@ func NewBuild(buildflags string, args []string, workingDir string, outputDir str
 	return b, nil
 }
 
+// Build calls 'go build' tool to do building
 func (b *Build) Build() error {
 	log.Infoln("Go building in temp...")
 	// new -o will overwrite  previous ones
@@ -109,7 +110,7 @@ func (b *Build) Build() error {
 // the binary name is always same as the directory name of current directory
 func (b *Build) determineOutputDir(outputDir string) (string, error) {
 	if b.TmpDir == "" {
-		return "", fmt.Errorf("can only be called after Build.MvProjectsToTmp(): %w", ErrWrongCallSequence)
+		return "", fmt.Errorf("can only be called after Build.MvProjectsToTmp(): %w", ErrEmptyTempWorkingDir)
 	}
 
 	// fix #43

pkg/build/build_test.go

@@ -70,7 +70,7 @@ func TestCheckParameters(t *testing.T) {
 func TestDetermineOutputDir(t *testing.T) {
 	b := &Build{}
 	_, err := b.determineOutputDir("")
-	assert.Equal(t, errors.Is(err, ErrWrongCallSequence), true, "called before Build.MvProjectsToTmp() should fail")
+	assert.Equal(t, errors.Is(err, ErrEmptyTempWorkingDir), true, "called before Build.MvProjectsToTmp() should fail")
 
 	b.TmpDir = "fake"
 	_, err = b.determineOutputDir("xx")

pkg/build/errors.go

@@ -5,12 +5,20 @@ import (
 )
 
 var (
-	ErrShouldNotReached           = errors.New("should never be reached")
+	// ErrShouldNotReached represents the logic should not be reached in normal flow
-	ErrGocShouldExecInProject     = errors.New("goc not executed in project directory")
+	ErrShouldNotReached = errors.New("should never be reached")
+	// ErrGocShouldExecInProject represents goc currently not support for the project
+	ErrGocShouldExecInProject = errors.New("goc not support for such project directory")
+	// ErrWrongPackageTypeForInstall represents goc install command only support limited arguments
 	ErrWrongPackageTypeForInstall = errors.New("packages only support \".\" and \"./...\"")
-	ErrWrongPackageTypeForBuild   = errors.New("packages only support \".\"")
+	// ErrWrongPackageTypeForBuild represents goc build command only support limited arguments
-	ErrTooManyArgs                = errors.New("too many args")
+	ErrWrongPackageTypeForBuild = errors.New("packages only support \".\"")
-	ErrInvalidWorkingDir          = errors.New("the working directory is invalid")
+	// ErrTooManyArgs represents goc CLI only support limited arguments
-	ErrWrongCallSequence          = errors.New("function should be called in a specified sequence")
+	ErrTooManyArgs = errors.New("too many args")
-	ErrNoplaceToInstall           = errors.New("dont know where to install")
+	// ErrInvalidWorkingDir represents the working directory is invalid
+	ErrInvalidWorkingDir = errors.New("the working directory is invalid")
+	// ErrEmptyTempWorkingDir represent the error that temporary working directory is empty
+	ErrEmptyTempWorkingDir = errors.New("temporary working directory is empty")
+	// ErrNoPlaceToInstall represents the err that no place to install the generated binary
+	ErrNoPlaceToInstall = errors.New("don't know where to install")
 )

pkg/build/install.go

@@ -45,6 +45,7 @@ func NewInstall(buildflags string, args []string, workingDir string) (*Build, er
 	return b, nil
 }
 
+// Install use the 'go install' tool to install packages
 func (b *Build) Install() error {
 	log.Println("Go building in temp...")
 	cmd := exec.Command("/bin/bash", "-c", "go install "+b.BuildFlags+" "+b.Packages)

pkg/build/tmpfolder.go

@@ -29,6 +29,7 @@ import (
 	"github.com/spf13/viper"
 )
 
+// MvProjectsToTmp moves the projects into a temporary directory
 func (b *Build) MvProjectsToTmp() error {
 	listArgs := []string{"-json"}
 	if len(b.BuildFlags) != 0 {
@@ -67,7 +68,7 @@ func (b *Build) MvProjectsToTmp() error {
 }
 
 func (b *Build) mvProjectsToTmp() error {
-	b.TmpDir = filepath.Join(os.TempDir(), TmpFolderName(b.WorkingDir))
+	b.TmpDir = filepath.Join(os.TempDir(), tmpFolderName(b.WorkingDir))
 
 	// Delete previous tmp folder and its content
 	os.RemoveAll(b.TmpDir)
@@ -110,7 +111,9 @@ func (b *Build) mvProjectsToTmp() error {
 	return nil
 }
 
-func TmpFolderName(path string) string {
+// tmpFolderName uses the first six characters of the input path's SHA256 checksum
+// as the suffix.
+func tmpFolderName(path string) string {
 	sum := sha256.Sum256([]byte(path))
 	h := fmt.Sprintf("%x", sum[:6])
 
@@ -169,7 +172,7 @@ func (b *Build) findWhereToInstall() (string, error) {
 
 	if false == b.IsMod {
 		if b.Root == "" {
-			return "", ErrNoplaceToInstall
+			return "", ErrNoPlaceToInstall
 		}
 		return filepath.Join(b.Root, "bin"), nil
 	}

pkg/build/tmpfolder_test.go

@@ -133,7 +133,7 @@ func TestFindWhereToInstall(t *testing.T) {
 		Root:  "",
 	}
 	_, err := b.findWhereToInstall()
-	assert.EqualError(t, err, ErrNoplaceToInstall.Error())
+	assert.EqualError(t, err, ErrNoPlaceToInstall.Error())
 
 	// if $GOBIN not found
 	// and if $GOPATH not found

pkg/cover/cover.go

@@ -39,7 +39,9 @@ import (
 )
 
 var (
-	ErrCoverPkgFailed  = errors.New("fail to inject code to project")
+	// ErrCoverPkgFailed represents the error that fails to inject the package
+	ErrCoverPkgFailed = errors.New("fail to inject code to project")
+	// ErrCoverListFailed represents the error that fails to list package dependencies
 	ErrCoverListFailed = errors.New("fail to list package dependencies")
 )
 
@@ -94,6 +96,7 @@ type Package struct {
 	DepsErrors []*PackageError `json:"DepsErrors,omitempty"` // errors loading dependencies
 }
 
+// ModulePublic represents the package info of a module
 type ModulePublic struct {
 	Path      string        `json:",omitempty"` // module path
 	Version   string        `json:",omitempty"` // module version
@@ -109,6 +112,7 @@ type ModulePublic struct {
 	Error     *ModuleError  `json:",omitempty"` // error loading module
 }
 
+// ModuleError represents the error loading module
 type ModuleError struct {
 	Err string // error text
 }
@@ -200,7 +204,7 @@ func Execute(args, newGopath, target, mode, agentPort, center string) error {
 
 						// Some internal package have same parent dir or import path
 						// Cache all vars by internal parent dir for all child internal counter vars
-						cacheCover := AddCacheCover(pkg, inPkgCover)
+						cacheCover := addCacheCover(pkg, inPkgCover)
 						if v, ok := tc.CacheCover[cacheCover.Package.Dir]; ok {
 							for cVar, val := range v.Vars {
 								cacheCover.Vars[cVar] = val
@@ -211,7 +215,7 @@ func Execute(args, newGopath, target, mode, agentPort, center string) error {
 						}
 
 						// Cache all internal vars to internal parent package
-						inCover := CacheInternalCover(inPkgCover)
+						inCover := cacheInternalCover(inPkgCover)
 						if v, ok := internalPkgCache[cacheCover.Package.Dir]; ok {
 							v = append(v, inCover)
 							internalPkgCache[cacheCover.Package.Dir] = v
@@ -417,7 +421,7 @@ func declareCacheVars(in *PackageCover) map[string]*FileVar {
 	return vars
 }
 
-func CacheInternalCover(in *PackageCover) *PackageCover {
+func cacheInternalCover(in *PackageCover) *PackageCover {
 	c := &PackageCover{}
 	vars := declareCacheVars(in)
 	c.Package = in.Package
@@ -425,7 +429,7 @@ func CacheInternalCover(in *PackageCover) *PackageCover {
 	return c
 }
 
-func AddCacheCover(pkg *Package, in *PackageCover) *PackageCover {
+func addCacheCover(pkg *Package, in *PackageCover) *PackageCover {
 	c := &PackageCover{}
 	sum := sha256.Sum256([]byte(pkg.ImportPath))
 	h := fmt.Sprintf("%x", sum[:6])
@@ -458,7 +462,7 @@ type codeBlock struct {
 	coverageCount int    // number of times the block is covered
 }
 
-//convert profile to CoverageList struct
+// CovList converts profile to CoverageList struct
 func CovList(f io.Reader) (g CoverageList, err error) {
 	scanner := bufio.NewScanner(f)
 	scanner.Scan() // discard first line
@@ -475,7 +479,7 @@ func CovList(f io.Reader) (g CoverageList, err error) {
 	return
 }
 
-// covert profile file to CoverageList struct
+// ReadFileToCoverList coverts profile file to CoverageList struct
 func ReadFileToCoverList(path string) (g CoverageList, err error) {
 	f, err := ioutil.ReadFile(path)
 	if err != nil {
@@ -538,13 +542,14 @@ func (g *CoverageList) append(c *Coverage) {
 	*g = append(*g, *c)
 }
 
-// sort CoverageList with filenames
+// Sort sorts CoverageList with filenames
 func (g CoverageList) Sort() {
 	sort.SliceStable(g, func(i, j int) bool {
 		return g[i].Name() < g[j].Name()
 	})
 }
 
+// TotalPercentage returns the total percentage of coverage
 func (g CoverageList) TotalPercentage() string {
 	ratio, err := g.TotalRatio()
 	if err == nil {
@@ -553,6 +558,7 @@ func (g CoverageList) TotalPercentage() string {
 	return "N/A"
 }
 
+// TotalRatio returns the total ratio of covered statements
 func (g CoverageList) TotalRatio() (ratio float32, err error) {
 	var total Coverage
 	for _, c := range g {
@@ -586,6 +592,7 @@ func (c *Coverage) Percentage() string {
 	return "N/A"
 }
 
+// Ratio calculates the ratio of statements in a profile
 func (c *Coverage) Ratio() (ratio float32, err error) {
 	if c.NAllStmts == 0 {
 		err = fmt.Errorf("[%s] has 0 statement", c.Name())

pkg/cover/cover_test.go

@@ -364,6 +364,9 @@ func TestCoverResultForInternalPackage(t *testing.T) {
 	}
 
 	out, err := ioutil.ReadFile(filepath.Join(testDir, "http_cover_apis_auto_generated.go"))
+	if err != nil {
+		assert.FailNow(t, "failed to read http_cover_apis_auto_generated.go file")
+	}
 	cnt := strings.Count(string(out), "GoCacheCover")
 	assert.Equal(t, cnt > 0, true, "GoCacheCover variable should be in http_cover_apis_auto_generated.go")
 

pkg/cover/server.go

@@ -116,7 +116,7 @@ func registerService(c *gin.Context) {
 
 	realIP := c.ClientIP()
 	if host != realIP {
-		log.Printf("the registed host %s of service %s is different with the real one %s, here we choose the real one", service.Name, host, realIP)
+		log.Printf("the registered host %s of service %s is different with the real one %s, here we choose the real one", service.Name, host, realIP)
 		service.Address = fmt.Sprintf("http://%s:%s", realIP, port)
 	}
 

pkg/github/github.go

@@ -34,8 +34,11 @@ import (
 	"github.com/qiniu/goc/pkg/cover"
 )
 
+// CommentsPrefix is the prefix when commenting on Github Pull Requests
+// 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 entry which is able to comment on Github Pull Requests
 type PrComment struct {
 	RobotUserName string
 	RepoOwner     string
@@ -81,7 +84,7 @@ func NewPrClient(githubTokenPath, repoOwner, repoName, prNumStr, botUserName, co
 	}
 }
 
-//post github comment of diff coverage
+// CreateGithubComment post github comment of diff coverage
 func (c *PrComment) CreateGithubComment(commentPrefix string, diffCovList cover.DeltaCovList) (err error) {
 	if len(diffCovList) == 0 {
 		logrus.Printf("Detect 0 files coverage diff, will not comment to github.")
@@ -97,6 +100,7 @@ func (c *PrComment) CreateGithubComment(commentPrefix string, diffCovList cover.
 	return
 }
 
+// PostComment post comment on github. It erased the old one if existed to avoid duplicate
 func (c *PrComment) PostComment(content, commentPrefix string) error {
 	//step1: erase history similar comment to avoid too many comment for same job
 	err := c.EraseHistoryComment(commentPrefix)
@@ -116,7 +120,7 @@ func (c *PrComment) PostComment(content, commentPrefix string) error {
 	return nil
 }
 
-// erase history similar comment before post again
+// EraseHistoryComment erase history similar comment before post again
 func (c *PrComment) EraseHistoryComment(commentPrefix string) error {
 	comments, _, err := c.GithubClient.Issues.ListComments(c.Ctx, c.RepoOwner, c.RepoName, c.PrNumber, nil)
 	if err != nil {
@@ -161,7 +165,7 @@ func (c *PrComment) GetPrChangedFiles() (files []string, err error) {
 	return
 }
 
-//generate github comment content based on diff coverage and commentFlag
+// GenCommentContent generate github comment content based on diff coverage and commentFlag
 func GenCommentContent(commentPrefix string, delta cover.DeltaCovList) string {
 	var buf bytes.Buffer
 	table := tablewriter.NewWriter(&buf)

pkg/prow/job.go

@@ -13,6 +13,7 @@
  See the License for the specific language governing permissions and
  limitations under the License.
 */
+
 package prow
 
 import (

pkg/qiniu/mock.go

@@ -26,6 +26,7 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// MockQiniuServer simulate qiniu cloud for testing
 func MockQiniuServer(config *Config) (client *Client, router *httprouter.Router, serverURL string, teardown func()) {
 	// router is the HTTP request multiplexer used with the test server.
 	router = httprouter.New()
@@ -54,7 +55,7 @@ func MockRouterAPI(router *httprouter.Router, profile string, count int) {
 		logrus.Infof("request url is: %s", r.URL.String())
 
 		if timeout > 0 {
-			timeout -= 1
+			timeout--
 			http.Error(w, "not found", http.StatusNotFound)
 			return
 		}

pkg/qiniu/object.go

@@ -38,6 +38,7 @@ type ObjectHandle struct {
 	client *client.Client
 }
 
+// Attrs get the object's metainfo
 func (o *ObjectHandle) Attrs(ctx context.Context) (storage.FileInfo, error) {
 	//TODO(CarlJi): need retry when errors
 	return o.bm.Stat(o.cfg.Bucket, o.key)

pkg/qiniu/qnPresubmit.go

@@ -19,11 +19,12 @@ package qiniu
 import (
 	"encoding/json"
 	"fmt"
-	log "github.com/sirupsen/logrus"
 	"os"
 	"path"
 	"sort"
 	"strconv"
+
+	log "github.com/sirupsen/logrus"
 )
 
 const (
@@ -33,10 +34,10 @@ const (
 	// ArtifactsDirName is the name of directory defined in prow to store test artifacts
 	ArtifactsDirName = "artifacts"
 
-	//default prow coverage file
+	//PostSubmitCoverProfile represents the default output coverage file generated in prow environment
 	PostSubmitCoverProfile = "filtered.cov"
 
-	//default to save changed file related coverage profile
+	//ChangedProfileName represents the default changed coverage profile based on files changed in Pull Request
 	ChangedProfileName = "changed-file-profile.cov"
 )
 
@@ -106,16 +107,19 @@ func FindBaseProfileFromQiniu(qc *Client, prowJobName, covProfileName string) ([
 	return qc.ReadObject(profilePath)
 }
 
+// Artifacts prepresents the rule to store test artifacts in prow
 type Artifacts struct {
 	Directory          string
 	ProfileName        string
 	ChangedProfileName string // create temporary to save changed file related coverage profile
 }
 
+// ProfilePath returns a full path for profile
 func (a *Artifacts) ProfilePath() string {
 	return path.Join(a.Directory, a.ProfileName)
 }
 
+// CreateChangedProfile creates a profile in order to store the most related files based on Github Pull Request
 func (a *Artifacts) CreateChangedProfile() *os.File {
 	if a.ChangedProfileName == "" {
 		log.Fatalf("param Artifacts.ChangedProfileName should not be empty")