diff --git a/test/bench/bench_test.go b/test/bench/bench_test.go index a6e83212bb93..3e565b9732c1 100644 --- a/test/bench/bench_test.go +++ b/test/bench/bench_test.go @@ -201,7 +201,7 @@ func runOne(b *testing.B, run func(*testing.B), progName string) *runResult { } func BenchmarkGolangciLint(b *testing.B) { - testshared.NewLintRunner(b).Install() + testshared.InstallGolangciLint(b) type bcase struct { name string diff --git a/test/data.go b/test/data.go index 63a6076bb981..217be64c6340 100644 --- a/test/data.go +++ b/test/data.go @@ -5,7 +5,6 @@ import ( ) const testdataDir = "testdata" -const binName = "../golangci-lint" var minimalPkg = getTestDataDir("minimalpkg") diff --git a/test/enabled_linters_test.go b/test/enabled_linters_test.go index 70cb4ffc47c2..0b1f5c56ce84 100644 --- a/test/enabled_linters_test.go +++ b/test/enabled_linters_test.go @@ -23,7 +23,7 @@ func inSlice(s []string, v string) bool { func getEnabledByDefaultFastLintersExcept(except ...string) []string { m := lintersdb.NewManager(nil, nil) ebdl := m.GetAllEnabledByDefaultLinters() - ret := []string{} + var ret []string for _, lc := range ebdl { if lc.IsSlowLinter() { continue @@ -52,7 +52,7 @@ func getAllFastLintersWith(with ...string) []string { func getEnabledByDefaultLinters() []string { ebdl := lintersdb.NewManager(nil, nil).GetAllEnabledByDefaultLinters() - ret := []string{} + var ret []string for _, lc := range ebdl { ret = append(ret, lc.Name()) } @@ -76,15 +76,13 @@ func getEnabledByDefaultFastLintersWith(with ...string) []string { //nolint:funlen func TestEnabledLinters(t *testing.T) { - type tc struct { + cases := []struct { name string cfg string - el []string - args string + enabledLinters []string + args []string noImplicitFast bool - } - - cases := []tc{ + }{ { name: "disable govet in config", cfg: ` @@ -92,7 +90,7 @@ func TestEnabledLinters(t *testing.T) { disable: - govet `, - el: getEnabledByDefaultFastLintersExcept("govet"), + enabledLinters: getEnabledByDefaultFastLintersExcept("govet"), }, { name: "enable golint in config", @@ -101,22 +99,22 @@ func TestEnabledLinters(t *testing.T) { enable: - golint `, - el: getEnabledByDefaultFastLintersWith("golint"), + enabledLinters: getEnabledByDefaultFastLintersWith("golint"), }, { - name: "disable govet in cmd", - args: "-Dgovet", - el: getEnabledByDefaultFastLintersExcept("govet"), + name: "disable govet in cmd", + args: []string{"-Dgovet"}, + enabledLinters: getEnabledByDefaultFastLintersExcept("govet"), }, { name: "enable gofmt in cmd and enable golint in config", - args: "-Egofmt", + args: []string{"-Egofmt"}, cfg: ` linters: enable: - golint `, - el: getEnabledByDefaultFastLintersWith("golint", "gofmt"), + enabledLinters: getEnabledByDefaultFastLintersWith("golint", "gofmt"), }, { name: "fast option in config", @@ -124,7 +122,7 @@ func TestEnabledLinters(t *testing.T) { linters: fast: true `, - el: getEnabledByDefaultFastLintersWith(), + enabledLinters: getEnabledByDefaultFastLintersWith(), noImplicitFast: true, }, { @@ -133,13 +131,13 @@ func TestEnabledLinters(t *testing.T) { linters: fast: false `, - el: getEnabledByDefaultLinters(), + enabledLinters: getEnabledByDefaultLinters(), noImplicitFast: true, }, { name: "set fast option in command-line", - args: "--fast", - el: getEnabledByDefaultFastLintersWith(), + args: []string{"--fast"}, + enabledLinters: getEnabledByDefaultFastLintersWith(), noImplicitFast: true, }, { @@ -148,8 +146,8 @@ func TestEnabledLinters(t *testing.T) { linters: fast: false `, - args: "--fast", - el: getEnabledByDefaultFastLintersWith(), + args: []string{"--fast"}, + enabledLinters: getEnabledByDefaultFastLintersWith(), noImplicitFast: true, }, { @@ -158,36 +156,42 @@ func TestEnabledLinters(t *testing.T) { linters: fast: true `, - args: "--fast=false", - el: getEnabledByDefaultLinters(), + args: []string{"--fast=false"}, + enabledLinters: getEnabledByDefaultLinters(), noImplicitFast: true, }, { name: "fast option combined with enable and enable-all", - args: "--enable-all --fast --enable=unused", - el: getAllFastLintersWith("unused"), + args: []string{"--enable-all", "--fast", "--enable=unused"}, + enabledLinters: getAllFastLintersWith("unused"), noImplicitFast: true, }, } - runner := testshared.NewLintRunner(t) + testshared.InstallGolangciLint(t) + for _, c := range cases { c := c t.Run(c.name, func(t *testing.T) { t.Parallel() - runArgs := []string{"--verbose"} + args := []string{"--verbose"} if !c.noImplicitFast { - runArgs = append(runArgs, "--fast") + args = append(args, "--fast") } - if c.args != "" { - runArgs = append(runArgs, strings.Split(c.args, " ")...) - } - 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, " ")) - r.ExpectOutputContains(expectedLine) + r := testshared.NewRunnerBuilder(t). + WithCommand("linters"). + WithArgs(args...). + WithArgs(c.args...). + WithConfig(c.cfg). + Runner(). + Run() + + sort.StringSlice(c.enabledLinters).Sort() + + r.ExpectOutputContains(fmt.Sprintf("Active %d linters: [%s]", + len(c.enabledLinters), strings.Join(c.enabledLinters, " "))) }) } } diff --git a/test/fix_test.go b/test/fix_test.go index 6b0e27e36db2..f39a96893c52 100644 --- a/test/fix_test.go +++ b/test/fix_test.go @@ -35,36 +35,34 @@ func TestFix(t *testing.T) { err := exec.Command("cp", "-R", fixDir, tmpDir).Run() require.NoError(t, err) + testshared.InstallGolangciLint(t) + inputs := findSources(tmpDir, "in", "*.go") for _, input := range inputs { input := input t.Run(filepath.Base(input), func(t *testing.T) { t.Parallel() - args := []string{ - "--go=1.17", // TODO(ldez): we force to use an old version of Go for the CI and the tests. - "--disable-all", "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number", - "--allow-parallel-runners", "--fix", - input, - } - rc := extractRunContextFromComments(t, input) + rc := testshared.ParseTestDirectives(t, input) if rc == nil { t.Logf("Skipped: %s", input) return } - args = append(args, rc.args...) - - var runResult *testshared.RunResult - if rc.configPath != "" { - args = append(args, "-c", rc.configPath) - runResult = testshared.NewLintRunner(t).RunCommand("run", args...) - } else { - runResult = testshared.NewLintRunner(t).RunWithYamlConfig("", args...) - } + runResult := testshared.NewRunnerBuilder(t). + WithRunContext(rc). + WithTargetPath(input). + WithArgs( + "--disable-all", + "--print-issued-lines=false", + "--print-linter-name=false", + "--out-format=line-number", + "--fix"). + Runner(). + Run() // nolintlint test uses non existing linters (bob, alice) - if rc.expectedLinter != "nolintlint" { + if rc.ExpectedLinter != "nolintlint" { runResult.ExpectExitCode(exitcodes.Success) } diff --git a/test/linters_test.go b/test/linters_test.go index 637067d959d5..c1e49ccf6361 100644 --- a/test/linters_test.go +++ b/test/linters_test.go @@ -1,45 +1,30 @@ package test import ( - "bufio" "fmt" - "go/build/constraint" "os" "os/exec" "path" "path/filepath" - "runtime" - "strings" "testing" - hcversion "github.com/hashicorp/go-version" "github.com/stretchr/testify/require" "github.com/golangci/golangci-lint/pkg/exitcodes" "github.com/golangci/golangci-lint/test/testshared" ) -func runGoErrchk(c *exec.Cmd, defaultExpectedLinter string, files []string, t *testing.T) { - output, err := c.CombinedOutput() - // The returned error will be nil if the test file does not have any issues - // and thus the linter exits with exit code 0. So perform the additional - // assertions only if the error is non-nil. - if err != nil { - var exitErr *exec.ExitError - require.ErrorAs(t, err, &exitErr) - require.Equal(t, exitcodes.IssuesFound, exitErr.ExitCode(), "Unexpected exit code: %s", string(output)) - } - - fullshort := make([]string, 0, len(files)*2) - for _, f := range files { - fullshort = append(fullshort, f, filepath.Base(f)) - } +func TestSourcesFromTestdataWithIssuesDir(t *testing.T) { + testSourcesFromDir(t, testdataDir) +} - err = errorCheck(string(output), false, defaultExpectedLinter, fullshort...) - require.NoError(t, err) +func TestTypecheck(t *testing.T) { + testSourcesFromDir(t, filepath.Join(testdataDir, "notcompiles")) } func testSourcesFromDir(t *testing.T, dir string) { + t.Helper() + t.Log(filepath.Join(dir, "*.go")) findSources := func(pathPatterns ...string) []string { @@ -50,7 +35,7 @@ func testSourcesFromDir(t *testing.T, dir string) { } sources := findSources(dir, "*.go") - testshared.NewLintRunner(t).Install() + testshared.InstallGolangciLint(t) for _, s := range sources { s := s @@ -61,88 +46,92 @@ func testSourcesFromDir(t *testing.T, dir string) { } } -func TestSourcesFromTestdataWithIssuesDir(t *testing.T) { - testSourcesFromDir(t, testdataDir) -} - -func TestTypecheck(t *testing.T) { - testSourcesFromDir(t, filepath.Join(testdataDir, "notcompiles")) -} +func testOneSource(t *testing.T, sourcePath string) { + t.Helper() -func TestGoimportsLocal(t *testing.T) { - sourcePath := filepath.Join(testdataDir, "goimports", "goimports.go") args := []string{ - "--disable-all", "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number", - sourcePath, + "--allow-parallel-runners", + "--disable-all", + "--print-issued-lines=false", + "--out-format=line-number", + "--max-same-issues=100", } - rc := extractRunContextFromComments(t, sourcePath) - require.NotNil(t, rc) - - args = append(args, rc.args...) - - cfg, err := os.ReadFile(rc.configPath) - require.NoError(t, err) - - testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...). - ExpectHasIssue("testdata/goimports/goimports.go:8: File is not `goimports`-ed") -} - -func TestGciLocal(t *testing.T) { - sourcePath := filepath.Join(testdataDir, "gci", "gci.go") - args := []string{ - "--disable-all", "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number", - sourcePath, + rc := testshared.ParseTestDirectives(t, sourcePath) + if rc == nil { + t.Skipf("Skipped: %s", sourcePath) } - rc := extractRunContextFromComments(t, sourcePath) - require.NotNil(t, rc) + for _, addArg := range []string{"", "-Etypecheck"} { + caseArgs := append([]string{}, args...) - args = append(args, rc.args...) + if addArg != "" { + caseArgs = append(caseArgs, addArg) + } - cfg, err := os.ReadFile(rc.configPath) - require.NoError(t, err) + files := []string{sourcePath} + + runner := testshared.NewRunnerBuilder(t). + WithNoParallelRunners(). + WithArgs(caseArgs...). + WithRunContext(rc). + WithTargetPath(sourcePath). + Runner() + + output, err := runner.RawRun() + // The returned error will be nil if the test file does not have any issues + // and thus the linter exits with exit code 0. + // So perform the additional assertions only if the error is non-nil. + if err != nil { + var exitErr *exec.ExitError + require.ErrorAs(t, err, &exitErr) + require.Equal(t, exitcodes.IssuesFound, exitErr.ExitCode(), "Unexpected exit code: %s", string(output)) + } - testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...). - ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed") + fullshort := make([]string, 0, len(files)*2) + for _, f := range files { + fullshort = append(fullshort, f, filepath.Base(f)) + } + + err = errorCheck(string(output), false, rc.ExpectedLinter, fullshort...) + require.NoError(t, err) + } } func TestMultipleOutputs(t *testing.T) { sourcePath := filepath.Join(testdataDir, "gci", "gci.go") - args := []string{ - "--disable-all", "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number,json:stdout", - sourcePath, - } - - rc := extractRunContextFromComments(t, sourcePath) - require.NotNil(t, rc) - - args = append(args, rc.args...) - - cfg, err := os.ReadFile(rc.configPath) - require.NoError(t, err) - testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...). + testshared.NewRunnerBuilder(t). + WithArgs( + "--disable-all", + "--print-issued-lines=false", + "--print-linter-name=false", + "--out-format=line-number,json:stdout", + ). + WithDirectives(sourcePath). + WithTargetPath(sourcePath). + Runner(). + Install(). + Run(). ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed"). ExpectOutputContains(`"Issues":[`) } func TestStderrOutput(t *testing.T) { sourcePath := filepath.Join(testdataDir, "gci", "gci.go") - args := []string{ - "--disable-all", "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number,json:stderr", - sourcePath, - } - - rc := extractRunContextFromComments(t, sourcePath) - require.NotNil(t, rc) - - args = append(args, rc.args...) - cfg, err := os.ReadFile(rc.configPath) - require.NoError(t, err) - - testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...). + testshared.NewRunnerBuilder(t). + WithArgs( + "--disable-all", + "--print-issued-lines=false", + "--print-linter-name=false", + "--out-format=line-number,json:stderr", + ). + WithDirectives(sourcePath). + WithTargetPath(sourcePath). + Runner(). + Install(). + Run(). ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed"). ExpectOutputContains(`"Issues":[`) } @@ -151,21 +140,19 @@ func TestFileOutput(t *testing.T) { resultPath := path.Join(t.TempDir(), "golangci_lint_test_result") sourcePath := filepath.Join(testdataDir, "gci", "gci.go") - args := []string{ - "--disable-all", "--print-issued-lines=false", "--print-linter-name=false", - fmt.Sprintf("--out-format=json:%s,line-number", resultPath), - sourcePath, - } - rc := extractRunContextFromComments(t, sourcePath) - require.NotNil(t, rc) - - args = append(args, rc.args...) - - cfg, err := os.ReadFile(rc.configPath) - require.NoError(t, err) - - testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...). + testshared.NewRunnerBuilder(t). + WithArgs( + "--disable-all", + "--print-issued-lines=false", + "--print-linter-name=false", + fmt.Sprintf("--out-format=json:%s,line-number", resultPath), + ). + WithDirectives(sourcePath). + WithTargetPath(sourcePath). + Runner(). + Install(). + Run(). ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed"). ExpectOutputNotContains(`"Issues":[`) @@ -174,203 +161,98 @@ func TestFileOutput(t *testing.T) { require.Contains(t, string(b), `"Issues":[`) } -func testOneSource(t *testing.T, sourcePath string) { - args := []string{ - "run", - "--go=1.17", // TODO(ldez): we force to use an old version of Go for the CI and the tests. - "--allow-parallel-runners", - "--disable-all", - "--print-issued-lines=false", - "--out-format=line-number", - "--max-same-issues=100", - } - - rc := extractRunContextFromComments(t, sourcePath) - if rc == nil { - t.Skipf("Skipped: %s", sourcePath) - } - - for _, addArg := range []string{"", "-Etypecheck"} { - caseArgs := append([]string{}, args...) - caseArgs = append(caseArgs, rc.args...) - if addArg != "" { - caseArgs = append(caseArgs, addArg) - } - if rc.configPath == "" { - caseArgs = append(caseArgs, "--no-config") - } else { - caseArgs = append(caseArgs, "-c", rc.configPath) - } - - caseArgs = append(caseArgs, sourcePath) - - cmd := exec.Command(binName, caseArgs...) - t.Log(caseArgs) - runGoErrchk(cmd, rc.expectedLinter, []string{sourcePath}, t) - } -} - -type runContext struct { - args []string - configPath string - expectedLinter string -} +func TestLinter_goimports_local(t *testing.T) { + sourcePath := filepath.Join(testdataDir, "goimports", "goimports.go") -func skipMultilineComment(scanner *bufio.Scanner) { - for line := scanner.Text(); !strings.Contains(line, "*/") && scanner.Scan(); { - line = scanner.Text() - } + testshared.NewRunnerBuilder(t). + WithArgs( + "--disable-all", + "--print-issued-lines=false", + "--print-linter-name=false", + "--out-format=line-number", + ). + WithDirectives(sourcePath). + WithTargetPath(sourcePath). + Runner(). + Install(). + Run(). + ExpectHasIssue("testdata/goimports/goimports.go:8: File is not `goimports`-ed") } -//nolint:gocyclo -func extractRunContextFromComments(t *testing.T, sourcePath string) *runContext { - f, err := os.Open(sourcePath) - require.NoError(t, err) - defer f.Close() - - rc := &runContext{} - - scanner := bufio.NewScanner(f) - for scanner.Scan() { - line := scanner.Text() - if strings.HasPrefix(line, "/*") { - skipMultilineComment(scanner) - continue - } - if strings.TrimSpace(line) == "" { - continue - } - if !strings.HasPrefix(line, "//") { - break - } - - if strings.HasPrefix(line, "//go:build") || strings.HasPrefix(line, "// +build") { - parse, err := constraint.Parse(line) - require.NoError(t, err) - - if !parse.Eval(buildTagGoVersion) { - return nil - } - - continue - } - - if !strings.HasPrefix(line, "//golangcitest:") { - require.Failf(t, "invalid prefix of comment line %s", line) - } - - before, after, found := strings.Cut(line, " ") - require.Truef(t, found, "invalid prefix of comment line %s", line) - - after = strings.TrimSpace(after) - - switch before { - case "//golangcitest:args": - require.Nil(t, rc.args) - require.NotEmpty(t, after) - rc.args = strings.Split(after, " ") - continue - - case "//golangcitest:config_path": - require.NotEmpty(t, after) - rc.configPath = after - continue - - case "//golangcitest:expected_linter": - require.NotEmpty(t, after) - rc.expectedLinter = after - continue - - default: - require.Failf(t, "invalid prefix of comment line %s", line) - } - } - - // guess the expected linter if none is specified - if rc.expectedLinter == "" { - for _, arg := range rc.args { - if strings.HasPrefix(arg, "-E") && !strings.Contains(arg, ",") { - require.Empty(t, rc.expectedLinter, "could not infer expected linter for errors because multiple linters are enabled. Please use the `//golangcitest:expected_linter ` directive in your test to indicate the linter-under-test.") //nolint:lll - rc.expectedLinter = arg[2:] - } - } - } +func TestLinter_gci_Local(t *testing.T) { + sourcePath := filepath.Join(testdataDir, "gci", "gci.go") - return rc + testshared.NewRunnerBuilder(t). + WithArgs( + "--disable-all", + "--print-issued-lines=false", + "--print-linter-name=false", + "--out-format=line-number", + ). + WithDirectives(sourcePath). + WithTargetPath(sourcePath). + Runner(). + Install(). + Run(). + ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed") } -func buildTagGoVersion(tag string) bool { - vRuntime, err := hcversion.NewVersion(strings.TrimPrefix(runtime.Version(), "go")) - if err != nil { - return false +// TODO(ldez) need to be converted to a classic linter test. +func TestLinter_tparallel(t *testing.T) { + testCases := []struct { + desc string + sourcePath string + expected func(result *testshared.RunnerResult) + }{ + { + desc: "should fail on missing top-level Parallel()", + sourcePath: filepath.Join(testdataDir, "tparallel", "missing_toplevel_test.go"), + expected: func(result *testshared.RunnerResult) { + result.ExpectHasIssue( + "testdata/tparallel/missing_toplevel_test.go:7:6: TestTopLevel should call t.Parallel on the top level as well as its subtests\n", + ) + }, + }, + { + desc: "should fail on missing subtest Parallel()", + sourcePath: filepath.Join(testdataDir, "tparallel", "missing_subtest_test.go"), + expected: func(result *testshared.RunnerResult) { + result.ExpectHasIssue( + "testdata/tparallel/missing_subtest_test.go:7:6: TestSubtests's subtests should call t.Parallel\n", + ) + }, + }, + { + desc: "should pass on parallel test with no subtests", + sourcePath: filepath.Join(testdataDir, "tparallel", "happy_path_test.go"), + expected: func(result *testshared.RunnerResult) { + result.ExpectNoIssues() + }, + }, } - vTag, err := hcversion.NewVersion(strings.TrimPrefix(tag, "go")) - if err != nil { - return false + testshared.InstallGolangciLint(t) + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + result := testshared.NewRunnerBuilder(t). + WithDirectives(test.sourcePath). + WithArgs( + "--disable-all", + "--enable", + "tparallel", + "--print-issued-lines=false", + "--print-linter-name=false", + "--out-format=line-number", + ). + WithTargetPath(test.sourcePath). + Runner(). + Run() + + test.expected(result) + }) } - - return vRuntime.GreaterThanOrEqual(vTag) -} - -func TestExtractRunContextFromComments(t *testing.T) { - rc := extractRunContextFromComments(t, filepath.Join(testdataDir, "goimports", "goimports.go")) - require.NotNil(t, rc) - require.Equal(t, []string{"-Egoimports"}, rc.args) -} - -func TestTparallel(t *testing.T) { - t.Run("should fail on missing top-level Parallel()", func(t *testing.T) { - sourcePath := filepath.Join(testdataDir, "tparallel", "missing_toplevel_test.go") - args := []string{ - "--disable-all", "--enable", "tparallel", - "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number", - sourcePath, - } - - rc := extractRunContextFromComments(t, sourcePath) - require.NotNil(t, rc) - - args = append(args, rc.args...) - - testshared.NewLintRunner(t).RunWithYamlConfig("", args...). - ExpectHasIssue( - "testdata/tparallel/missing_toplevel_test.go:7:6: TestTopLevel should call t.Parallel on the top level as well as its subtests\n", - ) - }) - - t.Run("should fail on missing subtest Parallel()", func(t *testing.T) { - sourcePath := filepath.Join(testdataDir, "tparallel", "missing_subtest_test.go") - args := []string{ - "--disable-all", "--enable", "tparallel", - "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number", - sourcePath, - } - - rc := extractRunContextFromComments(t, sourcePath) - require.NotNil(t, rc) - - args = append(args, rc.args...) - - testshared.NewLintRunner(t).RunWithYamlConfig("", args...). - ExpectHasIssue( - "testdata/tparallel/missing_subtest_test.go:7:6: TestSubtests's subtests should call t.Parallel\n", - ) - }) - - t.Run("should pass on parallel test with no subtests", func(t *testing.T) { - sourcePath := filepath.Join(testdataDir, "tparallel", "happy_path_test.go") - args := []string{ - "--disable-all", "--enable", "tparallel", - "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number", - sourcePath, - } - - rc := extractRunContextFromComments(t, sourcePath) - require.NotNil(t, rc) - - args = append(args, rc.args...) - - testshared.NewLintRunner(t).RunWithYamlConfig("", args...).ExpectNoIssues() - }) } diff --git a/test/run_test.go b/test/run_test.go index cf1f55496912..da938540a613 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -16,39 +16,66 @@ func getCommonRunArgs() []string { return []string{"--skip-dirs", "testdata_etc/,pkg/golinters/goanalysis/(checker|passes)"} } -func withCommonRunArgs(args ...string) []string { - return append(getCommonRunArgs(), args...) -} - func TestAutogeneratedNoIssues(t *testing.T) { - testshared.NewLintRunner(t).Run(getTestDataDir("autogenerated")).ExpectNoIssues() + testshared.NewRunnerBuilder(t). + WithTargetPath(getTestDataDir("autogenerated")). + Runner(). + Install(). + Run(). + ExpectNoIssues() } func TestEmptyDirRun(t *testing.T) { - testshared.NewLintRunner(t, "GO111MODULE=off").Run(getTestDataDir("nogofiles")). + testshared.NewRunnerBuilder(t). + WithEnviron("GO111MODULE=off"). + WithTargetPath(getTestDataDir("nogofiles")). + Runner(). + Install(). + Run(). ExpectExitCode(exitcodes.NoGoFiles). ExpectOutputContains(": no go files to analyze") } func TestNotExistingDirRun(t *testing.T) { - testshared.NewLintRunner(t, "GO111MODULE=off").Run(getTestDataDir("no_such_dir")). + testshared.NewRunnerBuilder(t). + WithEnviron("GO111MODULE=off"). + WithTargetPath(getTestDataDir("no_such_dir")). + Runner(). + Install(). + Run(). ExpectExitCode(exitcodes.Failure). ExpectOutputContains("cannot find package"). ExpectOutputContains("/testdata/no_such_dir") } func TestSymlinkLoop(t *testing.T) { - testshared.NewLintRunner(t).Run(getTestDataDir("symlink_loop", "...")).ExpectNoIssues() + testshared.NewRunnerBuilder(t). + WithTargetPath(getTestDataDir("symlink_loop", "...")). + Runner(). + Install(). + Run(). + ExpectNoIssues() } +// TODO(ldez): remove this in v2. func TestDeadline(t *testing.T) { - testshared.NewLintRunner(t).Run("--deadline=1ms", getProjectRoot()). + testshared.NewRunnerBuilder(t). + WithArgs("--deadline=1ms"). + WithTargetPath(getProjectRoot()). + Runner(). + Install(). + Run(). ExpectExitCode(exitcodes.Timeout). ExpectOutputContains(`Timeout exceeded: try increasing it by passing --timeout option`) } func TestTimeout(t *testing.T) { - testshared.NewLintRunner(t).Run("--timeout=1ms", getProjectRoot()). + testshared.NewRunnerBuilder(t). + WithArgs("--timeout=1ms"). + WithTargetPath(getProjectRoot()). + Runner(). + Install(). + Run(). ExpectExitCode(exitcodes.Timeout). ExpectOutputContains(`Timeout exceeded: try increasing it by passing --timeout option`) } @@ -81,53 +108,135 @@ func TestTimeoutInConfig(t *testing.T) { }, } - r := testshared.NewLintRunner(t) + testshared.InstallGolangciLint(t) + for _, c := range cases { // Run with disallowed option set only in config - r.RunWithYamlConfig(c.cfg, withCommonRunArgs(minimalPkg)...).ExpectExitCode(exitcodes.Timeout). + testshared.NewRunnerBuilder(t). + WithConfig(c.cfg). + WithArgs(getCommonRunArgs()...). + WithTargetPath(minimalPkg). + Runner(). + Run(). + ExpectExitCode(exitcodes.Timeout). ExpectOutputContains(`Timeout exceeded: try increasing it by passing --timeout option`) } } func TestTestsAreLintedByDefault(t *testing.T) { - testshared.NewLintRunner(t).Run(getTestDataDir("withtests")). + testshared.NewRunnerBuilder(t). + WithTargetPath(getTestDataDir("withtests")). + Runner(). + Install(). + Run(). ExpectHasIssue("don't use `init` function") } func TestCgoOk(t *testing.T) { - testshared.NewLintRunner(t).Run("--no-config", "--enable-all", "-D", "nosnakecase,gci", getTestDataDir("cgo")).ExpectNoIssues() + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs( + "--timeout=3m", + "--enable-all", + "-D", + "nosnakecase,gci", + ). + WithTargetPath(getTestDataDir("cgo")). + Runner(). + Install(). + Run(). + ExpectNoIssues() } func TestCgoWithIssues(t *testing.T) { - r := testshared.NewLintRunner(t) - r.Run("--no-config", "--disable-all", "-Egovet", getTestDataDir("cgo_with_issues")). - ExpectHasIssue("Printf format %t has arg cs of wrong type") - r.Run("--no-config", "--disable-all", "-Estaticcheck", getTestDataDir("cgo_with_issues")). - ExpectHasIssue("SA5009: Printf format %t has arg #1 of wrong type") - r.Run("--no-config", "--disable-all", "-Egofmt", getTestDataDir("cgo_with_issues")). - ExpectHasIssue("File is not `gofmt`-ed with `-s` (gofmt)") - r.Run("--no-config", "--disable-all", "-Erevive", getTestDataDir("cgo_with_issues")). - ExpectHasIssue("indent-error-flow: if block ends with a return statement") + testshared.InstallGolangciLint(t) + + testCases := []struct { + desc string + args []string + targetPath string + expected string + }{ + { + desc: "govet", + args: []string{"--no-config", "--disable-all", "-Egovet"}, + targetPath: getTestDataDir("cgo_with_issues"), + expected: "Printf format %t has arg cs of wrong type", + }, + { + desc: "staticcheck", + args: []string{"--no-config", "--disable-all", "-Estaticcheck"}, + targetPath: getTestDataDir("cgo_with_issues"), + expected: "SA5009: Printf format %t has arg #1 of wrong type", + }, + { + desc: "gofmt", + args: []string{"--no-config", "--disable-all", "-Egofmt"}, + targetPath: getTestDataDir("cgo_with_issues"), + expected: "File is not `gofmt`-ed with `-s` (gofmt)", + }, + { + desc: "revive", + args: []string{"--no-config", "--disable-all", "-Erevive"}, + targetPath: getTestDataDir("cgo_with_issues"), + expected: "indent-error-flow: if block ends with a return statement", + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + testshared.NewRunnerBuilder(t). + WithArgs(test.args...). + WithTargetPath(test.targetPath). + Runner(). + Run(). + ExpectHasIssue(test.expected) + }) + } } func TestUnsafeOk(t *testing.T) { - testshared.NewLintRunner(t).Run("--no-config", "--enable-all", getTestDataDir("unsafe")).ExpectNoIssues() + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs("--enable-all"). + WithTargetPath(getTestDataDir("unsafe")). + Runner(). + Install(). + Run(). + ExpectNoIssues() } func TestGovetCustomFormatter(t *testing.T) { - testshared.NewLintRunner(t).Run(getTestDataDir("govet_custom_formatter")).ExpectNoIssues() + testshared.NewRunnerBuilder(t). + WithTargetPath(getTestDataDir("govet_custom_formatter")). + Runner(). + Install(). + Run(). + ExpectNoIssues() } func TestLineDirectiveProcessedFilesLiteLoading(t *testing.T) { - r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config", - "--exclude-use-default=false", "-Egolint", getTestDataDir("quicktemplate")) - output := strings.Join([]string{ "testdata/quicktemplate/hello.qtpl.go:26:1: exported function `StreamHello` should have comment or be unexported (golint)", "testdata/quicktemplate/hello.qtpl.go:50:1: exported function `Hello` should have comment or be unexported (golint)", "testdata/quicktemplate/hello.qtpl.go:39:1: exported function `WriteHello` should have comment or be unexported (golint)", }, "\n") - r.ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(output + "\n") + + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs( + "--print-issued-lines=false", + "--exclude-use-default=false", + "-Egolint", + ). + WithTargetPath(getTestDataDir("quicktemplate")). + Runner(). + Install(). + Run(). + ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(output + "\n") } func TestSortedResults(t *testing.T) { @@ -136,16 +245,16 @@ func TestSortedResults(t *testing.T) { want string }{ { - "--sort-results=false", - strings.Join([]string{ + opt: "--sort-results=false", + want: strings.Join([]string{ "testdata/sort_results/main.go:12:5: `db` is unused (deadcode)", "testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)", "testdata/sort_results/main.go:8:6: func `returnError` is unused (unused)", }, "\n"), }, { - "--sort-results=true", - strings.Join([]string{ + opt: "--sort-results=true", + want: strings.Join([]string{ "testdata/sort_results/main.go:8:6: func `returnError` is unused (unused)", "testdata/sort_results/main.go:12:5: `db` is unused (deadcode)", "testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)", @@ -155,97 +264,286 @@ func TestSortedResults(t *testing.T) { dir := getTestDataDir("sort_results") - t.Parallel() - for i := range testCases { - test := testCases[i] + testshared.InstallGolangciLint(t) + + for _, test := range testCases { + test := test t.Run(test.opt, func(t *testing.T) { - r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", test.opt, dir) - r.ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(test.want + "\n") + t.Parallel() + + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs("--print-issued-lines=false", test.opt). + WithTargetPath(dir). + Runner(). + Run(). + ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(test.want + "\n") }) } } func TestLineDirectiveProcessedFilesFullLoading(t *testing.T) { - r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config", - "--exclude-use-default=false", "-Egolint,govet", getTestDataDir("quicktemplate")) - output := strings.Join([]string{ "testdata/quicktemplate/hello.qtpl.go:26:1: exported function `StreamHello` should have comment or be unexported (golint)", "testdata/quicktemplate/hello.qtpl.go:50:1: exported function `Hello` should have comment or be unexported (golint)", "testdata/quicktemplate/hello.qtpl.go:39:1: exported function `WriteHello` should have comment or be unexported (golint)", }, "\n") - r.ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(output + "\n") + + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs( + "--print-issued-lines=false", + "--exclude-use-default=false", + "-Egolint,govet", + ). + WithTargetPath(getTestDataDir("quicktemplate")). + Runner(). + Install(). + Run(). + ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(output + "\n") } func TestLintFilesWithLineDirective(t *testing.T) { - r := testshared.NewLintRunner(t) - r.Run("-Edupl", "--disable-all", "--config=testdata/linedirective/dupl.yml", getTestDataDir("linedirective")). - ExpectHasIssue("21-23 lines are duplicate of `testdata/linedirective/hello.go:25-27` (dupl)") - r.Run("-Egofmt", "--disable-all", "--no-config", getTestDataDir("linedirective")). - ExpectHasIssue("File is not `gofmt`-ed with `-s` (gofmt)") - r.Run("-Egoimports", "--disable-all", "--no-config", getTestDataDir("linedirective")). - ExpectHasIssue("File is not `goimports`-ed (goimports)") - r. - Run("-Egomodguard", "--disable-all", "--config=testdata/linedirective/gomodguard.yml", getTestDataDir("linedirective")). - ExpectHasIssue("import of package `github.com/ryancurrah/gomodguard` is blocked because the module is not " + - "in the allowed modules list. (gomodguard)") - r.Run("-Elll", "--disable-all", "--config=testdata/linedirective/lll.yml", getTestDataDir("linedirective")). - ExpectHasIssue("line is 57 characters (lll)") - r.Run("-Emisspell", "--disable-all", "--no-config", getTestDataDir("linedirective")). - ExpectHasIssue("is a misspelling of `language` (misspell)") - r.Run("-Ewsl", "--disable-all", "--no-config", getTestDataDir("linedirective")). - ExpectHasIssue("block should not start with a whitespace (wsl)") + testshared.InstallGolangciLint(t) + + testCases := []struct { + desc string + args []string + configPath string + targetPath string + expected string + }{ + { + desc: "dupl", + args: []string{ + "-Edupl", + "--disable-all", + }, + configPath: "testdata/linedirective/dupl.yml", + targetPath: getTestDataDir("linedirective"), + expected: "21-23 lines are duplicate of `testdata/linedirective/hello.go:25-27` (dupl)", + }, + { + desc: "gofmt", + args: []string{ + "-Egofmt", + "--disable-all", + }, + targetPath: getTestDataDir("linedirective"), + expected: "File is not `gofmt`-ed with `-s` (gofmt)", + }, + { + desc: "goimports", + args: []string{ + "-Egoimports", + "--disable-all", + }, + targetPath: getTestDataDir("linedirective"), + expected: "File is not `goimports`-ed (goimports)", + }, + { + desc: "gomodguard", + args: []string{ + "-Egomodguard", + "--disable-all", + }, + configPath: "testdata/linedirective/gomodguard.yml", + targetPath: getTestDataDir("linedirective"), + expected: "import of package `github.com/ryancurrah/gomodguard` is blocked because the module is not " + + "in the allowed modules list. (gomodguard)", + }, + { + desc: "lll", + args: []string{ + "-Elll", + "--disable-all", + }, + configPath: "testdata/linedirective/lll.yml", + targetPath: getTestDataDir("linedirective"), + expected: "line is 57 characters (lll)", + }, + { + desc: "misspell", + args: []string{ + "-Emisspell", + "--disable-all", + }, + configPath: "", + targetPath: getTestDataDir("linedirective"), + expected: "is a misspelling of `language` (misspell)", + }, + { + desc: "wsl", + args: []string{ + "-Ewsl", + "--disable-all", + }, + configPath: "", + targetPath: getTestDataDir("linedirective"), + expected: "block should not start with a whitespace (wsl)", + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + testshared.NewRunnerBuilder(t). + WithArgs(test.args...). + WithTargetPath(test.targetPath). + WithConfigFile(test.configPath). + Runner(). + Run(). + ExpectHasIssue(test.expected) + }) + } } func TestSkippedDirsNoMatchArg(t *testing.T) { dir := getTestDataDir("skipdirs", "skip_me", "nested") - res := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config", "--skip-dirs", dir, "-Egolint", dir) - res.ExpectExitCode(exitcodes.IssuesFound). + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs( + "--print-issued-lines=false", + "--skip-dirs", dir, + "-Egolint", + ). + WithTargetPath(dir). + Runner(). + Install(). + Run(). + ExpectExitCode(exitcodes.IssuesFound). ExpectOutputEq("testdata/skipdirs/skip_me/nested/with_issue.go:8:9: `if` block ends with " + "a `return` statement, so drop this `else` and outdent its block (golint)\n") } func TestSkippedDirsTestdata(t *testing.T) { - r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config", "-Egolint", getTestDataDir("skipdirs", "...")) - - r.ExpectNoIssues() // all was skipped because in testdata + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs( + "--print-issued-lines=false", + "-Egolint", + ). + WithTargetPath(getTestDataDir("skipdirs", "...")). + Runner(). + Install(). + Run(). + ExpectNoIssues() // all was skipped because in testdata } func TestDeadcodeNoFalsePositivesInMainPkg(t *testing.T) { - testshared.NewLintRunner(t).Run("--no-config", "--disable-all", "-Edeadcode", getTestDataDir("deadcode_main_pkg")).ExpectNoIssues() + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs("--disable-all", "-Edeadcode"). + WithTargetPath(getTestDataDir("deadcode_main_pkg")). + Runner(). + Install(). + Run(). + ExpectNoIssues() } func TestIdentifierUsedOnlyInTests(t *testing.T) { - testshared.NewLintRunner(t).Run("--no-config", "--disable-all", "-Eunused", getTestDataDir("used_only_in_tests")).ExpectNoIssues() + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs("--disable-all", "-Eunused"). + WithTargetPath(getTestDataDir("used_only_in_tests")). + Runner(). + Install(). + Run(). + ExpectNoIssues() } func TestUnusedCheckExported(t *testing.T) { - t.Skip("Issue955") - testshared.NewLintRunner(t).Run("-c", "testdata_etc/unused_exported/golangci.yml", "testdata_etc/unused_exported/...").ExpectNoIssues() + testshared.NewRunnerBuilder(t). + WithConfigFile("testdata_etc/unused_exported/golangci.yml"). + WithTargetPath("testdata_etc/unused_exported/..."). + Runner(). + Install(). + Run(). + ExpectNoIssues() } func TestConfigFileIsDetected(t *testing.T) { - checkGotConfig := func(r *testshared.RunResult) { - r.ExpectExitCode(exitcodes.Success). - ExpectOutputEq("test\n") // test config contains InternalTest: true, it triggers such output + testshared.InstallGolangciLint(t) + + testCases := []struct { + desc string + targetPath string + }{ + { + desc: "explicit", + targetPath: getTestDataDir("withconfig", "pkg"), + }, + { + desc: "recursive", + targetPath: getTestDataDir("withconfig", "..."), + }, } - r := testshared.NewLintRunner(t) - checkGotConfig(r.Run(getTestDataDir("withconfig", "pkg"))) - checkGotConfig(r.Run(getTestDataDir("withconfig", "..."))) + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + testshared.NewRunnerBuilder(t). + // WithNoConfig(). + WithTargetPath(test.targetPath). + Runner(). + Run(). + ExpectExitCode(exitcodes.Success). + // test config contains InternalTest: true, it triggers such output + ExpectOutputEq("test\n") + }) + } } func TestEnableAllFastAndEnableCanCoexist(t *testing.T) { - r := testshared.NewLintRunner(t) - r.Run(withCommonRunArgs("--no-config", "--fast", "--enable-all", "--enable=typecheck", minimalPkg)...). - ExpectExitCode(exitcodes.Success, exitcodes.IssuesFound) - r.Run(withCommonRunArgs("--no-config", "--enable-all", "--enable=typecheck", minimalPkg)...). - ExpectExitCode(exitcodes.Failure) + testshared.InstallGolangciLint(t) + + testCases := []struct { + desc string + args []string + expected []int + }{ + { + desc: "fast", + args: []string{"--fast", "--enable-all", "--enable=typecheck"}, + expected: []int{exitcodes.Success, exitcodes.IssuesFound}, + }, + { + desc: "all", + args: []string{"--enable-all", "--enable=typecheck"}, + expected: []int{exitcodes.Failure}, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs(getCommonRunArgs()...). + WithArgs(test.args...). + WithTargetPath(minimalPkg). + Runner(). + Run(). + ExpectExitCode(test.expected...) + }) + } } func TestEnabledPresetsAreNotDuplicated(t *testing.T) { - testshared.NewLintRunner(t).Run("--no-config", "-v", "-p", "style,bugs", minimalPkg). + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs("-v", "-p", "style,bugs"). + WithTargetPath(minimalPkg). + Runner(). + Install(). + Run(). ExpectOutputContains("Active presets: [bugs style]") } @@ -254,8 +552,17 @@ func TestAbsPathDirAnalysis(t *testing.T) { absDir, err := filepath.Abs(dir) assert.NoError(t, err) - r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config", "-Egolint", absDir) - r.ExpectHasIssue("`if` block ends with a `return` statement") + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs( + "--print-issued-lines=false", + "-Egolint", + ). + WithTargetPath(absDir). + Runner(). + Install(). + Run(). + ExpectHasIssue("`if` block ends with a `return` statement") } func TestAbsPathFileAnalysis(t *testing.T) { @@ -263,8 +570,17 @@ func TestAbsPathFileAnalysis(t *testing.T) { absDir, err := filepath.Abs(dir) assert.NoError(t, err) - r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config", "-Egolint", absDir) - r.ExpectHasIssue("`if` block ends with a `return` statement") + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs( + "--print-issued-lines=false", + "-Egolint", + ). + WithTargetPath(absDir). + Runner(). + Install(). + Run(). + ExpectHasIssue("`if` block ends with a `return` statement") } func TestDisallowedOptionsInConfig(t *testing.T) { @@ -311,40 +627,75 @@ func TestDisallowedOptionsInConfig(t *testing.T) { }, } - r := testshared.NewLintRunner(t) + testshared.InstallGolangciLint(t) + for _, c := range cases { // Run with disallowed option set only in config - r.RunWithYamlConfig(c.cfg, withCommonRunArgs(minimalPkg)...).ExpectExitCode(exitcodes.Failure) + testshared.NewRunnerBuilder(t). + WithConfig(c.cfg). + WithArgs(getCommonRunArgs()...). + WithTargetPath(minimalPkg). + Runner(). + Run(). + ExpectExitCode(exitcodes.Failure) if c.option == "" { continue } - args := []string{c.option, "--fast", minimalPkg} + args := []string{c.option, "--fast"} // Run with disallowed option set only in command-line - r.Run(withCommonRunArgs(args...)...).ExpectExitCode(exitcodes.Success) + testshared.NewRunnerBuilder(t). + WithNoConfig(). + WithArgs(getCommonRunArgs()...). + WithArgs(args...). + WithTargetPath(minimalPkg). + Runner(). + Run(). + ExpectExitCode(exitcodes.Success) // Run with disallowed option set both in command-line and in config - r.RunWithYamlConfig(c.cfg, withCommonRunArgs(args...)...).ExpectExitCode(exitcodes.Failure) + + testshared.NewRunnerBuilder(t). + WithConfig(c.cfg). + WithArgs(getCommonRunArgs()...). + WithArgs(args...). + WithTargetPath(minimalPkg). + Runner(). + Run(). + ExpectExitCode(exitcodes.Failure) } } func TestPathPrefix(t *testing.T) { - for _, tt := range []struct { - Name string - Args []string - Pattern string + testCases := []struct { + desc string + args []string + pattern string }{ - {"empty", nil, "^testdata/withtests/"}, - {"prefixed", []string{"--path-prefix=cool"}, "^cool/testdata/withtests"}, - } { - t.Run(tt.Name, func(t *testing.T) { - testshared.NewLintRunner(t).Run( - append(tt.Args, getTestDataDir("withtests"))..., - ).ExpectOutputRegexp( - tt.Pattern, - ) + { + desc: "empty", + pattern: "^testdata/withtests/", + }, + { + desc: "prefixed", + args: []string{"--path-prefix=cool"}, + pattern: "^cool/testdata/withtests", + }, + } + + testshared.InstallGolangciLint(t) + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + testshared.NewRunnerBuilder(t). + WithArgs(test.args...). + WithTargetPath(getTestDataDir("withtests")). + Runner(). + Run(). + ExpectOutputRegexp(test.pattern) }) } } diff --git a/test/testdata/staticcheck.go b/test/testdata/staticcheck.go index da775618139b..bf1b4e3962d4 100644 --- a/test/testdata/staticcheck.go +++ b/test/testdata/staticcheck.go @@ -3,7 +3,6 @@ package testdata import ( "fmt" - "runtime" ) func Staticcheck() { @@ -22,10 +21,6 @@ func StaticcheckNolintMegacheck() { x = x //nolint:megacheck } -func StaticcheckDeprecated() { - _ = runtime.CPUProfile() // ERROR "SA1019: runtime.CPUProfile has been deprecated .*" -} - func StaticcheckPrintf() { x := "dummy" fmt.Printf("%d", x) // ERROR "SA5009: Printf format %d has arg #1 of wrong type" diff --git a/test/testshared/directives.go b/test/testshared/directives.go new file mode 100644 index 000000000000..5599ad6ab10c --- /dev/null +++ b/test/testshared/directives.go @@ -0,0 +1,121 @@ +package testshared + +import ( + "bufio" + "go/build/constraint" + "os" + "runtime" + "strings" + "testing" + + hcversion "github.com/hashicorp/go-version" + "github.com/stretchr/testify/require" +) + +// RunContext FIXME rename? +type RunContext struct { + Args []string + ConfigPath string + ExpectedLinter string +} + +// ParseTestDirectives parses test directives from sources files. +// +//nolint:gocyclo +func ParseTestDirectives(tb testing.TB, sourcePath string) *RunContext { + tb.Helper() + + f, err := os.Open(sourcePath) + require.NoError(tb, err) + tb.Cleanup(func() { _ = f.Close() }) + + rc := &RunContext{} + + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, "/*") { + skipMultilineComment(scanner) + continue + } + if strings.TrimSpace(line) == "" { + continue + } + if !strings.HasPrefix(line, "//") { + break + } + + if strings.HasPrefix(line, "//go:build") || strings.HasPrefix(line, "// +build") { + parse, err := constraint.Parse(line) + require.NoError(tb, err) + + if !parse.Eval(buildTagGoVersion) { + return nil + } + + continue + } + + if !strings.HasPrefix(line, "//golangcitest:") { + require.Failf(tb, "invalid prefix of comment line %s", line) + } + + before, after, found := strings.Cut(line, " ") + require.Truef(tb, found, "invalid prefix of comment line %s", line) + + after = strings.TrimSpace(after) + + switch before { + case "//golangcitest:args": + require.Nil(tb, rc.Args) + require.NotEmpty(tb, after) + rc.Args = strings.Split(after, " ") + continue + + case "//golangcitest:config_path": + require.NotEmpty(tb, after) + rc.ConfigPath = after + continue + + case "//golangcitest:expected_linter": + require.NotEmpty(tb, after) + rc.ExpectedLinter = after + continue + + default: + require.Failf(tb, "invalid prefix of comment line %s", line) + } + } + + // guess the expected linter if none is specified + if rc.ExpectedLinter == "" { + for _, arg := range rc.Args { + if strings.HasPrefix(arg, "-E") && !strings.Contains(arg, ",") { + require.Empty(tb, rc.ExpectedLinter, "could not infer expected linter for errors because multiple linters are enabled. Please use the `//golangcitest:expected_linter ` directive in your test to indicate the linter-under-test.") //nolint:lll + rc.ExpectedLinter = arg[2:] + } + } + } + + return rc +} + +func skipMultilineComment(scanner *bufio.Scanner) { + for line := scanner.Text(); !strings.Contains(line, "*/") && scanner.Scan(); { + line = scanner.Text() + } +} + +func buildTagGoVersion(tag string) bool { + vRuntime, err := hcversion.NewVersion(strings.TrimPrefix(runtime.Version(), "go")) + if err != nil { + return false + } + + vTag, err := hcversion.NewVersion(strings.TrimPrefix(tag, "go")) + if err != nil { + return false + } + + return vRuntime.GreaterThanOrEqual(vTag) +} diff --git a/test/testshared/directives_test.go b/test/testshared/directives_test.go new file mode 100644 index 000000000000..fa0c62820897 --- /dev/null +++ b/test/testshared/directives_test.go @@ -0,0 +1,20 @@ +package testshared + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseTestDirectives(t *testing.T) { + rc := ParseTestDirectives(t, "./testdata/all.go") + require.NotNil(t, rc) + + expected := &RunContext{ + Args: []string{"-Efoo", "--simple", "--hello=world"}, + ConfigPath: "testdata/example.yml", + ExpectedLinter: "bar", + } + assert.Equal(t, expected, rc) +} diff --git a/test/testshared/runner.go b/test/testshared/runner.go new file mode 100644 index 000000000000..1b872e837cb1 --- /dev/null +++ b/test/testshared/runner.go @@ -0,0 +1,324 @@ +package testshared + +import ( + "os" + "os/exec" + "strings" + "sync" + "syscall" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/golangci/golangci-lint/pkg/exitcodes" + "github.com/golangci/golangci-lint/pkg/logutils" +) + +const binName = "../golangci-lint" + +type RunnerBuilder struct { + tb testing.TB + log logutils.Log + + command string + env []string + + configPath string + noConfig bool + allowParallelRunners bool + args []string + target string +} + +func NewRunnerBuilder(tb testing.TB) *RunnerBuilder { + tb.Helper() + + log := logutils.NewStderrLog("test") + log.SetLevel(logutils.LogLevelInfo) + + return &RunnerBuilder{ + tb: tb, + log: log, + command: "run", + allowParallelRunners: true, + } +} + +func (b *RunnerBuilder) WithCommand(command string) *RunnerBuilder { + b.command = command + + return b +} + +func (b *RunnerBuilder) WithNoConfig() *RunnerBuilder { + b.noConfig = true + + return b +} + +func (b *RunnerBuilder) WithConfigFile(cfgPath string) *RunnerBuilder { + b.configPath = cfgPath + b.noConfig = cfgPath == "" + + return b +} + +func (b *RunnerBuilder) WithConfig(cfg string) *RunnerBuilder { + b.tb.Helper() + + content := strings.ReplaceAll(strings.TrimSpace(cfg), "\t", " ") + + if content == "" { + return b.WithNoConfig() + } + + cfgFile, err := os.CreateTemp("", "golangci_lint_test*.yml") + require.NoError(b.tb, err) + + cfgPath := cfgFile.Name() + b.tb.Cleanup(func() { + if os.Getenv("GL_KEEP_TEMP_FILES") != "1" { + _ = os.Remove(cfgPath) + } + }) + + _, err = cfgFile.WriteString(content) + require.NoError(b.tb, err) + + return b.WithConfigFile(cfgPath) +} + +func (b *RunnerBuilder) WithRunContext(rc *RunContext) *RunnerBuilder { + if rc == nil { + return b + } + + return b.WithConfigFile(rc.ConfigPath).WithArgs(rc.Args...) +} + +func (b *RunnerBuilder) WithDirectives(sourcePath string) *RunnerBuilder { + b.tb.Helper() + + return b.WithRunContext(ParseTestDirectives(b.tb, sourcePath)) +} + +func (b *RunnerBuilder) WithEnviron(environ ...string) *RunnerBuilder { + b.env = environ + + return b +} + +func (b *RunnerBuilder) WithNoParallelRunners() *RunnerBuilder { + b.allowParallelRunners = false + + return b +} + +func (b *RunnerBuilder) WithArgs(args ...string) *RunnerBuilder { + b.args = append(b.args, args...) + + return b +} + +func (b *RunnerBuilder) WithTargetPath(target string) *RunnerBuilder { + b.target = target + + return b +} + +func (b *RunnerBuilder) Runner() *Runner { + b.tb.Helper() + + if b.noConfig && b.configPath != "" { + b.tb.Fatal("--no-config and -c cannot be used at the same time") + } + + arguments := []string{ + "--go=1.17", // TODO(ldez): we force to use an old version of Go for the CI and the tests. + "--internal-cmd-test", + } + + if b.allowParallelRunners { + arguments = append(arguments, "--allow-parallel-runners") + } + + if b.noConfig { + arguments = append(arguments, "--no-config") + } + + if b.configPath != "" { + arguments = append(arguments, "-c", b.configPath) + } + + if len(b.args) != 0 { + arguments = append(arguments, b.args...) + } + + if b.target != "" { + arguments = append(arguments, b.target) + } + + return &Runner{ + log: b.log, + tb: b.tb, + env: b.env, + command: b.command, + args: arguments, + } +} + +type Runner struct { + log logutils.Log + tb testing.TB + + env []string + command string + args []string + + installOnce sync.Once +} + +func (r *Runner) Install() *Runner { + r.tb.Helper() + + r.installOnce.Do(func() { + InstallGolangciLint(r.tb) + }) + + return r +} + +func (r *Runner) Run() *RunnerResult { + r.tb.Helper() + + runArgs := append([]string{r.command}, r.args...) + + defer func(startedAt time.Time) { + r.log.Infof("ran [%s %s] in %s", binName, strings.Join(runArgs, " "), time.Since(startedAt)) + }(time.Now()) + + cmd := exec.Command(binName, runArgs...) + cmd.Env = append(os.Environ(), r.env...) + + out, err := cmd.CombinedOutput() + if err != nil { + if exitError, ok := err.(*exec.ExitError); ok { + if len(exitError.Stderr) != 0 { + r.log.Infof("stderr: %s", exitError.Stderr) + } + + ws := exitError.Sys().(syscall.WaitStatus) + + return &RunnerResult{ + tb: r.tb, + output: string(out), + exitCode: ws.ExitStatus(), + } + } + + r.tb.Errorf("can't get error code from %s", err) + + return nil + } + + // success, exitCode should be 0 if go is ok + ws := cmd.ProcessState.Sys().(syscall.WaitStatus) + + return &RunnerResult{ + tb: r.tb, + output: string(out), + exitCode: ws.ExitStatus(), + } +} + +func (r *Runner) RawRun() ([]byte, error) { + r.tb.Helper() + + runArgs := append([]string{r.command}, r.args...) + + defer func(startedAt time.Time) { + r.log.Infof("ran [../golangci-lint %s] in %s", strings.Join(runArgs, " "), time.Since(startedAt)) + }(time.Now()) + + cmd := exec.Command("../golangci-lint", runArgs...) + cmd.Env = append(os.Environ(), r.env...) + + return cmd.CombinedOutput() +} + +type RunnerResult struct { + tb testing.TB + + output string + exitCode int +} + +func (r *RunnerResult) ExpectNoIssues() { + r.tb.Helper() + + assert.Equal(r.tb, "", r.output, "exit code is %d", r.exitCode) + assert.Equal(r.tb, exitcodes.Success, r.exitCode, "output is %s", r.output) +} + +func (r *RunnerResult) ExpectExitCode(possibleCodes ...int) *RunnerResult { + r.tb.Helper() + + for _, pc := range possibleCodes { + if pc == r.exitCode { + return r + } + } + + assert.Fail(r.tb, "invalid exit code", "exit code (%d) must be one of %v: %s", r.exitCode, possibleCodes, r.output) + return r +} + +// ExpectOutputRegexp can be called with either a string or compiled regexp +func (r *RunnerResult) ExpectOutputRegexp(s interface{}) *RunnerResult { + r.tb.Helper() + + assert.Regexp(r.tb, s, r.output, "exit code is %d", r.exitCode) + return r +} + +func (r *RunnerResult) ExpectOutputContains(s string) *RunnerResult { + r.tb.Helper() + + assert.Contains(r.tb, r.output, s, "exit code is %d", r.exitCode) + return r +} + +func (r *RunnerResult) ExpectOutputNotContains(s string) *RunnerResult { + r.tb.Helper() + + assert.NotContains(r.tb, r.output, s, "exit code is %d", r.exitCode) + return r +} + +func (r *RunnerResult) ExpectOutputEq(s string) *RunnerResult { + r.tb.Helper() + + assert.Equal(r.tb, s, r.output, "exit code is %d", r.exitCode) + return r +} + +func (r *RunnerResult) ExpectHasIssue(issueText string) *RunnerResult { + r.tb.Helper() + + return r.ExpectExitCode(exitcodes.IssuesFound).ExpectOutputContains(issueText) +} + +func InstallGolangciLint(tb testing.TB) { + tb.Helper() + + if os.Getenv("GOLANGCI_LINT_INSTALLED") == "true" { + return + } + + cmd := exec.Command("make", "-C", "..", "build") + + err := cmd.Run() + assert.NoError(tb, err, "Can't go install golangci-lint") +} diff --git a/test/testshared/runner_test.go b/test/testshared/runner_test.go new file mode 100644 index 000000000000..36c1e347dacd --- /dev/null +++ b/test/testshared/runner_test.go @@ -0,0 +1,225 @@ +package testshared + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/golangci/golangci-lint/pkg/exitcodes" +) + +//nolint:funlen +func TestRunnerBuilder_Runner(t *testing.T) { + testCases := []struct { + desc string + builder *RunnerBuilder + expected *Runner + }{ + { + desc: "default", + builder: NewRunnerBuilder(t), + expected: &Runner{ + env: []string(nil), + command: "run", + args: []string{ + "--go=1.17", + "--internal-cmd-test", + "--allow-parallel-runners", + }, + }, + }, + { + desc: "with command", + builder: NewRunnerBuilder(t).WithCommand("example"), + expected: &Runner{ + env: []string(nil), + command: "example", + args: []string{ + "--go=1.17", + "--internal-cmd-test", + "--allow-parallel-runners", + }, + }, + }, + { + desc: "with no-config", + builder: NewRunnerBuilder(t).WithNoConfig(), + expected: &Runner{ + env: []string(nil), + command: "run", + args: []string{ + "--go=1.17", + "--internal-cmd-test", + "--allow-parallel-runners", + "--no-config", + }, + }, + }, + { + desc: "with config file", + builder: NewRunnerBuilder(t).WithConfigFile("./testdata/example.yml"), + expected: &Runner{ + env: []string(nil), + command: "run", + args: []string{ + "--go=1.17", + "--internal-cmd-test", + "--allow-parallel-runners", + "-c", + "./testdata/example.yml", + }, + }, + }, + { + desc: "with directives", + builder: NewRunnerBuilder(t).WithDirectives("./testdata/all.go"), + expected: &Runner{ + env: []string(nil), + command: "run", + args: []string{ + "--go=1.17", + "--internal-cmd-test", + "--allow-parallel-runners", + "-c", + "testdata/example.yml", + "-Efoo", + "--simple", + "--hello=world", + }, + }, + }, + { + desc: "with environ", + builder: NewRunnerBuilder(t).WithEnviron("FOO=BAR", "FII=BIR"), + expected: &Runner{ + env: []string{"FOO=BAR", "FII=BIR"}, + command: "run", + args: []string{ + "--go=1.17", + "--internal-cmd-test", + "--allow-parallel-runners", + }, + }, + }, + { + desc: "with no parallel runners", + builder: NewRunnerBuilder(t).WithNoParallelRunners(), + expected: &Runner{ + env: []string(nil), + command: "run", + args: []string{ + "--go=1.17", + "--internal-cmd-test", + }, + }, + }, + { + desc: "with args", + builder: NewRunnerBuilder(t).WithArgs("-Efoo", "--simple", "--hello=world"), + expected: &Runner{ + env: []string(nil), + command: "run", + args: []string{ + "--go=1.17", + "--internal-cmd-test", + "--allow-parallel-runners", + "-Efoo", + "--simple", + "--hello=world", + }, + }, + }, + { + desc: "with target path", + builder: NewRunnerBuilder(t).WithTargetPath("./testdata/all.go"), + expected: &Runner{ + env: []string(nil), + command: "run", + args: []string{ + "--go=1.17", + "--internal-cmd-test", + "--allow-parallel-runners", + "./testdata/all.go", + }, + }, + }, + { + desc: "with RunContext (directives)", + builder: NewRunnerBuilder(t). + WithRunContext(&RunContext{ + Args: []string{"-Efoo", "--simple", "--hello=world"}, + ConfigPath: "testdata/example.yml", + ExpectedLinter: "test", + }), + expected: &Runner{ + env: []string(nil), + command: "run", + args: []string{ + "--go=1.17", + "--internal-cmd-test", + "--allow-parallel-runners", + "-c", + "testdata/example.yml", + "-Efoo", + "--simple", + "--hello=world", + }, + }, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + runner := test.builder.Runner() + + assert.NotNil(t, runner.log) + assert.NotNil(t, runner.tb) + assert.Equal(t, test.expected.env, runner.env) + assert.Equal(t, test.expected.env, runner.env) + assert.Equal(t, test.expected.env, runner.env) + assert.Equal(t, test.expected.command, runner.command) + assert.Equal(t, test.expected.args, runner.args) + }) + } +} + +func TestRunnerResult_ExpectExitCode(t *testing.T) { + r := &RunnerResult{tb: t, exitCode: exitcodes.Success} + r.ExpectExitCode(exitcodes.Failure, exitcodes.Success) +} + +func TestRunnerResult_ExpectNoIssues(t *testing.T) { + r := &RunnerResult{tb: t} + r.ExpectNoIssues() +} + +func TestRunnerResult_ExpectOutputContains(t *testing.T) { + r := &RunnerResult{tb: t, output: "this is an output"} + r.ExpectOutputContains("an") +} + +func TestRunnerResult_ExpectHasIssue(t *testing.T) { + r := &RunnerResult{tb: t, exitCode: exitcodes.IssuesFound, output: "this is an output"} + r.ExpectHasIssue("an") +} + +func TestRunnerResult_ExpectOutputEq(t *testing.T) { + r := &RunnerResult{tb: t, output: "this is an output"} + r.ExpectOutputEq("this is an output") +} + +func TestRunnerResult_ExpectOutputNotContains(t *testing.T) { + r := &RunnerResult{tb: t, output: "this is an output"} + r.ExpectOutputNotContains("one") +} + +func TestRunnerResult_ExpectOutputRegexp(t *testing.T) { + r := &RunnerResult{tb: t, output: "this is an output"} + r.ExpectOutputRegexp(regexp.MustCompile(`an.+`)) + r.ExpectOutputRegexp(`an.+`) + r.ExpectOutputRegexp("an") +} diff --git a/test/testshared/testdata/all.go b/test/testshared/testdata/all.go new file mode 100644 index 000000000000..ca997786281b --- /dev/null +++ b/test/testshared/testdata/all.go @@ -0,0 +1,10 @@ +//golangcitest:args -Efoo --simple --hello=world +//golangcitest:config_path testdata/example.yml +//golangcitest:expected_linter bar +package testdata + +import "fmt" + +func main() { + fmt.Println("Hello") +} diff --git a/test/testshared/testshared.go b/test/testshared/testshared.go deleted file mode 100644 index e3c55cd0ec28..000000000000 --- a/test/testshared/testshared.go +++ /dev/null @@ -1,163 +0,0 @@ -package testshared - -import ( - "os" - "os/exec" - "strings" - "sync" - "syscall" - "time" - - "github.com/stretchr/testify/assert" - - "github.com/golangci/golangci-lint/pkg/exitcodes" - "github.com/golangci/golangci-lint/pkg/logutils" -) - -type LintRunner struct { - t assert.TestingT - log logutils.Log - env []string - installOnce sync.Once -} - -func NewLintRunner(t assert.TestingT, environ ...string) *LintRunner { - log := logutils.NewStderrLog("test") - log.SetLevel(logutils.LogLevelInfo) - return &LintRunner{ - t: t, - log: log, - env: environ, - } -} - -func (r *LintRunner) Install() { - r.installOnce.Do(func() { - if os.Getenv("GOLANGCI_LINT_INSTALLED") == "true" { - return - } - - cmd := exec.Command("make", "-C", "..", "build") - assert.NoError(r.t, cmd.Run(), "Can't go install golangci-lint") - }) -} - -type RunResult struct { - t assert.TestingT - - output string - exitCode int -} - -func (r *RunResult) ExpectNoIssues() { - assert.Equal(r.t, "", r.output, "exit code is %d", r.exitCode) - assert.Equal(r.t, exitcodes.Success, r.exitCode, "output is %s", r.output) -} - -func (r *RunResult) ExpectExitCode(possibleCodes ...int) *RunResult { - for _, pc := range possibleCodes { - if pc == r.exitCode { - return r - } - } - - assert.Fail(r.t, "invalid exit code", "exit code (%d) must be one of %v: %s", r.exitCode, possibleCodes, r.output) - return r -} - -// ExpectOutputRegexp can be called with either a string or compiled regexp -func (r *RunResult) ExpectOutputRegexp(s interface{}) *RunResult { - assert.Regexp(r.t, s, r.output, "exit code is %d", r.exitCode) - return r -} - -func (r *RunResult) ExpectOutputContains(s string) *RunResult { - assert.Contains(r.t, r.output, s, "exit code is %d", r.exitCode) - return r -} - -func (r *RunResult) ExpectOutputNotContains(s string) *RunResult { - assert.NotContains(r.t, r.output, s, "exit code is %d", r.exitCode) - return r -} - -func (r *RunResult) ExpectOutputEq(s string) *RunResult { - assert.Equal(r.t, s, r.output, "exit code is %d", r.exitCode) - return r -} - -func (r *RunResult) ExpectHasIssue(issueText string) *RunResult { - return r.ExpectExitCode(exitcodes.IssuesFound).ExpectOutputContains(issueText) -} - -func (r *LintRunner) Run(args ...string) *RunResult { - newArgs := append([]string{"--allow-parallel-runners"}, args...) - return r.RunCommand("run", newArgs...) -} - -func (r *LintRunner) RunCommand(command string, args ...string) *RunResult { - r.Install() - - runArgs := append([]string{command}, - "--go=1.17", // TODO(ldez): we force to use an old version of Go for the CI and the tests. - "--internal-cmd-test", - ) - runArgs = append(runArgs, args...) - - defer func(startedAt time.Time) { - r.log.Infof("ran [../golangci-lint %s] in %s", strings.Join(runArgs, " "), time.Since(startedAt)) - }(time.Now()) - - cmd := exec.Command("../golangci-lint", runArgs...) - cmd.Env = append(os.Environ(), r.env...) - out, err := cmd.CombinedOutput() - if err != nil { - if exitError, ok := err.(*exec.ExitError); ok { - r.log.Infof("stderr: %s", exitError.Stderr) - ws := exitError.Sys().(syscall.WaitStatus) - return &RunResult{ - t: r.t, - output: string(out), - exitCode: ws.ExitStatus(), - } - } - - r.t.Errorf("can't get error code from %s", err) - return nil - } - - // success, exitCode should be 0 if go is ok - ws := cmd.ProcessState.Sys().(syscall.WaitStatus) - return &RunResult{ - t: r.t, - output: string(out), - exitCode: ws.ExitStatus(), - } -} - -func (r *LintRunner) RunWithYamlConfig(cfg string, args ...string) *RunResult { - newArgs := append([]string{"--allow-parallel-runners"}, args...) - return r.RunCommandWithYamlConfig(cfg, "run", newArgs...) -} - -func (r *LintRunner) RunCommandWithYamlConfig(cfg, command string, args ...string) *RunResult { - f, err := os.CreateTemp("", "golangci_lint_test") - assert.NoError(r.t, err) - f.Close() - - cfgPath := f.Name() + ".yml" - err = os.Rename(f.Name(), cfgPath) - assert.NoError(r.t, err) - - if os.Getenv("GL_KEEP_TEMP_FILES") != "1" { - defer os.Remove(cfgPath) - } - - cfg = strings.ReplaceAll(strings.TrimSpace(cfg), "\t", " ") - - err = os.WriteFile(cfgPath, []byte(cfg), os.ModePerm) - assert.NoError(r.t, err) - - pargs := append([]string{"-c", cfgPath}, args...) - return r.RunCommand(command, pargs...) -}