Modernize codebase: Fix linting issues, update CI/CD workflow, and improve test categorization (#549)

* Fix linter failures

* Lint in modern Go

* Update github workflows

* Fix linting

* Remove io/ioutil

* Modern Go code

* Only main

* Verbose unit tests

* Split tests into integration and unit with short flag

* Don't use integration flag

* Use t.TempDir instead of custom temp dirs

* Remove more manual temp dirs

* Fix Hg tests

* Remove mercurial stuff

* Cleanup hg/mercurial messy test

* Upgrade to Go 1.24, remove 1.23 validation

* Remove ancient go mod download

* Fix caching

Author: YakDriver

.github/workflows/go-getter.yml

@@ -1,22 +1,24 @@
 name: go-getter
 
-on: [push]
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
 
 env:
   TEST_RESULTS_PATH: /tmp/test-results
 
 jobs:
 
-  linux-tests:
+  # Basic validation that runs on both PRs and pushes (safe operations)
+  basic-validation:
     runs-on: ubuntu-latest
     strategy:
       matrix:
         go-version:
-          - 1.18
-          - 1.19
-    permissions:
-      id-token: write
-      contents: read
+          - "1.24"
+          - "1.25"
     steps:
       - name: Setup go
         uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0
@@ -26,23 +28,17 @@ jobs:
       - name: Checkout code
         uses: actions/checkout@ff7abcd0c3c05ccf6adc123a8cd1fd4fb30fb493 # v4.0.0
 
-      - name: Create test directory
-        run: |
-          mkdir -p ${{ env.TEST_RESULTS_PATH }}
-
       - name: Setup cache for go modules
         uses: actions/cache@0400d5f644dc74513175e3cd8d07132dd4860809 # v4.2.4
         with:
           path: |
             ~/.cache/go-build
             ~/go/pkg/mod
-          key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
+          key: ${{ runner.os }}-go${{ matrix.go-version }}-${{ hashFiles('**/go.sum') }}
           restore-keys: |
+            ${{ runner.os }}-go${{ matrix.go-version }}-
             ${{ runner.os }}-go-
 
-      - name: Download go modules
-        run: go mod download
-
       # Check go fmt output because it does not report non-zero when there are fmt changes
       - name: Run gofmt
         run: |
@@ -54,6 +50,74 @@ jobs:
               exit 1
             fi
 
+      - name: Build all packages
+        run: go build ./...
+
+      - name: Run unit tests (without cloud integration)
+        run: go test -short -v ./...
+
+  # Linter runs on both PRs and pushes (safe operation)
+  linter:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@ff7abcd0c3c05ccf6adc123a8cd1fd4fb30fb493 # v4.0.0
+      
+      - name: Setup go
+        uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0
+        with:
+          go-version: "1.25" # Use latest for linting
+      
+      - name: Setup cache for go modules
+        uses: actions/cache@0400d5f644dc74513175e3cd8d07132dd4860809 # v4.2.4
+        with:
+          path: |
+            ~/.cache/go-build
+            ~/go/pkg/mod
+            ~/.cache/golangci-lint
+          key: ${{ runner.os }}-lint-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ runner.os }}-lint-
+      
+      - name: Lint code
+        uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0
+
+  # Full integration tests with cloud resources (only on push to protected branches)
+  linux-integration-tests:
+    runs-on: ubuntu-latest
+    if: github.event_name == 'push'
+    strategy:
+      matrix:
+        go-version:
+          - "1.24"
+          - "1.25"
+    permissions:
+      id-token: write
+      contents: read
+    steps:
+      - name: Setup go
+        uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0
+        with:
+          go-version: ${{ matrix.go-version }}
+
+      - name: Checkout code
+        uses: actions/checkout@ff7abcd0c3c05ccf6adc123a8cd1fd4fb30fb493 # v4.0.0
+
+      - name: Create test directory
+        run: |
+          mkdir -p ${{ env.TEST_RESULTS_PATH }}
+
+      - name: Setup cache for go modules
+        uses: actions/cache@0400d5f644dc74513175e3cd8d07132dd4860809 # v4.2.4
+        with:
+          path: |
+            ~/.cache/go-build
+            ~/go/pkg/mod
+          key: ${{ runner.os }}-go${{ matrix.go-version }}-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ runner.os }}-go${{ matrix.go-version }}-
+            ${{ runner.os }}-go-
+
       - name: Install gotestsum
         run: go install gotest.tools/gotestsum@v1.8.2
 
@@ -86,13 +150,14 @@ jobs:
           name: linux-test-results-${{ matrix.go-version }}
           path: linux_cov.part
 
-  windows-tests:
+  windows-integration-tests:
     runs-on: windows-latest
+    if: github.event_name == 'push'
     strategy:
       matrix:
         go-version:
-          - 1.18
-          - 1.19
+          - "1.24"
+          - "1.25"
     permissions:
       id-token: write
       contents: read
@@ -114,13 +179,11 @@ jobs:
           path: |
             ~\AppData\Local\go-build
             ~\go\pkg\mod
-          key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
+          key: ${{ runner.os }}-go${{ matrix.go-version }}-${{ hashFiles('**/go.sum') }}
           restore-keys: |
+            ${{ runner.os }}-go${{ matrix.go-version }}-
             ${{ runner.os }}-go-
 
-      - name: Download go modules
-        run: go mod download
-
       - name: Install gotestsum
         shell: bash
         run: go install gotest.tools/gotestsum@v1.8.2
@@ -154,11 +217,3 @@ jobs:
         with:
           name: windows-test-results-${{ matrix.go-version }}
           path: win_cov.part
-
-  linter:
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout code
-        uses: actions/checkout@ff7abcd0c3c05ccf6adc123a8cd1fd4fb30fb493 # v4.0.0
-      - name: Lint code
-        uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0

client.go

@@ -7,7 +7,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -205,7 +204,7 @@ func (c *Client) Get() error {
 	if decompressor != nil {
 		// Create a temporary directory to store our archive. We delete
 		// this at the end of everything.
-		td, err := ioutil.TempDir("", "getter")
+		td, err := os.MkdirTemp("", "getter")
 		if err != nil {
 			return fmt.Errorf(
 				"Error creating temporary directory for archive: %s", err)

client_option_progress_test.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
-	"os"
 	"path/filepath"
 	"sync"
 	"testing"
@@ -40,17 +39,15 @@ func TestGet_progress(t *testing.T) {
 	defer s.Close()
 
 	{ // dl without tracking
-		dst := tempTestFile(t)
-		defer func() { _ = os.RemoveAll(filepath.Dir(dst)) }()
+		dst := filepath.Join(t.TempDir(), "test-file")
 		if err := GetFile(dst, s.URL+"/file?thig=this&that"); err != nil {
 			t.Fatalf("download failed: %v", err)
 		}
 	}
 
 	{ // tracking
 		p := &MockProgressTracking{}
-		dst := tempTestFile(t)
-		defer func() { _ = os.RemoveAll(filepath.Dir(dst)) }()
+		dst := filepath.Join(t.TempDir(), "test-file")
 		if err := GetFile(dst, s.URL+"/file?thig=this&that", WithProgress(p)); err != nil {
 			t.Fatalf("download failed: %v", err)
 		}

common.go

@@ -4,11 +4,11 @@
 package getter
 
 import (
-	"io/ioutil"
+	"os"
 )
 
 func tmpFile(dir, pattern string) (string, error) {
-	f, err := ioutil.TempFile(dir, pattern)
+	f, err := os.CreateTemp(dir, pattern)
 	if err != nil {
 		return "", err
 	}

decompress.go

@@ -5,6 +5,7 @@ package getter
 
 import (
 	"os"
+	"slices"
 	"strings"
 )
 
@@ -72,12 +73,7 @@ func containsDotDot(v string) bool {
 	if !strings.Contains(v, "..") {
 		return false
 	}
-	for _, ent := range strings.FieldsFunc(v, isSlashRune) {
-		if ent == ".." {
-			return true
-		}
-	}
-	return false
+	return slices.Contains(strings.FieldsFunc(v, isSlashRune), "..")
 }
 
 func isSlashRune(r rune) bool { return r == '/' || r == '\\' }

decompress_tar_test.go

@@ -6,7 +6,6 @@ package getter
 import (
 	"archive/tar"
 	"bytes"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -82,14 +81,11 @@ func TestTarLimits(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	td, err := ioutil.TempDir("", "getter")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
+	td := t.TempDir()
 
 	tarFilePath := filepath.Join(td, "input.tar")
 
-	err = os.WriteFile(tarFilePath, b.Bytes(), 0666)
+	err := os.WriteFile(tarFilePath, b.Bytes(), 0666)
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -133,15 +129,12 @@ func TestTarLimits(t *testing.T) {
 
 // testDecompressPermissions decompresses a directory and checks the permissions of the expanded files
 func testDecompressorPermissions(t *testing.T, d Decompressor, input string, expected map[string]int, umask os.FileMode) {
-	td, err := ioutil.TempDir("", "getter")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
+	td := t.TempDir()
 
 	// Destination is always joining result so that we have a new path
 	dst := filepath.Join(td, "subdir", "result")
 
-	err = d.Decompress(dst, input, true, umask)
+	err := d.Decompress(dst, input, true, umask)
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}

decompress_testing.go

@@ -7,7 +7,6 @@ import (
 	"crypto/md5"
 	"encoding/hex"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -36,7 +35,7 @@ func TestDecompressor(t testing.TB, d Decompressor, cases []TestDecompressCase)
 		t.Logf("Testing: %s", tc.Input)
 
 		// Temporary dir to store stuff
-		td, err := ioutil.TempDir("", "getter")
+		td, err := os.MkdirTemp("", "getter")
 		if err != nil {
 			t.Fatalf("err: %s", err)
 		}

decompress_zip_test.go

@@ -6,7 +6,6 @@ package getter
 import (
 	"archive/zip"
 	"bytes"
-	"io/ioutil"
 	"log"
 	"os"
 	"path/filepath"
@@ -169,14 +168,11 @@ func TestDecompressZipBomb(t *testing.T) {
 		}
 	}
 
-	td, err := ioutil.TempDir("", "go-getter-zip")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
+	td := t.TempDir()
 
 	zipFilePath := filepath.Join(td, "input.zip")
 
-	err = os.WriteFile(zipFilePath, buf.Bytes(), 0666)
+	err := os.WriteFile(zipFilePath, buf.Bytes(), 0666)
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}

detect_bitbucket_test.go

@@ -38,7 +38,7 @@ func TestBitBucketDetector(t *testing.T) {
 	f := new(BitBucketDetector)
 	for i, tc := range cases {
 		var err error
-		for i := 0; i < 3; i++ {
+		for i := range 3 {
 			var output string
 			var ok bool
 			output, ok, err = f.Detect(tc.Input, pwd)

detect_file_unix_test.go

@@ -7,7 +7,6 @@
 package getter
 
 import (
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -16,16 +15,11 @@ import (
 // If a relative symlink is passed in as the pwd to Detect, the resulting URL
 // can have an invalid path.
 func TestFileDetector_relativeSymlink(t *testing.T) {
-	tmpDir, err := ioutil.TempDir("", "go-getter")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	defer func() { _ = os.RemoveAll(tmpDir) }()
+	tmpDir := t.TempDir()
 
 	// We may have a symlinked tmp dir,
 	// e.g. OSX uses /var -> /private/var
-	tmpDir, err = filepath.EvalSymlinks(tmpDir)
+	tmpDir, err := filepath.EvalSymlinks(tmpDir)
 	if err != nil {
 		t.Fatal(err)
 	}