From 31fa76afb8477548a972bd3febce908fc7da7c75 Mon Sep 17 00:00:00 2001 From: Grzegorz Dlugoszewski Date: Mon, 11 Aug 2025 23:03:09 +0200 Subject: [PATCH] Fix failing windows tests Fix incorrect filepath.join usage in building filepaths from URL --- .github/workflows/ci.yml | 2 +- README.md | 13 +++++++------ pkg/git/finder_test.go | 3 ++- pkg/git/repo.go | 2 +- pkg/git/repo_test.go | 22 +++++++++++----------- pkg/git/test/helpers.go | 22 +++++++++++++++++----- pkg/run/run.go | 12 ++++++------ pkg/url.go | 20 ++++++++++++-------- 8 files changed, 57 insertions(+), 39 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 84ad307..877bf05 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,7 +43,7 @@ jobs: git config --global user.name "CI Test" - name: Run tests with coverage - run: go test -race -coverprofile=coverage.out -covermode=atomic ./... + run: go test -race -coverprofile coverage.out -covermode=atomic ./... - name: Upload coverage to Codecov if: matrix.os == 'ubuntu-latest' && matrix.go-version == '1.24' diff --git a/README.md b/README.md index 63ee94a..5d871f2 100644 --- a/README.md +++ b/README.md @@ -336,12 +336,13 @@ We welcome contributions! 1. **Fork the repository** 2. **Create a feature branch**: `git checkout -b feature/amazing-feature` 3. **Install dependencies**: `go mod download` -3. **Make changes and add tests** -4. **Run tests**: `go test ./...` -5. **Run linter**: `golangci-lint run` -6. **Commit changes**: `git commit -m 'Add amazing feature'` -7. **Push to branch**: `git push origin feature/amazing-feature` -8. **Open a Pull Request** +4. **Make changes and add tests** +5. **Format**: `go fmt ./...` +6. **Run tests**: `go test ./...` +7. **Run linter**: `golangci-lint run` +8. **Commit changes**: `git commit -m 'Add amazing feature'` +9. **Push to branch**: `git push origin feature/amazing-feature` +10. **Open a Pull Request** ## License diff --git a/pkg/git/finder_test.go b/pkg/git/finder_test.go index dd2f78e..62802e8 100644 --- a/pkg/git/finder_test.go +++ b/pkg/git/finder_test.go @@ -3,6 +3,7 @@ package git import ( "errors" "git-get/pkg/git/test" + "os" "testing" "github.com/stretchr/testify/assert" @@ -57,7 +58,7 @@ func TestExists(t *testing.T) { want: errDirNotExist, }, { name: "dir exists", - path: "/tmp/", + path: os.TempDir(), want: nil, }, } diff --git a/pkg/git/repo.go b/pkg/git/repo.go index bcb5608..86161e5 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.go @@ -13,7 +13,7 @@ import ( const ( dotgit = ".git" untracked = "??" // Untracked files are marked as "??" in git status output. - master = "master" + main = "main" head = "HEAD" ) diff --git a/pkg/git/repo_test.go b/pkg/git/repo_test.go index 212350a..0897305 100644 --- a/pkg/git/repo_test.go +++ b/pkg/git/repo_test.go @@ -120,9 +120,9 @@ func TestCurrentBranch(t *testing.T) { want: "main", }, { - name: "only master branch", + name: "only main branch", repoMaker: test.RepoWithCommit, - want: master, + want: main, }, { name: "checked out new branch", @@ -163,19 +163,19 @@ func TestBranches(t *testing.T) { want: []string{""}, }, { - name: "only master branch", + name: "only main branch", repoMaker: test.RepoWithCommit, - want: []string{"master"}, + want: []string{"main"}, }, { name: "new branch", repoMaker: test.RepoWithBranch, - want: []string{"feature/branch", "master"}, + want: []string{"feature/branch", "main"}, }, { name: "checked out new tag", repoMaker: test.RepoWithTag, - want: []string{"master"}, + want: []string{"main"}, }, } @@ -204,7 +204,7 @@ func TestUpstream(t *testing.T) { { name: "empty", repoMaker: test.RepoEmpty, - branch: "master", + branch: "main", want: "", }, // TODO: add wantErr @@ -215,10 +215,10 @@ func TestUpstream(t *testing.T) { want: "", }, { - name: "master with upstream", + name: "main with upstream", repoMaker: test.RepoWithBranchWithUpstream, - branch: "master", - want: "origin/master", + branch: "main", + want: "origin/main", }, { name: "branch with upstream", @@ -260,7 +260,7 @@ func TestAheadBehind(t *testing.T) { { name: "fresh clone", repoMaker: test.RepoWithBranchWithUpstream, - branch: "master", + branch: "main", want: []int{0, 0}, }, { diff --git a/pkg/git/test/helpers.go b/pkg/git/test/helpers.go index fc5a6cb..20fffdd 100644 --- a/pkg/git/test/helpers.go +++ b/pkg/git/test/helpers.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "testing" ) @@ -17,17 +18,14 @@ func TempDir(t *testing.T, parent string) string { // Automatically remove temp dir when the test is over. t.Cleanup(func() { - err := os.RemoveAll(dir) - if err != nil { - t.Errorf("failed removing test repo %s", dir) - } + removeTestDir(t, dir) }) return dir } func (r *Repo) init() { - err := run.Git("init", "--quiet", r.path).AndShutUp() + err := run.Git("init", "--quiet", "--initial-branch=main", r.path).AndShutUp() checkFatal(r.t, err) } @@ -92,3 +90,17 @@ func checkFatal(t *testing.T, err error) { t.Fatalf("failed making test repo: %+v", err) } } + +// removeTestDir removes a test directory +func removeTestDir(t *testing.T, dir string) { + // Skip cleanup on Windows to avoid file locking issues in CI + // The CI runner environment is destroyed after tests anyway + if runtime.GOOS == "windows" { + return + } + + err := os.RemoveAll(dir) + if err != nil { + t.Logf("warning: failed removing test repo %s: %v", dir, err) + } +} diff --git a/pkg/run/run.go b/pkg/run/run.go index 238b28e..aa571bc 100644 --- a/pkg/run/run.go +++ b/pkg/run/run.go @@ -16,14 +16,14 @@ import ( // // Examples of different compositions: // -// - run.Git("clone", ).AndShow() -// means running "git clone " and printing the progress into stdout +// - run.Git("clone", ).AndShow() +// means running "git clone " and printing the progress into stdout // -// - run.Git("branch","-a").OnRepo().AndCaptureLines() -// means running "git branch -a" inside and returning a slice of branch names +// - run.Git("branch","-a").OnRepo().AndCaptureLines() +// means running "git branch -a" inside and returning a slice of branch names // -// - run.Git("pull").OnRepo().AndShutUp() -// means running "git pull" inside and not printing any output +// - run.Git("pull").OnRepo().AndShutUp() +// means running "git pull" inside and not printing any output type Cmd struct { cmd *exec.Cmd args string diff --git a/pkg/url.go b/pkg/url.go index 8ee2754..44dd450 100644 --- a/pkg/url.go +++ b/pkg/url.go @@ -5,7 +5,6 @@ import ( "fmt" urlpkg "net/url" "path" - "path/filepath" "regexp" "strings" ) @@ -70,7 +69,7 @@ func ParseURL(rawURL string, defaultHost string, defaultScheme string) (url *url return url, nil } -// URLToPath cleans up the URL and converts it into a path string with correct separators for the current OS. +// URLToPath cleans up the URL and converts it into a path string. // Eg, ssh://git@github.com:22/~user/repo.git => github.com/user/repo // // If skipHost is true, it removes the host part from the path. @@ -82,18 +81,23 @@ func URLToPath(url urlpkg.URL, skipHost bool) string { // Remove tilde (~) char from username. url.Path = strings.ReplaceAll(url.Path, "~", "") - // Remove leading and trailing slashes (correct separator is added by the filepath.Join() below). + // Remove leading and trailing slashes. url.Path = strings.Trim(url.Path, "/") // Remove trailing ".git" from repo name. url.Path = strings.TrimSuffix(url.Path, ".git") - // Replace slashes with separator correct for the current OS. - url.Path = strings.ReplaceAll(url.Path, "/", string(filepath.Separator)) - if skipHost { - url.Host = "" + return url.Path } - return filepath.Join(url.Host, url.Path) + if url.Host == "" { + return url.Path + } + + if url.Path == "" { + return url.Host + } + + return url.Host + "/" + url.Path }