From 1bc7c48e9e0dcedb879412af0e800dea6d4cd96d Mon Sep 17 00:00:00 2001 From: Nicolas Duchon Date: Sat, 22 Feb 2025 15:10:33 +0100 Subject: [PATCH 1/7] ci: run tests on multiple go versions and oses --- .github/workflows/tests.yml | 20 ++++++++++++++++---- Makefile | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f2775aef..b9ca177e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -13,8 +13,19 @@ on: jobs: unit: - name: Unit Tests - runs-on: ubuntu-latest + strategy: + matrix: + go-version: + - "1.23" + - "1.24" + os: + - macos + - ubuntu + - windows + fail-fast: false + + name: Unit Tests (${{ matrix.os }}/go-${{ matrix.go-version }}) + runs-on: ${{ matrix.os }}-latest steps: - uses: actions/checkout@v4 @@ -23,7 +34,7 @@ jobs: - uses: actions/setup-go@v5 with: - go-version-file: go.mod + go-version: ${{ matrix.go-version }} - name: Install dependencies run: make get-deps @@ -32,7 +43,8 @@ jobs: run: make docker-gen - name: Check code formatting + if: runner.os != 'Windows' run: make check-gofmt - name: Run tests - run: go test -race -v ./internal/... + run: make test diff --git a/Makefile b/Makefile index d067fa4c..f130219d 100644 --- a/Makefile +++ b/Makefile @@ -62,4 +62,4 @@ check-gofmt: fi test: - go test -race ./internal/... + go test -v ./internal/... From f8098015b513381eb1a41f14ec7a839735d6796d Mon Sep 17 00:00:00 2001 From: Nicolas Duchon Date: Sat, 22 Feb 2025 15:37:19 +0100 Subject: [PATCH 2/7] tests: fix TestInclude for Windows --- internal/template/functions_test.go | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/internal/template/functions_test.go b/internal/template/functions_test.go index a3cd4cdc..8a2e59cd 100644 --- a/internal/template/functions_test.go +++ b/internal/template/functions_test.go @@ -96,10 +96,28 @@ func TestInclude(t *testing.T) { data := include("some_random_file") assert.Equal(t, "", data) - _ = os.WriteFile("/tmp/docker-gen-test-temp-file", []byte("some string"), 0o777) - data = include("/tmp/docker-gen-test-temp-file") + f, err := os.CreateTemp("", "docker-gen-test-temp-file") + if err != nil { + t.Fatal(err) + } + + defer func() { + f.Close() + os.Remove(f.Name()) + }() + + err = f.Chmod(0o644) + if err != nil { + t.Fatal(err) + } + + _, err = f.WriteString("some string") + if err != nil { + t.Fatal(err) + } + + data = include(f.Name()) assert.Equal(t, "some string", data) - _ = os.Remove("/tmp/docker-gen-test-temp-file") } func TestIntersect(t *testing.T) { From b8273983b6850ddbe34cfcdc59152c4949c9e251 Mon Sep 17 00:00:00 2001 From: Nicolas Duchon Date: Sat, 22 Feb 2025 15:45:41 +0100 Subject: [PATCH 3/7] tests: fix TestDirList for Windows --- internal/template/functions_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/template/functions_test.go b/internal/template/functions_test.go index 8a2e59cd..04e16561 100644 --- a/internal/template/functions_test.go +++ b/internal/template/functions_test.go @@ -2,7 +2,7 @@ package template import ( "os" - "path" + "path/filepath" "reflect" "testing" @@ -243,9 +243,9 @@ func TestDirList(t *testing.T) { } expected := []string{ - path.Base(files["aaa"]), - path.Base(files["bbb"]), - path.Base(files["ccc"]), + filepath.Base(files["aaa"]), + filepath.Base(files["bbb"]), + filepath.Base(files["ccc"]), } filesList, _ := dirList(dir) From 49429b36e5fc0f6ba1652a5fa50757a0ad37eb36 Mon Sep 17 00:00:00 2001 From: Nicolas Duchon Date: Sun, 23 Feb 2025 18:28:19 +0100 Subject: [PATCH 4/7] fix: write content of dest file in place --- internal/template/template.go | 43 +++++------------------------------ 1 file changed, 6 insertions(+), 37 deletions(-) diff --git a/internal/template/template.go b/internal/template/template.go index c6dd8d5d..ffba197c 100644 --- a/internal/template/template.go +++ b/internal/template/template.go @@ -190,48 +190,17 @@ func GenerateFile(config config.Config, containers context.Context) bool { } if config.Dest != "" { - dest, err := os.CreateTemp(filepath.Dir(config.Dest), "docker-gen") - defer func() { - dest.Close() - os.Remove(dest.Name()) - }() - if err != nil { - log.Fatalf("Unable to create temp file: %s\n", err) - } - - if n, err := dest.Write(contents); n != len(contents) || err != nil { - log.Fatalf("Failed to write to temp file: wrote %d, exp %d, err=%v", n, len(contents), err) - } - - oldContents := []byte{} - if fi, err := os.Stat(config.Dest); err == nil || os.IsNotExist(err) { - if err != nil && os.IsNotExist(err) { - emptyFile, err := os.Create(config.Dest) - if err != nil { - log.Fatalf("Unable to create empty destination file: %s\n", err) - } else { - emptyFile.Close() - fi, _ = os.Stat(config.Dest) - } - } - - if err := dest.Chmod(fi.Mode()); err != nil { - log.Fatalf("Unable to chmod temp file: %s\n", err) - } - - chown(dest, fi) - - oldContents, err = os.ReadFile(config.Dest) - if err != nil { - log.Fatalf("Unable to compare current file contents: %s: %s\n", config.Dest, err) - } + oldContents, err := os.ReadFile(config.Dest) + if err != nil && !os.IsNotExist(err) { + log.Fatalf("Unable to compare current file contents: %s: %s\n", config.Dest, err) } if !bytes.Equal(oldContents, contents) { - err = os.Rename(dest.Name(), config.Dest) + err := os.WriteFile(config.Dest, contents, 0644) if err != nil { - log.Fatalf("Unable to create dest file %s: %s\n", config.Dest, err) + log.Fatalf("Unable to write to dest file %s: %s\n", config.Dest, err) } + log.Printf("Generated '%s' from %d containers", config.Dest, len(filteredContainers)) return true } From 64f5e8b59562fded90b044681cec9d2953171241 Mon Sep 17 00:00:00 2001 From: Nicolas Duchon Date: Sun, 23 Feb 2025 18:57:01 +0100 Subject: [PATCH 5/7] tests: fix mocked /info Docker API response --- internal/generator/generator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go index e968d653..d8f69fec 100644 --- a/internal/generator/generator_test.go +++ b/internal/generator/generator_test.go @@ -30,7 +30,7 @@ func TestGenerateFromEvents(t *testing.T) { {"status":"stop","id":"8dfafdbc3a40","from":"base:latest","time":1374067966} {"status":"start","id":"8dfafdbc3a40","from":"base:latest","time":1374067970} {"status":"destroy","id":"8dfafdbc3a40","from":"base:latest","time":1374067990}` - infoResponse := `{"Containers":1,"Images":1,"Debug":0,"NFd":11,"NGoroutines":21,"MemoryLimit":1,"SwapLimit":0}` + infoResponse := `{"Containers":1,"Images":1,"Debug":false,"NFd":11,"NGoroutines":21,"MemoryLimit":true,"SwapLimit":false}` versionResponse := `{"Version":"1.8.0","Os":"Linux","KernelVersion":"3.18.5-tinycore64","GoVersion":"go1.4.1","GitCommit":"a8a31ef","Arch":"amd64","ApiVersion":"1.19"}` server, _ := dockertest.NewServer("127.0.0.1:0", nil, nil) From c68e64d8ef7c508c0c0e69f7e1589c0e615dbeb2 Mon Sep 17 00:00:00 2001 From: Nicolas Duchon Date: Sun, 23 Feb 2025 22:40:43 +0100 Subject: [PATCH 6/7] refactor: remove unused chown function --- internal/template/chown_unix.go | 18 ------------------ internal/template/chown_windows.go | 12 ------------ 2 files changed, 30 deletions(-) delete mode 100644 internal/template/chown_unix.go delete mode 100644 internal/template/chown_windows.go diff --git a/internal/template/chown_unix.go b/internal/template/chown_unix.go deleted file mode 100644 index dcc62352..00000000 --- a/internal/template/chown_unix.go +++ /dev/null @@ -1,18 +0,0 @@ -//go:build darwin || freebsd || linux || netbsd || openbsd - -package template - -import ( - "io/fs" - "log" - "os" - "syscall" -) - -func chown(dest *os.File, fi fs.FileInfo) { - if stat, ok := fi.Sys().(*syscall.Stat_t); ok { - if err := dest.Chown(int(stat.Uid), int(stat.Gid)); err != nil { - log.Fatalf("Unable to chown temp file: %s\n", err) - } - } -} diff --git a/internal/template/chown_windows.go b/internal/template/chown_windows.go deleted file mode 100644 index a15923d9..00000000 --- a/internal/template/chown_windows.go +++ /dev/null @@ -1,12 +0,0 @@ -//go:build windows - -package template - -import ( - "io/fs" - "os" -) - -func chown(dest *os.File, fi fs.FileInfo) { - // do nothing -} From 82739b37b7733883f97527f3f0deebc3f3bf5f35 Mon Sep 17 00:00:00 2001 From: Nicolas Duchon Date: Sun, 23 Feb 2025 23:16:42 +0100 Subject: [PATCH 7/7] refactor: replace os.IsNotExist() with errors.Is() --- internal/template/template.go | 3 ++- internal/utils/utils.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/template/template.go b/internal/template/template.go index ffba197c..b6ab00b3 100644 --- a/internal/template/template.go +++ b/internal/template/template.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "io/fs" "log" "net/url" "os" @@ -191,7 +192,7 @@ func GenerateFile(config config.Config, containers context.Context) bool { if config.Dest != "" { oldContents, err := os.ReadFile(config.Dest) - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, fs.ErrNotExist) { log.Fatalf("Unable to compare current file contents: %s: %s\n", config.Dest, err) } diff --git a/internal/utils/utils.go b/internal/utils/utils.go index eb514456..2d4fb89b 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -1,6 +1,8 @@ package utils import ( + "errors" + "io/fs" "os" "strings" ) @@ -27,7 +29,7 @@ func PathExists(path string) (bool, error) { if err == nil { return true, nil } - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { return false, nil } return false, err