From df6c239099224a841a2ec6699f101d8870782912 Mon Sep 17 00:00:00 2001 From: Grzegorz Dlugoszewski Date: Mon, 25 Aug 2025 19:13:09 +0200 Subject: [PATCH 1/3] Add test parallelization and re-enable paralleltest linter --- .golangci.yml | 1 - cmd/main_test.go | 4 ++++ pkg/cfg/config_test.go | 1 + pkg/dump_test.go | 4 ++++ pkg/git/config_test.go | 2 ++ pkg/git/finder_test.go | 7 +++++++ pkg/git/repo.go | 3 +-- pkg/git/repo_test.go | 23 +++++++++++++++++++++++ pkg/url_test.go | 8 ++++++++ 9 files changed, 50 insertions(+), 3 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index b401034..5d0d076 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,7 +19,6 @@ linters: - godox # TODO: enable it and handle all the remaning TODOs - mnd # Impractical. We deal with numbers like file permissions here, it's much clearer to see them explicitly. - noinlineerr # Impractical. Inline error handling is a common and idiomatic practice - - paralleltest # Tests are fast already and paralellizing them adds complexity - testpackage # TODO: renable it and refactor tests into separate packages - unparam # Impractical, it flags functions that are designed to be general-purpose, but happen to only be used with specific values currently - wsl # We use wsl_v5 instead diff --git a/cmd/main_test.go b/cmd/main_test.go index 2570ec0..945703f 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -6,6 +6,7 @@ import ( "testing" ) +//nolint:paralleltest // Tests modifies global state (os.Args) and cannot run in parallel func TestDetermineCommand(t *testing.T) { tests := []struct { name string @@ -102,6 +103,7 @@ func TestDetermineCommand(t *testing.T) { } } +//nolint:paralleltest // Tests modifies global state (os.Args) and cannot run in parallel func TestHandleGitGetInvocation(t *testing.T) { tests := []struct { name string @@ -164,6 +166,7 @@ func TestHandleGitGetInvocation(t *testing.T) { } } +//nolint:paralleltest // Tests modifies global state (os.Args) and cannot run in parallel func TestHandleGitListInvocation(t *testing.T) { tests := []struct { name string @@ -208,6 +211,7 @@ func TestHandleGitListInvocation(t *testing.T) { } } +//nolint:paralleltest // Tests modifies global state (os.Args) and cannot run in parallel func TestHandleDefaultInvocation(t *testing.T) { tests := []struct { name string diff --git a/pkg/cfg/config_test.go b/pkg/cfg/config_test.go index 81563c7..fd79a8b 100644 --- a/pkg/cfg/config_test.go +++ b/pkg/cfg/config_test.go @@ -17,6 +17,7 @@ var ( fromFlag = "value.from.flag" ) +//nolint:paralleltest // These tests modify global state (viper, env vars) and cannot run in parallel func TestConfig(t *testing.T) { tests := []struct { name string diff --git a/pkg/dump_test.go b/pkg/dump_test.go index 2d7c5b7..af12496 100644 --- a/pkg/dump_test.go +++ b/pkg/dump_test.go @@ -5,6 +5,8 @@ import ( ) func TestParsingRefs(t *testing.T) { + t.Parallel() + var tests = []struct { name string line string @@ -39,6 +41,8 @@ func TestParsingRefs(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() + got, err := parseLine(test.line) if err != nil && test.wantErr == nil { t.Fatalf("got error %q", err) diff --git a/pkg/git/config_test.go b/pkg/git/config_test.go index 1af7f14..1ac112f 100644 --- a/pkg/git/config_test.go +++ b/pkg/git/config_test.go @@ -21,6 +21,7 @@ func (c *cfgStub) Get(key string) string { } func TestGitConfig(t *testing.T) { + t.Parallel() tests := []struct { name string configMaker func(t *testing.T) *cfgStub @@ -53,6 +54,7 @@ func TestGitConfig(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() cfg := test.configMaker(t) got := cfg.Get(test.key) diff --git a/pkg/git/finder_test.go b/pkg/git/finder_test.go index b35a65d..9a91872 100644 --- a/pkg/git/finder_test.go +++ b/pkg/git/finder_test.go @@ -9,6 +9,8 @@ import ( ) func TestFinder(t *testing.T) { + t.Parallel() + tests := []struct { name string reposMaker func(*testing.T) string @@ -31,6 +33,7 @@ func TestFinder(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() root := test.reposMaker(t) finder := NewRepoFinder(root) @@ -46,6 +49,8 @@ func TestFinder(t *testing.T) { } func TestExists(t *testing.T) { + t.Parallel() + tests := []struct { name string path string @@ -64,6 +69,8 @@ func TestExists(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() + _, err := Exists(test.path) assert.ErrorIs(t, err, test.want) diff --git a/pkg/git/repo.go b/pkg/git/repo.go index 8095481..055192e 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.go @@ -157,8 +157,7 @@ func (r *Repo) Branches() ([]string, error) { func (r *Repo) Upstream(branch string) (string, error) { out, err := run.Git("rev-parse", "--abbrev-ref", "--symbolic-full-name", branch+"@{upstream}").OnRepo(r.path).AndCaptureLine() if err != nil { - // TODO: no upstream will also throw an error. - // lint:ignore nilerr fix when working on TODO + //nolint:nilerr // TODO: no upstream will also throw an error. return "", nil } diff --git a/pkg/git/repo_test.go b/pkg/git/repo_test.go index 1d806a1..23f1985 100644 --- a/pkg/git/repo_test.go +++ b/pkg/git/repo_test.go @@ -12,6 +12,8 @@ import ( ) func TestUncommitted(t *testing.T) { + t.Parallel() + tests := []struct { name string repoMaker func(*testing.T) *test.Repo @@ -46,6 +48,7 @@ func TestUncommitted(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() r, _ := Open(test.repoMaker(t).Path()) got, err := r.Uncommitted() @@ -60,6 +63,8 @@ func TestUncommitted(t *testing.T) { } } func TestUntracked(t *testing.T) { + t.Parallel() + tests := []struct { name string repoMaker func(*testing.T) *test.Repo @@ -94,6 +99,7 @@ func TestUntracked(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() r, _ := Open(test.repoMaker(t).Path()) got, err := r.Untracked() @@ -109,6 +115,8 @@ func TestUntracked(t *testing.T) { } func TestCurrentBranch(t *testing.T) { + t.Parallel() + tests := []struct { name string repoMaker func(*testing.T) *test.Repo @@ -133,6 +141,7 @@ func TestCurrentBranch(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() r, _ := Open(test.repoMaker(t).Path()) got, err := r.CurrentBranch() @@ -147,6 +156,8 @@ func TestCurrentBranch(t *testing.T) { } } func TestBranches(t *testing.T) { + t.Parallel() + tests := []struct { name string repoMaker func(*testing.T) *test.Repo @@ -176,6 +187,7 @@ func TestBranches(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() r, _ := Open(test.repoMaker(t).Path()) got, err := r.Branches() @@ -190,6 +202,8 @@ func TestBranches(t *testing.T) { } } func TestUpstream(t *testing.T) { + t.Parallel() + tests := []struct { name string repoMaker func(*testing.T) *test.Repo @@ -231,6 +245,7 @@ func TestUpstream(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() r, _ := Open(test.repoMaker(t).Path()) got, _ := r.Upstream(test.branch) @@ -246,6 +261,8 @@ func TestUpstream(t *testing.T) { } } func TestAheadBehind(t *testing.T) { + t.Parallel() + tests := []struct { name string repoMaker func(*testing.T) *test.Repo @@ -281,6 +298,7 @@ func TestAheadBehind(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() r, _ := Open(test.repoMaker(t).Path()) upstream, err := r.Upstream(test.branch) @@ -301,6 +319,7 @@ func TestAheadBehind(t *testing.T) { } func TestCleanupFailedClone(t *testing.T) { + t.Parallel() // Test dir structure: // root // └── a/ @@ -335,6 +354,7 @@ func TestCleanupFailedClone(t *testing.T) { for i, test := range tests { t.Run(strconv.Itoa(i), func(t *testing.T) { + t.Parallel() root := createTestDirTree(t) path := filepath.Join(root, test.path) @@ -354,6 +374,8 @@ func TestCleanupFailedClone(t *testing.T) { } func TestRemote(t *testing.T) { + t.Parallel() + tests := []struct { name string repoMaker func(*testing.T) *test.Repo @@ -382,6 +404,7 @@ func TestRemote(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() r, _ := Open(test.repoMaker(t).Path()) got, err := r.Remote() diff --git a/pkg/url_test.go b/pkg/url_test.go index f9a6700..af65680 100644 --- a/pkg/url_test.go +++ b/pkg/url_test.go @@ -21,6 +21,8 @@ import ( // file:///path/to/repo.git/ func TestURLParse(t *testing.T) { + t.Parallel() + tests := []struct { in string want string @@ -61,6 +63,8 @@ func TestURLParse(t *testing.T) { } } func TestURLParseSkipHost(t *testing.T) { + t.Parallel() + tests := []struct { in string want string @@ -102,6 +106,8 @@ func TestURLParseSkipHost(t *testing.T) { } func TestDefaultScheme(t *testing.T) { + t.Parallel() + tests := []struct { in string scheme string @@ -130,6 +136,8 @@ func TestDefaultScheme(t *testing.T) { } func TestInvalidURLParse(t *testing.T) { + t.Parallel() + invalidURLs := []string{ "", // TODO: This Path is technically a correct scp-like syntax. Not sure how to handle it From 152e91444b4d2e68fa60addb100a33562e41c008 Mon Sep 17 00:00:00 2001 From: Grzegorz Dlugoszewski Date: Mon, 25 Aug 2025 19:24:31 +0200 Subject: [PATCH 2/3] Remove redundant steps from CI workflow - go mod verify is not needed when go.sum is checked in. go build and go test will automatically verify checksums - go mod download is handled by the setup-go action already --- .github/workflows/ci.yml | 6 ------ .golangci.yml | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c84ed6f..d5659ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,12 +31,6 @@ jobs: with: go-version: ${{ matrix.go-version }} cache: true - - - name: Download dependencies - run: go mod download - - - name: Verify dependencies - run: go mod verify - name: Build binary run: go build -v -o bin/git-get ./cmd/ diff --git a/.golangci.yml b/.golangci.yml index 5d0d076..59c2706 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,7 +19,7 @@ linters: - godox # TODO: enable it and handle all the remaning TODOs - mnd # Impractical. We deal with numbers like file permissions here, it's much clearer to see them explicitly. - noinlineerr # Impractical. Inline error handling is a common and idiomatic practice - - testpackage # TODO: renable it and refactor tests into separate packages + - testpackage # Impractical for CLI apps. This linter is more valuable for libraries to ensure a clean public API. - unparam # Impractical, it flags functions that are designed to be general-purpose, but happen to only be used with specific values currently - wsl # We use wsl_v5 instead - wrapcheck # Adds too much bloat, many of the errors are contextual enough and don't need wrapping From 0df82ba272948fb676445ac7184991429cb025b6 Mon Sep 17 00:00:00 2001 From: Grzegorz Dlugoszewski Date: Mon, 25 Aug 2025 20:22:12 +0200 Subject: [PATCH 3/3] Update incorrect module name - Go requires a domain-based path for public modules --- .goreleaser.yml | 8 ++++---- cmd/get.go | 7 ++++--- cmd/list.go | 7 ++++--- go.mod | 2 +- pkg/get.go | 3 ++- pkg/git/config.go | 2 +- pkg/git/config_test.go | 5 +++-- pkg/git/finder_test.go | 3 ++- pkg/git/repo.go | 3 ++- pkg/git/repo_test.go | 3 ++- pkg/git/test/helpers.go | 3 ++- pkg/list.go | 7 ++++--- pkg/url_test.go | 3 ++- 13 files changed, 33 insertions(+), 23 deletions(-) diff --git a/.goreleaser.yml b/.goreleaser.yml index dfe9b07..fc0a2af 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -10,8 +10,8 @@ builds: binary: git-get ldflags: - -s -w - - -X git-get/pkg/cfg.version={{.Version}} - - -X git-get/pkg/cfg.commit={{.Commit}} + - -X github.com/grdl/git-get/pkg/cfg.version={{.Version}} + - -X github.com/grdl/git-get/pkg/cfg.commit={{.Commit}} goos: - linux - windows @@ -26,8 +26,8 @@ builds: binary: git-get ldflags: - -s -w - - -X git-get/pkg/cfg.version={{.Version}} - - -X git-get/pkg/cfg.commit={{.Commit}} + - -X github.com/grdl/git-get/pkg/cfg.version={{.Version}} + - -X github.com/grdl/git-get/pkg/cfg.commit={{.Commit}} goos: - darwin goarch: diff --git a/cmd/get.go b/cmd/get.go index b54f73e..9fc255d 100644 --- a/cmd/get.go +++ b/cmd/get.go @@ -2,11 +2,12 @@ package main import ( "fmt" - "git-get/pkg" - "git-get/pkg/cfg" - "git-get/pkg/git" "os" + "github.com/grdl/git-get/pkg" + "github.com/grdl/git-get/pkg/cfg" + "github.com/grdl/git-get/pkg/git" + "github.com/spf13/cobra" "github.com/spf13/viper" ) diff --git a/cmd/list.go b/cmd/list.go index 3011cc8..1621cf9 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -2,12 +2,13 @@ package main import ( "fmt" - "git-get/pkg" - "git-get/pkg/cfg" - "git-get/pkg/git" "os" "strings" + "github.com/grdl/git-get/pkg" + "github.com/grdl/git-get/pkg/cfg" + "github.com/grdl/git-get/pkg/git" + "github.com/spf13/cobra" "github.com/spf13/viper" ) diff --git a/go.mod b/go.mod index 38eda42..34b902c 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module git-get +module github.com/grdl/git-get go 1.24 diff --git a/pkg/get.go b/pkg/get.go index 6878ebb..7133b3e 100644 --- a/pkg/get.go +++ b/pkg/get.go @@ -3,8 +3,9 @@ package pkg import ( "errors" "fmt" - "git-get/pkg/git" "path/filepath" + + "github.com/grdl/git-get/pkg/git" ) var ErrMissingRepoArg = errors.New("missing argument or --dump flag") diff --git a/pkg/git/config.go b/pkg/git/config.go index 7d29556..601f140 100644 --- a/pkg/git/config.go +++ b/pkg/git/config.go @@ -2,7 +2,7 @@ package git import ( - "git-get/pkg/run" + "github.com/grdl/git-get/pkg/run" ) // ConfigGlobal represents a global gitconfig file. diff --git a/pkg/git/config_test.go b/pkg/git/config_test.go index 1ac112f..364ee5e 100644 --- a/pkg/git/config_test.go +++ b/pkg/git/config_test.go @@ -1,9 +1,10 @@ package git import ( - "git-get/pkg/git/test" - "git-get/pkg/run" "testing" + + "github.com/grdl/git-get/pkg/git/test" + "github.com/grdl/git-get/pkg/run" ) // cfgStub represents a gitconfig file but instead of using a global one, it creates a temporary git repo and uses its local gitconfig. diff --git a/pkg/git/finder_test.go b/pkg/git/finder_test.go index 9a91872..eeadf9e 100644 --- a/pkg/git/finder_test.go +++ b/pkg/git/finder_test.go @@ -1,10 +1,11 @@ package git import ( - "git-get/pkg/git/test" "os" "testing" + "github.com/grdl/git-get/pkg/git/test" + "github.com/stretchr/testify/assert" ) diff --git a/pkg/git/repo.go b/pkg/git/repo.go index 055192e..d6a72a5 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.go @@ -2,12 +2,13 @@ package git import ( "fmt" - "git-get/pkg/run" "net/url" "os" "path/filepath" "strconv" "strings" + + "github.com/grdl/git-get/pkg/run" ) const ( diff --git a/pkg/git/repo_test.go b/pkg/git/repo_test.go index 23f1985..322076b 100644 --- a/pkg/git/repo_test.go +++ b/pkg/git/repo_test.go @@ -1,13 +1,14 @@ package git import ( - "git-get/pkg/git/test" "os" "path/filepath" "reflect" "strconv" "testing" + "github.com/grdl/git-get/pkg/git/test" + "github.com/stretchr/testify/assert" ) diff --git a/pkg/git/test/helpers.go b/pkg/git/test/helpers.go index 582666c..958c576 100644 --- a/pkg/git/test/helpers.go +++ b/pkg/git/test/helpers.go @@ -3,11 +3,12 @@ package test import ( "fmt" - "git-get/pkg/run" "os" "path/filepath" "runtime" "testing" + + "github.com/grdl/git-get/pkg/run" ) // TempDir creates a temporary directory inside the parent dir. diff --git a/pkg/list.go b/pkg/list.go index 9037978..21b30f9 100644 --- a/pkg/list.go +++ b/pkg/list.go @@ -3,10 +3,11 @@ package pkg import ( "errors" "fmt" - "git-get/pkg/cfg" - "git-get/pkg/git" - "git-get/pkg/out" "strings" + + "github.com/grdl/git-get/pkg/cfg" + "github.com/grdl/git-get/pkg/git" + "github.com/grdl/git-get/pkg/out" ) var ErrInvalidOutput = errors.New("invalid output format") diff --git a/pkg/url_test.go b/pkg/url_test.go index af65680..8805a84 100644 --- a/pkg/url_test.go +++ b/pkg/url_test.go @@ -1,9 +1,10 @@ package pkg import ( - "git-get/pkg/cfg" "testing" + "github.com/grdl/git-get/pkg/cfg" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" )