Merge pull request #34 from grdl/tests-refactor

Add test parallelization and re-enable paralleltest linter

.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/

.golangci.yml

@@ -19,8 +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
-    - paralleltest        # Tests are fast already and paralellizing them adds complexity
-    - 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

.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:

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"
 )

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"
 )

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

go.mod

@@ -1,4 +1,4 @@
-module git-get
+module github.com/grdl/git-get
 
 go 1.24
 

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

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)

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 <REPO> argument or --dump flag")

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.

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.
@@ -21,6 +22,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 +55,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)

pkg/git/finder_test.go

@@ -1,14 +1,17 @@
 package git
 
 import (
-	"git-get/pkg/git/test"
 	"os"
 	"testing"
 
+	"github.com/grdl/git-get/pkg/git/test"
+
 	"github.com/stretchr/testify/assert"
 )
 
 func TestFinder(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		name       string
 		reposMaker func(*testing.T) string
@@ -31,6 +34,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 +50,8 @@ func TestFinder(t *testing.T) {
 }
 
 func TestExists(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		name string
 		path string
@@ -64,6 +70,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)

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 (
@@ -157,8 +158,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
 	}
 

pkg/git/repo_test.go

@@ -1,17 +1,20 @@
 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"
 )
 
 func TestUncommitted(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		name      string
 		repoMaker func(*testing.T) *test.Repo
@@ -46,6 +49,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 +64,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 +100,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 +116,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 +142,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 +157,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 +188,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 +203,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 +246,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 +262,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 +299,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 +320,7 @@ func TestAheadBehind(t *testing.T) {
 }
 
 func TestCleanupFailedClone(t *testing.T) {
+	t.Parallel()
 	// Test dir structure:
 	// root
 	// └── a/
@@ -335,6 +355,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 +375,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 +405,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()
 

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.

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")

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"
 )
@@ -21,6 +22,8 @@ import (
 // file:///path/to/repo.git/
 
 func TestURLParse(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		in   string
 		want string
@@ -61,6 +64,8 @@ func TestURLParse(t *testing.T) {
 	}
 }
 func TestURLParseSkipHost(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		in   string
 		want string
@@ -102,6 +107,8 @@ func TestURLParseSkipHost(t *testing.T) {
 }
 
 func TestDefaultScheme(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		in     string
 		scheme string
@@ -130,6 +137,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