From 8eff169bd871a8954598fe345f96681d656a965a Mon Sep 17 00:00:00 2001 From: Daniel Caballero Date: Tue, 15 Oct 2019 10:02:13 +0200 Subject: [PATCH 1/5] test that demostrates that deadline is not working if comes from the config --- test/run_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/run_test.go b/test/run_test.go index 4c4cf71aab06..ca126255cb53 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -54,6 +54,34 @@ func TestTimeout(t *testing.T) { ExpectOutputContains(`Timeout exceeded: try increase it by passing --timeout option`) } +func TestTimeoutInConfig(t *testing.T) { + type tc struct { + cfg string + } + + cases := []tc{ + { + cfg: ` + run: + deadline: 1ms + `, + }, + { + cfg: ` + run: + timeout: 1ms + `, + }, + } + + r := testshared.NewLintRunner(t) + for _, c := range cases { + // Run with disallowed option set only in config + r.RunWithYamlConfig(c.cfg, withCommonRunArgs(minimalPkg)...).ExpectExitCode(exitcodes.Timeout). + ExpectOutputContains(`Timeout exceeded: try increase it by passing --timeout option`) + } +} + func TestTestsAreLintedByDefault(t *testing.T) { testshared.NewLintRunner(t).Run(getTestDataDir("withtests")). ExpectHasIssue("`if` block ends with a `return`") From 8e360635fdfc6ab08101e2d0ffe20dc031974285 Mon Sep 17 00:00:00 2001 From: Daniel Caballero Date: Tue, 15 Oct 2019 11:50:09 +0200 Subject: [PATCH 2/5] overriding timeout with deadline when only deadline is different from its default value --- pkg/commands/run.go | 15 +++++++++++++-- test/run_test.go | 12 ++++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index a3531c6d5a3c..f0115b0ceae0 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -54,6 +54,8 @@ func wh(text string) string { return color.GreenString(text) } +const defaultTimeout = time.Minute + //nolint:funlen func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, isFinalInit bool) { hideFlag := func(name string) { @@ -87,9 +89,10 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code", exitcodes.IssuesFound, wh("Exit code when issues were found")) fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags")) - fs.DurationVar(&rc.Timeout, "deadline", time.Minute, wh("Deadline for total work")) + + fs.DurationVar(&rc.Timeout, "deadline", defaultTimeout, wh("Deadline for total work")) hideFlag("deadline") - fs.DurationVar(&rc.Timeout, "timeout", time.Minute, wh("Timeout for total work")) + fs.DurationVar(&rc.Timeout, "timeout", defaultTimeout, wh("Timeout for total work")) fs.BoolVar(&rc.AnalyzeTests, "tests", true, wh("Analyze tests (*_test.go)")) fs.BoolVar(&rc.PrintResourcesUsage, "print-resources-usage", false, @@ -397,6 +400,7 @@ func (e *Executor) executeRun(_ *cobra.Command, args []string) { } }() + e.setTimeoutToDeadlineIfOnlyDeadlineIsSet() ctx, cancel := context.WithTimeout(context.Background(), e.cfg.Run.Timeout) defer cancel() @@ -418,6 +422,13 @@ func (e *Executor) executeRun(_ *cobra.Command, args []string) { e.setupExitCode(ctx) } +// to be removed when deadline is finally removed +func (e *Executor) setTimeoutToDeadlineIfOnlyDeadlineIsSet() { + if e.cfg.Run.Deadline != defaultTimeout && e.cfg.Run.Timeout == defaultTimeout { + e.cfg.Run.Timeout = e.cfg.Run.Deadline + } +} + func (e *Executor) setupExitCode(ctx context.Context) { if ctx.Err() != nil { e.exitCode = exitcodes.Timeout diff --git a/test/run_test.go b/test/run_test.go index ca126255cb53..f9f40e20ab7f 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -63,13 +63,21 @@ func TestTimeoutInConfig(t *testing.T) { { cfg: ` run: - deadline: 1ms + deadline: 1ms `, }, { cfg: ` run: - timeout: 1ms + timeout: 1ms + `, + }, + { + // timeout should override deadline + cfg: ` + run: + deadline: 100s + timeout: 1ms `, }, } From d62429170a271f7c18c1fc617e522b8818d5cc4e Mon Sep 17 00:00:00 2001 From: Daniel Caballero Date: Tue, 15 Oct 2019 12:25:47 +0200 Subject: [PATCH 3/5] tests were not passing. default value for Deadline, that now only comes from config, is 0. Plus static check is going to fail because of deprecated cfg used --- pkg/commands/run.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index f0115b0ceae0..454e7410000b 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -422,10 +422,12 @@ func (e *Executor) executeRun(_ *cobra.Command, args []string) { e.setupExitCode(ctx) } -// to be removed when deadline is finally removed +// to be removed when deadline is finally decommissioned func (e *Executor) setTimeoutToDeadlineIfOnlyDeadlineIsSet() { - if e.cfg.Run.Deadline != defaultTimeout && e.cfg.Run.Timeout == defaultTimeout { - e.cfg.Run.Timeout = e.cfg.Run.Deadline + //lint:ignore SA1019 We want to promoted the deprecated config value when needed + deadlineValue := e.cfg.Run.Deadline // nolint: staticcheck + if deadlineValue != 0 && e.cfg.Run.Timeout == defaultTimeout { + e.cfg.Run.Timeout = deadlineValue } } From 78aac4096617483bf857bc269f18a5bfff2c3b7d Mon Sep 17 00:00:00 2001 From: Daniel Caballero Date: Tue, 15 Oct 2019 12:33:44 +0200 Subject: [PATCH 4/5] golangci should use the latest golangci-lint version, that is the one deprecating deadline in favour of timeout --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 4929f25e7f07..2a1eec7a3f7c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -112,6 +112,6 @@ issues: # golangci.com configuration # https://github.com/golangci/golangci/wiki/Configuration service: - golangci-lint-version: 1.19.x # use the fixed version to not introduce new linters unexpectedly + golangci-lint-version: 1.20.x # use the fixed version to not introduce new linters unexpectedly prepare: - echo "here I can run custom commands, but no preparation needed for this repo" From fdd81c9feb4d704e2a05753556c361ed5ad8cc44 Mon Sep 17 00:00:00 2001 From: Daniel Caballero Date: Tue, 15 Oct 2019 12:41:38 +0200 Subject: [PATCH 5/5] README updated - looks the ci config in this project is used to generate usage instructions.. great! --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 29227b2f951f..0ef63c363cd8 100644 --- a/README.md +++ b/README.md @@ -1022,7 +1022,7 @@ issues: # golangci.com configuration # https://github.com/golangci/golangci/wiki/Configuration service: - golangci-lint-version: 1.19.x # use the fixed version to not introduce new linters unexpectedly + golangci-lint-version: 1.20.x # use the fixed version to not introduce new linters unexpectedly prepare: - echo "here I can run custom commands, but no preparation needed for this repo" ```