Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev: fix CI workflow for Windows #3134

Merged
merged 14 commits into from
Aug 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
go.sum linguist-generated
* text=auto eol=lf
*.ps1 text eol=crlf
6 changes: 2 additions & 4 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ env:
GO_VERSION: 1.19

jobs:
# Check if there any dirty change for go mod tidy
# Check if there is any dirty change for go mod tidy
go-mod:
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -41,8 +41,7 @@ jobs:
# ex:
# - 1.18beta1 -> 1.18.0-beta.1
# - 1.18rc1 -> 1.18.0-rc.1
# go-version: ${{ env.GO_VERSION }} # todo(ldez) uncomment after the next release v1.48.0
go-version: 1.18
go-version: ${{ env.GO_VERSION }}
- name: lint
uses: golangci/[email protected]
with:
Expand All @@ -66,7 +65,6 @@ jobs:
go-version: ${{ env.GO_VERSION }} # test only the latest go version to speed up CI
- name: Run tests
run: make.exe test
continue-on-error: true

tests-on-macos:
needs: golangci-lint # run after golangci-lint action to not produce duplicated errors
Expand Down
19 changes: 12 additions & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,22 @@
# enable consistent Go 1.12/1.13 GOPROXY behavior.
export GOPROXY = https://proxy.golang.org

BINARY = golangci-lint
ifeq ($(OS),Windows_NT)
BINARY := $(BINARY).exe
endif

# Build

build: golangci-lint
build: $(BINARY)
.PHONY: build

build_race:
go build -race -o golangci-lint ./cmd/golangci-lint
go build -race -o $(BINARY) ./cmd/golangci-lint
.PHONY: build_race

clean:
rm -f golangci-lint
rm -f $(BINARY)
rm -f test/path
rm -f tools/Dracula.itermcolors
rm -f tools/goreleaser
Expand All @@ -25,7 +30,7 @@ clean:
# Test
test: export GOLANGCI_LINT_INSTALLED = true
test: build
GL_TEST_RUN=1 ./golangci-lint run -v
GL_TEST_RUN=1 ./$(BINARY) run -v
GL_TEST_RUN=1 go test -v -parallel 2 ./...
.PHONY: test

Expand All @@ -36,7 +41,7 @@ test_fix: build
.PHONY: test_fix

test_race: build_race
GL_TEST_RUN=1 ./golangci-lint run -v --timeout=5m
GL_TEST_RUN=1 ./$(BINARY) run -v --timeout=5m
.PHONY: test_race

test_linters:
Expand Down Expand Up @@ -67,7 +72,7 @@ snapshot: .goreleaser.yml tools/goreleaser

# Non-PHONY targets (real files)

golangci-lint: FORCE
$(BINARY): FORCE
go build -o $@ ./cmd/golangci-lint

tools/goreleaser: export GOFLAGS = -mod=readonly
Expand All @@ -87,7 +92,7 @@ tools/Dracula.itermcolors:
assets/demo.svg: tools/svg-term tools/Dracula.itermcolors
./tools/svg-term --cast=183662 --out assets/demo.svg --window --width 110 --height 30 --from 2000 --to 20000 --profile ./tools/Dracula.itermcolors --term iterm2

assets/github-action-config.json: FORCE golangci-lint
assets/github-action-config.json: FORCE $(BINARY)
# go run ./scripts/gen_github_action_config/main.go $@
cd ./scripts/gen_github_action_config/; go run ./main.go ../../$@

Expand Down
16 changes: 0 additions & 16 deletions pkg/result/processors/utils.go → pkg/result/processors/issues.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
package processors

import (
"path/filepath"
"regexp"
"strings"

"github.com/pkg/errors"

"github.com/golangci/golangci-lint/pkg/result"
Expand Down Expand Up @@ -48,15 +44,3 @@ func transformIssues(issues []result.Issue, transform func(i *result.Issue) *res

return retIssues
}

var separatorToReplace = regexp.QuoteMeta(string(filepath.Separator))

func normalizePathInRegex(path string) string {
if filepath.Separator == '/' {
return path
}

// This replacing should be safe because "/" are disallowed in Windows
// https://docs.microsoft.com/ru-ru/windows/win32/fileio/naming-a-file
return strings.ReplaceAll(path, "/", separatorToReplace)
}
4 changes: 2 additions & 2 deletions pkg/result/processors/path_prefixer.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package processors

import (
"path"
"path/filepath"

"github.com/golangci/golangci-lint/pkg/result"
)
Expand All @@ -27,7 +27,7 @@ func (*PathPrefixer) Name() string {
func (p *PathPrefixer) Process(issues []result.Issue) ([]result.Issue, error) {
if p.prefix != "" {
for i := range issues {
issues[i].Pos.Filename = path.Join(p.prefix, issues[i].Pos.Filename)
issues[i].Pos.Filename = filepath.Join(p.prefix, issues[i].Pos.Filename)
}
}
return issues, nil
Expand Down
32 changes: 24 additions & 8 deletions pkg/result/processors/path_prefixer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package processors

import (
"go/token"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/golangci/golangci-lint/pkg/result"
Expand All @@ -12,26 +14,40 @@ import (
func TestPathPrefixer_Process(t *testing.T) {
paths := func(ps ...string) (issues []result.Issue) {
for _, p := range ps {
issues = append(issues, result.Issue{Pos: token.Position{Filename: p}})
issues = append(issues, result.Issue{Pos: token.Position{Filename: filepath.FromSlash(p)}})
}
return
}

for _, tt := range []struct {
name, prefix string
issues, want []result.Issue
}{
{"empty prefix", "", paths("some/path", "cool"), paths("some/path", "cool")},
{"prefix", "ok", paths("some/path", "cool"), paths("ok/some/path", "ok/cool")},
{"prefix slashed", "ok/", paths("some/path", "cool"), paths("ok/some/path", "ok/cool")},
{
name: "empty prefix",
issues: paths("some/path", "cool"),
want: paths("some/path", "cool"),
},
{
name: "prefix",
prefix: "ok",
issues: paths("some/path", "cool"),
want: paths("ok/some/path", "ok/cool"),
},
{
name: "prefix slashed",
prefix: "ok/",
issues: paths("some/path", "cool"),
want: paths("ok/some/path", "ok/cool"),
},
} {
t.Run(tt.name, func(t *testing.T) {
r := require.New(t)

p := NewPathPrefixer(tt.prefix)

got, err := p.Process(tt.issues)
r.NoError(err, "prefixer should never error")
require.NoError(t, err)

r.Equal(got, tt.want)
assert.Equal(t, got, tt.want)
})
}
}
8 changes: 8 additions & 0 deletions pkg/result/processors/path_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//go:build !windows

package processors

// normalizePathInRegex it's a noop function on Unix.
func normalizePathInRegex(path string) string {
return path
}
19 changes: 19 additions & 0 deletions pkg/result/processors/path_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//go:build windows

package processors

import (
"path/filepath"
"regexp"
"strings"
)

var separatorToReplace = regexp.QuoteMeta(string(filepath.Separator))

// normalizePathInRegex normalizes path in regular expressions.
// noop on Unix.
// This replacing should be safe because "/" are disallowed in Windows
// https://docs.microsoft.com/windows/win32/fileio/naming-a-file
func normalizePathInRegex(path string) string {
return strings.ReplaceAll(path, "/", separatorToReplace)
}
3 changes: 2 additions & 1 deletion pkg/result/processors/severity_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ func createSeverityRules(rules []SeverityRule, prefix string) []severityRule {
parsedRule.source = regexp.MustCompile(prefix + rule.Source)
}
if rule.Path != "" {
parsedRule.path = regexp.MustCompile(rule.Path)
path := normalizePathInRegex(rule.Path)
parsedRule.path = regexp.MustCompile(path)
}
parsedRules = append(parsedRules, parsedRule)
}
Expand Down
15 changes: 9 additions & 6 deletions pkg/result/processors/skip_files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package processors

import (
"go/token"
"path/filepath"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -33,14 +35,15 @@ func TestSkipFiles(t *testing.T) {

processAssertEmpty(t, newTestSkipFiles(t, ".*"), newFileIssue("any.go"))

processAssertEmpty(t, newTestSkipFiles(t, "a/b/c.go"), newFileIssue("a/b/c.go"))
processAssertSame(t, newTestSkipFiles(t, "a/b/c.go"), newFileIssue("a/b/d.go"))
cleanPath := strings.ReplaceAll(filepath.FromSlash("a/b/c.go"), `\`, `\\`)
processAssertEmpty(t, newTestSkipFiles(t, cleanPath), newFileIssue(filepath.FromSlash("a/b/c.go")))
processAssertSame(t, newTestSkipFiles(t, cleanPath), newFileIssue(filepath.FromSlash("a/b/d.go")))

processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue("a/b.pb.go"))
processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue("a/b.go"))
processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue(filepath.FromSlash("a/b.pb.go")))
processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue(filepath.FromSlash("a/b.go")))

processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue("a/b.pb.go"))
processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue("a/b.go"))
processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue(filepath.FromSlash("a/b.pb.go")))
processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue(filepath.FromSlash("a/b.go")))
}

func TestSkipFilesInvalidPattern(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions test/fix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
)

func TestFix(t *testing.T) {
testshared.SkipOnWindows(t)

tmpDir := filepath.Join(testdataDir, "fix.tmp")
_ = os.RemoveAll(tmpDir) // cleanup previous runs

Expand Down
2 changes: 2 additions & 0 deletions test/linters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ func TestSourcesFromTestdata(t *testing.T) {
}

func TestTypecheck(t *testing.T) {
testshared.SkipOnWindows(t)

testSourcesFromDir(t, filepath.Join(testdataDir, "notcompiles"))
}

Expand Down
9 changes: 4 additions & 5 deletions test/output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package test
import (
"fmt"
"os"
"path"
"path/filepath"
"testing"

Expand Down Expand Up @@ -50,11 +49,11 @@ func TestOutput_Stderr(t *testing.T) {
Runner().
Install().
Run().
ExpectHasIssue(expectedJSONOutput)
ExpectHasIssue(testshared.NormalizeFilePathInJSON(expectedJSONOutput))
}

func TestOutput_File(t *testing.T) {
resultPath := path.Join(t.TempDir(), "golangci_lint_test_result")
resultPath := filepath.Join(t.TempDir(), "golangci_lint_test_result")

sourcePath := filepath.Join(testdataDir, "misspell.go")

Expand All @@ -74,7 +73,7 @@ func TestOutput_File(t *testing.T) {

b, err := os.ReadFile(resultPath)
require.NoError(t, err)
require.Contains(t, string(b), expectedJSONOutput)
require.Contains(t, string(b), testshared.NormalizeFilePathInJSON(expectedJSONOutput))
}

func TestOutput_Multiple(t *testing.T) {
Expand All @@ -94,5 +93,5 @@ func TestOutput_Multiple(t *testing.T) {
Run().
//nolint:misspell
ExpectHasIssue("testdata/misspell.go:6:38: `occured` is a misspelling of `occurred`").
ExpectOutputContains(expectedJSONOutput)
ExpectOutputContains(testshared.NormalizeFilePathInJSON(expectedJSONOutput))
}
2 changes: 1 addition & 1 deletion test/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestNotExistingDirRun(t *testing.T) {
Run().
ExpectExitCode(exitcodes.Failure).
ExpectOutputContains("cannot find package").
ExpectOutputContains("/testdata/no_such_dir")
ExpectOutputContains(testshared.NormalizeFileInString("/testdata/no_such_dir"))
}

func TestSymlinkLoop(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions test/testdata/depguard_ignore_file_rules.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:build !windows

//golangcitest:args -Edepguard
//golangcitest:config_path testdata/configs/depguard_ignore_file_rules.yml
//golangcitest:expected_exitcode 0
Expand Down
2 changes: 2 additions & 0 deletions test/testdata/ifshort.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:build !windows

//golangcitest:args -Eifshort --internal-cmd-test
package testdata

Expand Down
1 change: 0 additions & 1 deletion test/testdata/tenv_go118.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//go:build go1.18
// +build go1.18

//golangcitest:args -Etenv
package testdata
Expand Down
1 change: 0 additions & 1 deletion test/testdata/thelper_go118.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//go:build go1.18
// +build go1.18

//golangcitest:args -Ethelper
package testdata
Expand Down
2 changes: 1 addition & 1 deletion test/testshared/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func Analyze(t *testing.T, sourcePath string, rawData []byte) {

var reportData jsonResult
err = json.Unmarshal(rawData, &reportData)
require.NoError(t, err)
require.NoError(t, err, string(rawData))

for _, issue := range reportData.Issues {
checkMessage(t, want, issue.Pos, "diagnostic", issue.FromLinter, issue.Text)
Expand Down
Loading