diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 0000000..cd453d5 --- /dev/null +++ b/.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 \ No newline at end of file diff --git a/README.md b/README.md index d81da22..e17fbfa 100644 --- a/README.md +++ b/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. diff --git a/cmd/build_test.go b/cmd/build_test.go index 6e86287..fc3841c 100644 --- a/cmd/build_test.go +++ b/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") } diff --git a/cmd/commonflags.go b/cmd/commonflags.go index fcd63d4..9440276 100644 --- a/cmd/commonflags.go +++ b/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" } diff --git a/cmd/install_test.go b/cmd/install_test.go index d18a674..5b94c97 100644 --- a/cmd/install_test.go +++ b/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") } diff --git a/pkg/build/build.go b/pkg/build/build.go index 01ad30b..50b2809 100644 --- a/pkg/build/build.go +++ b/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 diff --git a/pkg/build/build_test.go b/pkg/build/build_test.go index 7255730..3237277 100644 --- a/pkg/build/build_test.go +++ b/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") diff --git a/pkg/build/errors.go b/pkg/build/errors.go index 6359434..e0376bd 100644 --- a/pkg/build/errors.go +++ b/pkg/build/errors.go @@ -5,12 +5,20 @@ import ( ) var ( - ErrShouldNotReached = errors.New("should never be reached") - ErrGocShouldExecInProject = errors.New("goc not executed in project directory") + // ErrShouldNotReached represents the logic should not be reached in normal flow + 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 \".\"") - ErrTooManyArgs = errors.New("too many args") - ErrInvalidWorkingDir = errors.New("the working directory is invalid") - ErrWrongCallSequence = errors.New("function should be called in a specified sequence") - ErrNoplaceToInstall = errors.New("dont know where to install") + // ErrWrongPackageTypeForBuild represents goc build command only support limited arguments + ErrWrongPackageTypeForBuild = errors.New("packages only support \".\"") + // ErrTooManyArgs represents goc CLI only support limited arguments + ErrTooManyArgs = errors.New("too many args") + // 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") ) diff --git a/pkg/build/install.go b/pkg/build/install.go index 8546e46..b5cb823 100644 --- a/pkg/build/install.go +++ b/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) diff --git a/pkg/build/tmpfolder.go b/pkg/build/tmpfolder.go index 5998ebd..9a1a08c 100644 --- a/pkg/build/tmpfolder.go +++ b/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 } diff --git a/pkg/build/tmpfolder_test.go b/pkg/build/tmpfolder_test.go index c730068..07614be 100644 --- a/pkg/build/tmpfolder_test.go +++ b/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 diff --git a/pkg/cover/cover.go b/pkg/cover/cover.go index 201dd57..59e66e1 100644 --- a/pkg/cover/cover.go +++ b/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()) diff --git a/pkg/cover/cover_test.go b/pkg/cover/cover_test.go index f1a0de4..9c415ef 100644 --- a/pkg/cover/cover_test.go +++ b/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") diff --git a/pkg/cover/server.go b/pkg/cover/server.go index 05d0efe..0d09f52 100644 --- a/pkg/cover/server.go +++ b/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) } diff --git a/pkg/github/github.go b/pkg/github/github.go index bbefbf0..5ed7609 100644 --- a/pkg/github/github.go +++ b/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) diff --git a/pkg/prow/job.go b/pkg/prow/job.go index 96269cf..91afefe 100644 --- a/pkg/prow/job.go +++ b/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 ( diff --git a/pkg/qiniu/mock.go b/pkg/qiniu/mock.go index 59e56ad..d60c7bc 100644 --- a/pkg/qiniu/mock.go +++ b/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 } diff --git a/pkg/qiniu/object.go b/pkg/qiniu/object.go index e374c60..3f10b37 100644 --- a/pkg/qiniu/object.go +++ b/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) diff --git a/pkg/qiniu/qnPresubmit.go b/pkg/qiniu/qnPresubmit.go index 203cb88..ed140e1 100644 --- a/pkg/qiniu/qnPresubmit.go +++ b/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")