Skip to content

Commit

Permalink
speed up CI and golangci-lint
Browse files Browse the repository at this point in the history
Run CI on mac os only with go1.13 and on windows only on go1.14.
Speed up tests. Introduce --allow-parallel-runners.
Block on parallel run lock 5s instead of 60s.
Don't invalidate analysis cache for minor config changes.
  • Loading branch information
jirfag committed May 6, 2020
1 parent 4e99c75 commit e47ee39
Show file tree
Hide file tree
Showing 16 changed files with 145 additions and 71 deletions.
13 changes: 6 additions & 7 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,12 @@ jobs:
tests-on-windows:
needs: golangci-lint # run after golangci-lint action to not produce duplicated errors
runs-on: windows-latest
strategy:
matrix:
golang:
- 1.13
- 1.14
steps:
- uses: actions/checkout@v2
- name: Install Go
uses: actions/setup-go@v2
with:
go-version: ${{ matrix.golang }}
go-version: 1.14 # test only the latest go version to speed up CI
- name: Run tests on Windows
run: make.exe test
continue-on-error: true
Expand All @@ -47,7 +42,11 @@ jobs:
- 1.14
os:
- ubuntu-latest
- macos-latest
include:
- os: macos-latest
# test only the one go version on Mac OS to speed up CI
# TODO: use the latet go version after https://github.com/golang/go/issues/38824
golang: 1.13
steps:
- uses: actions/checkout@v2
- name: Install Go
Expand Down
4 changes: 4 additions & 0 deletions .golangci.example.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ run:
# the dependency descriptions in go.mod.
modules-download-mode: readonly|release|vendor

# Allow multiple parallel golangci-lint instances running.
# If false (default) - golangci-lint acquires file lock on start.
allow-parallel-runners: false


# output configuration options
output:
Expand Down
4 changes: 1 addition & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ clean:
test: export GOLANGCI_LINT_INSTALLED = true
test: build
GL_TEST_RUN=1 ./golangci-lint run -v
GL_TEST_RUN=1 ./golangci-lint run --fast --no-config -v --skip-dirs 'test/testdata_etc,internal/(cache|renameio|robustio)'
GL_TEST_RUN=1 ./golangci-lint run --no-config -v --skip-dirs 'test/testdata_etc,internal/(cache|renameio|robustio)'
GL_TEST_RUN=1 go test -v ./...
GL_TEST_RUN=1 go test -v -parallel 2 ./...
.PHONY: test

test_race: build_race
Expand Down
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ Flags:
- (^|/)builtin($|/)
(default true)
--skip-files strings Regexps of files to skip
--allow-parallel-runners Allow multiple parallel golangci-lint instances running. If false (default) - golangci-lint acquires file lock on start.
-E, --enable strings Enable specific linter
-D, --disable strings Disable specific linter
--disable-all Disable all linters
Expand Down Expand Up @@ -679,6 +680,10 @@ run:
# the dependency descriptions in go.mod.
modules-download-mode: readonly|release|vendor
# Allow multiple parallel golangci-lint instances running.
# If false (default) - golangci-lint acquires file lock on start.
allow-parallel-runners: false
# output configuration options
output:
Expand Down
6 changes: 6 additions & 0 deletions internal/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ func init() {
}

func TestBasic(t *testing.T) {
t.Parallel()

dir, err := ioutil.TempDir("", "cachetest-")
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -65,6 +67,8 @@ func TestBasic(t *testing.T) {
}

func TestGrowth(t *testing.T) {
t.Parallel()

dir, err := ioutil.TempDir("", "cachetest-")
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -151,6 +155,8 @@ func dummyID(x int) [HashSize]byte {
}

func TestCacheTrim(t *testing.T) {
t.Parallel()

dir, err := ioutil.TempDir("", "cachetest-")
if err != nil {
t.Fatal(err)
Expand Down
22 changes: 16 additions & 6 deletions pkg/commands/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,32 @@ func (e *Executor) initConfig() {
cmd.AddCommand(pathCmd)
}

func (e *Executor) getUsedConfig() string {
usedConfigFile := viper.ConfigFileUsed()
if usedConfigFile == "" {
return ""
}

prettyUsedConfigFile, err := fsutils.ShortestRelPath(usedConfigFile, "")
if err != nil {
e.log.Warnf("Can't pretty print config file path: %s", err)
return usedConfigFile
}

return prettyUsedConfigFile
}

func (e *Executor) executePathCmd(_ *cobra.Command, args []string) {
if len(args) != 0 {
e.log.Fatalf("Usage: golangci-lint config path")
}

usedConfigFile := viper.ConfigFileUsed()
usedConfigFile := e.getUsedConfig()
if usedConfigFile == "" {
e.log.Warnf("No config file detected")
os.Exit(exitcodes.NoConfigFileDetected)
}

usedConfigFile, err := fsutils.ShortestRelPath(usedConfigFile, "")
if err != nil {
e.log.Warnf("Can't pretty print config file path: %s", err)
}

fmt.Println(usedConfigFile)
os.Exit(0)
}
46 changes: 31 additions & 15 deletions pkg/commands/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import (
"context"
"crypto/sha256"
"encoding/json"
"fmt"
"io"
"os"
"path/filepath"
"strings"
"time"

"github.com/fatih/color"
Expand All @@ -31,8 +31,9 @@ import (
)

type Executor struct {
rootCmd *cobra.Command
runCmd *cobra.Command
rootCmd *cobra.Command
runCmd *cobra.Command
lintersCmd *cobra.Command

exitCode int
version, commit, date string
Expand All @@ -55,6 +56,7 @@ type Executor struct {
}

func NewExecutor(version, commit, date string) *Executor {
startedAt := time.Now()
e := &Executor{
cfg: config.NewDefault(),
version: version,
Expand All @@ -66,9 +68,6 @@ func NewExecutor(version, commit, date string) *Executor {

e.debugf("Starting execution...")
e.log = report.NewLogWrapper(logutils.NewStderrLog(""), &e.reportData)
if ok := e.acquireFileLock(); !ok {
e.log.Fatalf("Parallel golangci-lint is running")
}

// to setup log level early we need to parse config from command line extra time to
// find `-v` option
Expand Down Expand Up @@ -121,6 +120,7 @@ func NewExecutor(version, commit, date string) *Executor {

// Slice options must be explicitly set for proper merging of config and command-line options.
fixSlicesFlags(e.runCmd.Flags())
fixSlicesFlags(e.lintersCmd.Flags())

e.EnabledLintersSet = lintersdb.NewEnabledSet(e.DBManager,
lintersdb.NewValidator(e.DBManager), e.log.Child("lintersdb"), e.cfg)
Expand All @@ -139,7 +139,7 @@ func NewExecutor(version, commit, date string) *Executor {
if err = e.initHashSalt(version); err != nil {
e.log.Fatalf("Failed to init hash salt: %s", err)
}
e.debugf("Initialized executor")
e.debugf("Initialized executor in %s", time.Since(startedAt))
return e
}

Expand Down Expand Up @@ -191,27 +191,39 @@ func computeBinarySalt(version string) ([]byte, error) {
}

func computeConfigSalt(cfg *config.Config) ([]byte, error) {
configBytes, err := json.Marshal(cfg)
// We don't hash all config fields to reduce meaningless cache
// invalidations. At least, it has a huge impact on tests speed.

lintersSettingsBytes, err := json.Marshal(cfg.LintersSettings)
if err != nil {
return nil, errors.Wrap(err, "failed to json marshal config")
return nil, errors.Wrap(err, "failed to json marshal config linter settings")
}

var configData bytes.Buffer
configData.WriteString("linters-settings=")
configData.Write(lintersSettingsBytes)
configData.WriteString("\nbuild-tags=%s" + strings.Join(cfg.Run.BuildTags, ","))

h := sha256.New()
if n, err := h.Write(configBytes); n != len(configBytes) {
return nil, fmt.Errorf("failed to hash config bytes: wrote %d/%d bytes, error: %s", n, len(configBytes), err)
}
h.Write(configData.Bytes()) //nolint:errcheck
return h.Sum(nil), nil
}

func (e *Executor) acquireFileLock() bool {
if e.cfg.Run.AllowParallelRunners {
e.debugf("Parallel runners are allowed, no locking")
return true
}

lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock")
e.debugf("Locking on file %s...", lockFile)
f := flock.New(lockFile)
ctx, finish := context.WithTimeout(context.Background(), time.Minute)
const totalTimeout = 5 * time.Second
const retryDelay = time.Second
ctx, finish := context.WithTimeout(context.Background(), totalTimeout)
defer finish()

timeout := time.Second * 3
if ok, _ := f.TryLockContext(ctx, timeout); !ok {
if ok, _ := f.TryLockContext(ctx, retryDelay); !ok {
return false
}

Expand All @@ -220,6 +232,10 @@ func (e *Executor) acquireFileLock() bool {
}

func (e *Executor) releaseFileLock() {
if e.cfg.Run.AllowParallelRunners {
return
}

if err := e.flock.Unlock(); err != nil {
e.debugf("Failed to unlock on file: %s", err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/commands/linters.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ import (
)

func (e *Executor) initLinters() {
lintersCmd := &cobra.Command{
e.lintersCmd = &cobra.Command{
Use: "linters",
Short: "List current linters configuration",
Run: e.executeLinters,
}
e.rootCmd.AddCommand(lintersCmd)
e.initRunConfiguration(lintersCmd)
e.rootCmd.AddCommand(e.lintersCmd)
e.initRunConfiguration(e.lintersCmd)
}

func (e *Executor) executeLinters(_ *cobra.Command, args []string) {
Expand Down
1 change: 0 additions & 1 deletion pkg/commands/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ func (e *Executor) persistentPostRun(_ *cobra.Command, _ []string) {
trace.Stop()
}

e.releaseFileLock()
os.Exit(e.exitCode)
}

Expand Down
12 changes: 12 additions & 0 deletions pkg/commands/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is
fs.BoolVar(&rc.UseDefaultSkipDirs, "skip-dirs-use-default", true, getDefaultDirectoryExcludeHelp())
fs.StringSliceVar(&rc.SkipFiles, "skip-files", nil, wh("Regexps of files to skip"))

const allowParallelDesc = "Allow multiple parallel golangci-lint instances running. " +
"If false (default) - golangci-lint acquires file lock on start."
fs.BoolVar(&rc.AllowParallelRunners, "allow-parallel-runners", false, wh(allowParallelDesc))

// Linters settings config
lsc := &cfg.LintersSettings

Expand Down Expand Up @@ -251,6 +255,14 @@ func (e *Executor) initRun() {
Use: "run",
Short: welcomeMessage,
Run: e.executeRun,
PreRun: func(_ *cobra.Command, _ []string) {
if ok := e.acquireFileLock(); !ok {
e.log.Fatalf("Parallel golangci-lint is running")
}
},
PostRun: func(_ *cobra.Command, _ []string) {
e.releaseFileLock()
},
}
e.rootCmd.AddCommand(e.runCmd)

Expand Down
2 changes: 2 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ type Run struct {
SkipFiles []string `mapstructure:"skip-files"`
SkipDirs []string `mapstructure:"skip-dirs"`
UseDefaultSkipDirs bool `mapstructure:"skip-dirs-use-default"`

AllowParallelRunners bool `mapstructure:"allow-parallel-runners"`
}

type LintersSettings struct {
Expand Down
8 changes: 7 additions & 1 deletion pkg/lint/lintersdb/enabled_set.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package lintersdb

import (
"os"
"sort"
"strings"

Expand Down Expand Up @@ -29,6 +30,7 @@ func NewEnabledSet(m *Manager, v *Validator, log logutils.Log, cfg *config.Confi
}

func (es EnabledSet) build(lcfg *config.Linters, enabledByDefaultLinters []*linter.Config) map[string]*linter.Config {
es.debugf("Linters config: %#v", lcfg)
resultLintersSet := map[string]*linter.Config{}
switch {
case len(lcfg.Presets) != 0:
Expand Down Expand Up @@ -82,7 +84,11 @@ func (es EnabledSet) GetEnabledLintersMap() (map[string]*linter.Config, error) {
return nil, err
}

return es.build(&es.cfg.Linters, es.m.GetAllEnabledByDefaultLinters()), nil
enabledLinters := es.build(&es.cfg.Linters, es.m.GetAllEnabledByDefaultLinters())
if os.Getenv("GL_TEST_RUN") == "1" {
es.verbosePrintLintersStatus(enabledLinters)
}
return enabledLinters, nil
}

// GetOptimizedLinters returns enabled linters after optimization (merging) of multiple linters
Expand Down
6 changes: 4 additions & 2 deletions test/enabled_linters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,18 +170,20 @@ func TestEnabledLinters(t *testing.T) {
},
}

runner := testshared.NewLintRunner(t)
for _, c := range cases {
c := c
t.Run(c.name, func(t *testing.T) {
t.Parallel()

runArgs := []string{"-v"}
if !c.noImplicitFast {
runArgs = append(runArgs, "--fast")
}
if c.args != "" {
runArgs = append(runArgs, strings.Split(c.args, " ")...)
}
runArgs = append(runArgs, minimalPkg)
r := testshared.NewLintRunner(t).RunWithYamlConfig(c.cfg, runArgs...)
r := runner.RunCommandWithYamlConfig(c.cfg, "linters", runArgs...)
sort.StringSlice(c.el).Sort()

expectedLine := fmt.Sprintf("Active %d linters: [%s]", len(c.el), strings.Join(c.el, " "))
Expand Down
9 changes: 5 additions & 4 deletions test/linters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func runGoErrchk(c *exec.Cmd, files []string, t *testing.T) {
output, err := c.CombinedOutput()
assert.Error(t, err)
_, ok := err.(*exec.ExitError)
assert.True(t, ok)
assert.True(t, ok, err)

// TODO: uncomment after deprecating go1.11
// assert.Equal(t, exitcodes.IssuesFound, exitErr.ExitCode())
Expand Down Expand Up @@ -48,9 +48,9 @@ func testSourcesFromDir(t *testing.T, dir string) {

for _, s := range sources {
s := s
t.Run(filepath.Base(s), func(t *testing.T) {
t.Parallel()
testOneSource(t, s)
t.Run(filepath.Base(s), func(subTest *testing.T) {
subTest.Parallel()
testOneSource(subTest, s)
})
}
}
Expand Down Expand Up @@ -101,6 +101,7 @@ func saveConfig(t *testing.T, cfg map[string]interface{}) (cfgPath string, finis
func testOneSource(t *testing.T, sourcePath string) {
args := []string{
"run",
"--allow-parallel-runners",
"--disable-all",
"--print-issued-lines=false",
"--print-linter-name=false",
Expand Down
Loading

0 comments on commit e47ee39

Please sign in to comment.