From 390d782dbce4e180cff80da4ab09a5bbdc5a2fd5 Mon Sep 17 00:00:00 2001 From: Sebastian Webber Date: Fri, 6 Feb 2026 02:18:16 -0300 Subject: [PATCH 1/7] test: add rspec-style bdd wrapper for goconvey Create pkg/tests package that wraps GoConvey with RSpec-like syntax using Describe, Context, It, and Expect. This provides more readable and maintainable test structure while keeping GoConvey's powerful assertion matchers. Benefits: - Single import (. "github.com/pgconfig/api/pkg/tests") - Familiar RSpec syntax for better readability - Re-exports all GoConvey matchers (ShouldEqual, ShouldBeNil, etc) - Cleaner hierarchical test output - Easier for Ruby/RSpec developers to understand Convert profile tests to demonstrate new testing style. Signed-off-by: Sebastian Webber --- pkg/input/profile/profile_test.go | 190 +++++++++++++++--------------- pkg/tests/bdd.go | 88 ++++++++++++++ 2 files changed, 181 insertions(+), 97 deletions(-) create mode 100644 pkg/tests/bdd.go diff --git a/pkg/input/profile/profile_test.go b/pkg/input/profile/profile_test.go index 1c2c82f..e060c42 100644 --- a/pkg/input/profile/profile_test.go +++ b/pkg/input/profile/profile_test.go @@ -2,108 +2,104 @@ package profile import ( "testing" + + . "github.com/pgconfig/api/pkg/tests" ) func TestProfile_Set(t *testing.T) { - tests := []struct { - name string - input string - want Profile - wantErr bool - }{ - { - name: "Web uppercase", - input: "WEB", - want: Web, - wantErr: false, - }, - { - name: "Web lowercase", - input: "web", - want: Web, - wantErr: false, - }, - { - name: "OLTP uppercase", - input: "OLTP", - want: OLTP, - wantErr: false, - }, - { - name: "OLTP lowercase", - input: "oltp", - want: OLTP, - wantErr: false, - }, - { - name: "DW uppercase", - input: "DW", - want: DW, - wantErr: false, - }, - { - name: "DW lowercase", - input: "dw", - want: DW, - wantErr: false, - }, - { - name: "Mixed uppercase", - input: "MIXED", - want: Mixed, - wantErr: false, - }, - { - name: "Mixed mixed case", - input: "Mixed", - want: Mixed, - wantErr: false, - }, - { - name: "Mixed lowercase", - input: "mixed", - want: Mixed, - wantErr: false, - }, - { - name: "Desktop uppercase", - input: "DESKTOP", - want: Desktop, - wantErr: false, - }, - { - name: "Desktop mixed case", - input: "Desktop", - want: Desktop, - wantErr: false, - }, - { - name: "Desktop lowercase", - input: "desktop", - want: Desktop, - wantErr: false, - }, - { - name: "Invalid profile", - input: "invalid", - want: "", - wantErr: true, - }, - } + Describe("Profile.Set()", t, func() { + Context("when input is valid", func() { + It("should accept WEB in uppercase", func() { + var p Profile + err := p.Set("WEB") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, Web) + }) + + It("should accept web in lowercase", func() { + var p Profile + err := p.Set("web") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, Web) + }) + + It("should accept OLTP in uppercase", func() { + var p Profile + err := p.Set("OLTP") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, OLTP) + }) + + It("should accept oltp in lowercase", func() { + var p Profile + err := p.Set("oltp") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, OLTP) + }) + + It("should accept DW in uppercase", func() { + var p Profile + err := p.Set("DW") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, DW) + }) + + It("should accept dw in lowercase", func() { + var p Profile + err := p.Set("dw") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, DW) + }) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var p Profile - err := p.Set(tt.input) + It("should accept MIXED in uppercase", func() { + var p Profile + err := p.Set("MIXED") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, Mixed) + }) - if (err != nil) != tt.wantErr { - t.Errorf("Profile.Set() error = %v, wantErr %v", err, tt.wantErr) - return - } + It("should accept Mixed in mixed case", func() { + var p Profile + err := p.Set("Mixed") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, Mixed) + }) + + It("should accept mixed in lowercase", func() { + var p Profile + err := p.Set("mixed") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, Mixed) + }) + + It("should accept DESKTOP in uppercase", func() { + var p Profile + err := p.Set("DESKTOP") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, Desktop) + }) + + It("should accept Desktop in mixed case", func() { + var p Profile + err := p.Set("Desktop") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, Desktop) + }) + + It("should accept desktop in lowercase", func() { + var p Profile + err := p.Set("desktop") + Expect(err, ShouldBeNil) + Expect(p, ShouldEqual, Desktop) + }) + }) - if !tt.wantErr && p != tt.want { - t.Errorf("Profile.Set() = %v, want %v", p, tt.want) - } + Context("when input is invalid", func() { + It("should return an error", func() { + var p Profile + err := p.Set("invalid") + Expect(err, ShouldNotBeNil) + }) }) - } + }) } diff --git a/pkg/tests/bdd.go b/pkg/tests/bdd.go new file mode 100644 index 0000000..6507d55 --- /dev/null +++ b/pkg/tests/bdd.go @@ -0,0 +1,88 @@ +package tests + +import ( + "testing" + + "github.com/smartystreets/goconvey/convey" +) + +// Describe is the top-level container for a group of tests +// Maps to RSpec's "describe" +func Describe(description string, t *testing.T, fn func()) { + convey.Convey(description, t, fn) +} + +// Context describes a specific context or scenario within a test +// Maps to RSpec's "context" +func Context(description string, fn func()) { + convey.Convey(description, fn) +} + +// It describes a specific behavior or expectation +// Maps to RSpec's "it" +func It(description string, fn func()) { + convey.Convey(description, fn) +} + +// Expect is an alias for So() to make assertions more readable +// Usage: Expect(value, ShouldEqual, expected) +func Expect(actual any, assertion convey.Assertion, expected ...any) { + convey.So(actual, assertion, expected...) +} + +// Re-export common matchers from GoConvey +var ( + // Equality matchers + ShouldEqual = convey.ShouldEqual + ShouldNotEqual = convey.ShouldNotEqual + ShouldResemble = convey.ShouldResemble + ShouldNotResemble = convey.ShouldNotResemble + ShouldPointTo = convey.ShouldPointTo + ShouldNotPointTo = convey.ShouldNotPointTo + ShouldBeNil = convey.ShouldBeNil + ShouldNotBeNil = convey.ShouldNotBeNil + ShouldBeTrue = convey.ShouldBeTrue + ShouldBeFalse = convey.ShouldBeFalse + ShouldBeZeroValue = convey.ShouldBeZeroValue + + // Numeric matchers + ShouldBeGreaterThan = convey.ShouldBeGreaterThan + ShouldBeGreaterThanOrEqualTo = convey.ShouldBeGreaterThanOrEqualTo + ShouldBeLessThan = convey.ShouldBeLessThan + ShouldBeLessThanOrEqualTo = convey.ShouldBeLessThanOrEqualTo + ShouldBeBetween = convey.ShouldBeBetween + ShouldNotBeBetween = convey.ShouldNotBeBetween + + // Collection matchers + ShouldContain = convey.ShouldContain + ShouldNotContain = convey.ShouldNotContain + ShouldBeIn = convey.ShouldBeIn + ShouldNotBeIn = convey.ShouldNotBeIn + ShouldBeEmpty = convey.ShouldBeEmpty + ShouldNotBeEmpty = convey.ShouldNotBeEmpty + ShouldHaveLength = convey.ShouldHaveLength + + // String matchers + ShouldStartWith = convey.ShouldStartWith + ShouldNotStartWith = convey.ShouldNotStartWith + ShouldEndWith = convey.ShouldEndWith + ShouldNotEndWith = convey.ShouldNotEndWith + ShouldBeBlank = convey.ShouldBeBlank + ShouldNotBeBlank = convey.ShouldNotBeBlank + ShouldContainSubstring = convey.ShouldContainSubstring + + // Panic matchers + ShouldPanic = convey.ShouldPanic + ShouldNotPanic = convey.ShouldNotPanic + ShouldPanicWith = convey.ShouldPanicWith + ShouldNotPanicWith = convey.ShouldNotPanicWith + + // Error matchers + ShouldBeError = convey.ShouldBeError + + // Type matchers + ShouldHaveSameTypeAs = convey.ShouldHaveSameTypeAs + ShouldNotHaveSameTypeAs = convey.ShouldNotHaveSameTypeAs + ShouldImplement = convey.ShouldImplement + ShouldNotImplement = convey.ShouldNotImplement +) From 363e2be19530afa3982602ddbef8a145dd67812f Mon Sep 17 00:00:00 2001 From: Sebastian Webber Date: Fri, 6 Feb 2026 02:22:53 -0300 Subject: [PATCH 2/7] test: convert cmd/ tests to rspec-style bdd Convert all test files in cmd/ package to use new BDD testing style with hybrid approach combining table-driven tests and RSpec-like structure. This improves test readability while maintaining the efficiency of table-driven patterns. Changes: - cmd/api/main_test.go: Simple BDD conversion - cmd/api/handlers/v1/config_test.go: Hybrid with captured args to avoid goroutine assertion issues in Fiber handlers - cmd/pgconfigctl/main_test.go: Compact table format inside It block Benefits: - More descriptive test output with nested contexts - Maintains test table efficiency for multiple cases - Consistent testing style across codebase - 63 lines removed through more concise test structure Signed-off-by: Sebastian Webber --- cmd/api/handlers/v1/config_test.go | 86 +++++++---------- cmd/api/main_test.go | 20 ++-- cmd/pgconfigctl/main_test.go | 147 ++++++++++------------------- 3 files changed, 95 insertions(+), 158 deletions(-) diff --git a/cmd/api/handlers/v1/config_test.go b/cmd/api/handlers/v1/config_test.go index 8e2921c..0b7d5fd 100644 --- a/cmd/api/handlers/v1/config_test.go +++ b/cmd/api/handlers/v1/config_test.go @@ -6,64 +6,46 @@ import ( "github.com/gofiber/fiber/v2" "github.com/pgconfig/api/pkg/input/profile" + . "github.com/pgconfig/api/pkg/tests" ) func TestParseConfigArgs_ProfileCaseInsensitive(t *testing.T) { - // Regression test for issue #37: environment_name should be case-insensitive - tests := []struct { - name string - environmentName string - expectedProfile profile.Profile - }{ - { - name: "uppercase MIXED", - environmentName: "MIXED", - expectedProfile: profile.Mixed, - }, - { - name: "mixed case Mixed", - environmentName: "Mixed", - expectedProfile: profile.Mixed, - }, - { - name: "lowercase mixed", - environmentName: "mixed", - expectedProfile: profile.Mixed, - }, - { - name: "mixed case WEB", - environmentName: "Web", - expectedProfile: profile.Web, - }, - { - name: "lowercase web", - environmentName: "web", - expectedProfile: profile.Web, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - app := fiber.New() - - app.Get("/test", func(c *fiber.Ctx) error { - args, err := parseConfigArgs(c) - if err != nil { - return err + Describe("parseConfigArgs", t, func() { + Context("profile parsing (issue #37)", func() { + It("should accept environment_name case insensitively", func() { + tests := []struct { + name string + environmentName string + expectedProfile profile.Profile + }{ + {"uppercase MIXED", "MIXED", profile.Mixed}, + {"mixed case Mixed", "Mixed", profile.Mixed}, + {"lowercase mixed", "mixed", profile.Mixed}, + {"mixed case WEB", "Web", profile.Web}, + {"lowercase web", "web", profile.Web}, } - if args.envName != tt.expectedProfile { - t.Errorf("expected profile %v, got %v", tt.expectedProfile, args.envName) + for _, tt := range tests { + app := fiber.New() + var capturedArgs *configArgs + + app.Get("/test", func(c *fiber.Ctx) error { + args, err := parseConfigArgs(c) + if err != nil { + return err + } + capturedArgs = args + return c.SendStatus(200) + }) + + req := httptest.NewRequest("GET", "/test?environment_name="+tt.environmentName, nil) + _, err := app.Test(req) + + Expect(err, ShouldBeNil) + Expect(capturedArgs, ShouldNotBeNil) + Expect(capturedArgs.envName, ShouldEqual, tt.expectedProfile) } - - return c.SendStatus(200) }) - - req := httptest.NewRequest("GET", "/test?environment_name="+tt.environmentName, nil) - _, err := app.Test(req) - if err != nil { - t.Fatalf("request failed: %v", err) - } }) - } + }) } diff --git a/cmd/api/main_test.go b/cmd/api/main_test.go index eecd00f..fb3c6bf 100644 --- a/cmd/api/main_test.go +++ b/cmd/api/main_test.go @@ -4,16 +4,20 @@ import ( "testing" v1 "github.com/pgconfig/api/cmd/api/handlers/v1" + . "github.com/pgconfig/api/pkg/tests" ) func TestLoadConfiguration(t *testing.T) { - // Paths relative to cmd/api - rulesPath := "../../rules.yml" - docsPath := "../../pg-docs.yml" + Describe("API Configuration", t, func() { + It("should load rules and docs successfully", func() { + // Paths relative to cmd/api + rulesPath := "../../rules.yml" + docsPath := "../../pg-docs.yml" - // We still need to call LoadConfig because the API handlers depend on - // allRules and pgDocs global variables being initialized. - if err := v1.LoadConfig(rulesPath, docsPath); err != nil { - t.Fatalf("Failed to load configuration: %v", err) - } + // We still need to call LoadConfig because the API handlers depend on + // allRules and pgDocs global variables being initialized. + err := v1.LoadConfig(rulesPath, docsPath) + Expect(err, ShouldBeNil) + }) + }) } diff --git a/cmd/pgconfigctl/main_test.go b/cmd/pgconfigctl/main_test.go index f5e5b15..e7be01d 100644 --- a/cmd/pgconfigctl/main_test.go +++ b/cmd/pgconfigctl/main_test.go @@ -3,115 +3,66 @@ package main import ( "bytes" "os" - "strings" "testing" "github.com/pgconfig/api/cmd/pgconfigctl/cmd" + . "github.com/pgconfig/api/pkg/tests" ) -// TestTuneProfileParsing validates that all profile types parse correctly -// This addresses issue #22 where Mixed and Desktop profiles were rejected func TestTuneProfileParsing(t *testing.T) { - tests := []struct { - name string - args []string - wantError bool - errorMsg string - }{ - { - name: "Mixed profile - mixed case (issue #22)", - args: []string{"tune", "--profile=Mixed"}, - wantError: false, - }, - { - name: "Mixed profile - uppercase", - args: []string{"tune", "--profile=MIXED"}, - wantError: false, - }, - { - name: "Mixed profile - lowercase", - args: []string{"tune", "--profile=mixed"}, - wantError: false, - }, - { - name: "Desktop profile - mixed case (issue #22)", - args: []string{"tune", "--profile=Desktop"}, - wantError: false, - }, - { - name: "Desktop profile - uppercase", - args: []string{"tune", "--profile=DESKTOP"}, - wantError: false, - }, - { - name: "Desktop profile - lowercase", - args: []string{"tune", "--profile=desktop"}, - wantError: false, - }, - { - name: "Web profile - uppercase", - args: []string{"tune", "--profile=WEB"}, - wantError: false, - }, - { - name: "Web profile - lowercase", - args: []string{"tune", "--profile=web"}, - wantError: false, - }, - { - name: "OLTP profile", - args: []string{"tune", "--profile=OLTP"}, - wantError: false, - }, - { - name: "DW profile", - args: []string{"tune", "--profile=DW"}, - wantError: false, - }, - { - name: "Invalid profile", - args: []string{"tune", "--profile=invalid"}, - wantError: true, - errorMsg: "must be one of", - }, - } + Describe("tune command", t, func() { + Context("profile parsing (issue #22)", func() { + It("should accept all profile types case insensitively", func() { + tests := []struct { + name string + args []string + wantError bool + errorMsg string + }{ + {"Mixed profile - mixed case", []string{"tune", "--profile=Mixed"}, false, ""}, + {"Mixed profile - uppercase", []string{"tune", "--profile=MIXED"}, false, ""}, + {"Mixed profile - lowercase", []string{"tune", "--profile=mixed"}, false, ""}, + {"Desktop profile - mixed case", []string{"tune", "--profile=Desktop"}, false, ""}, + {"Desktop profile - uppercase", []string{"tune", "--profile=DESKTOP"}, false, ""}, + {"Desktop profile - lowercase", []string{"tune", "--profile=desktop"}, false, ""}, + {"Web profile - uppercase", []string{"tune", "--profile=WEB"}, false, ""}, + {"Web profile - lowercase", []string{"tune", "--profile=web"}, false, ""}, + {"OLTP profile", []string{"tune", "--profile=OLTP"}, false, ""}, + {"DW profile", []string{"tune", "--profile=DW"}, false, ""}, + {"Invalid profile", []string{"tune", "--profile=invalid"}, true, "must be one of"}, + } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Capture output - outBuf := new(bytes.Buffer) - errBuf := new(bytes.Buffer) + for _, tt := range tests { + // Capture output + outBuf := new(bytes.Buffer) + errBuf := new(bytes.Buffer) - // Save and restore original output - originalOut := os.Stdout - originalErr := os.Stderr - t.Cleanup(func() { - os.Stdout = originalOut - os.Stderr = originalErr - }) + // Save and restore original output + originalOut := os.Stdout + originalErr := os.Stderr + defer func() { + os.Stdout = originalOut + os.Stderr = originalErr + }() - // Set command output - cmd.RootCmd.SetOut(outBuf) - cmd.RootCmd.SetErr(errBuf) - cmd.RootCmd.SetArgs(tt.args) + // Set command output + cmd.RootCmd.SetOut(outBuf) + cmd.RootCmd.SetErr(errBuf) + cmd.RootCmd.SetArgs(tt.args) - // Execute command - err := cmd.RootCmd.Execute() + // Execute command + err := cmd.RootCmd.Execute() - if tt.wantError { - if err == nil { - t.Errorf("Expected error but got none") - return - } - if tt.errorMsg != "" && !strings.Contains(err.Error(), tt.errorMsg) { - t.Errorf("Error message = %v, want to contain %v", err.Error(), tt.errorMsg) + if tt.wantError { + Expect(err, ShouldNotBeNil) + if tt.errorMsg != "" { + Expect(err.Error(), ShouldContainSubstring, tt.errorMsg) + } + } else { + Expect(err, ShouldBeNil) + } } - } else { - if err != nil { - t.Errorf("Unexpected error: %v\nOutput: %s\nError output: %s", - err, outBuf.String(), errBuf.String()) - } - } + }) }) - } + }) } From 7772b89a630f985d42973550f0b9a137c07fe895 Mon Sep 17 00:00:00 2001 From: Sebastian Webber Date: Fri, 6 Feb 2026 02:25:44 -0300 Subject: [PATCH 3/7] test: convert pkg/input/bytes and pkg/version to bdd Convert remaining input package tests and version tests to RSpec-style BDD format. Both packages already used GoConvey, just needed import updates and structure improvements. Changes: - pkg/input/bytes: Use Context/It with hybrid table-driven approach for formatting tests, keeps test data compact - pkg/version: Simple Describe/It conversion Both maintain the same test coverage with more readable structure. Signed-off-by: Sebastian Webber --- pkg/input/bytes/bytes_test.go | 86 +++++++++++++++++------------------ pkg/version/version_test.go | 8 ++-- 2 files changed, 46 insertions(+), 48 deletions(-) diff --git a/pkg/input/bytes/bytes_test.go b/pkg/input/bytes/bytes_test.go index 611749f..6e449c2 100644 --- a/pkg/input/bytes/bytes_test.go +++ b/pkg/input/bytes/bytes_test.go @@ -1,77 +1,75 @@ package bytes import ( - "fmt" "testing" - . "github.com/smartystreets/goconvey/convey" + . "github.com/pgconfig/api/pkg/tests" ) func Test_Bytes(t *testing.T) { - Convey("Parsing", t, func() { - Convey("should parse bytes to the postgres byte format", func() { + Describe("Byte parsing and formatting", t, func() { + It("should parse bytes to the postgres byte format", func() { input := 10 * GB got, err := marshalBytes(&input) - So(err, ShouldBeNil) - So(got, ShouldResemble, []byte(`"10GB"`)) + Expect(err, ShouldBeNil) + Expect(got, ShouldResemble, []byte(`"10GB"`)) }) - Convey("should format bytes to string", func() { - tests := []struct { - desc string - args Byte - want string - }{ - {"negative values", -1, "-1"}, - {"zero", 0, "0"}, - {"Bytes", 5, "5B"}, - {"KiloBytes", 455 * KB, "455kB"}, - {"MegaBytes", 1023 * MB, "1023MB"}, - {"GigaBytes", 565 * GB, "565GB"}, - {"TeraBytes", 396 * TB, "396TB"}, - } - for _, tt := range tests { - Convey(fmt.Sprintf("should format %s", tt.desc), func() { + Context("when formatting bytes to string", func() { + It("should format all byte units correctly", func() { + tests := []struct { + desc string + args Byte + want string + }{ + {"negative values", -1, "-1"}, + {"zero", 0, "0"}, + {"Bytes", 5, "5B"}, + {"KiloBytes", 455 * KB, "455kB"}, + {"MegaBytes", 1023 * MB, "1023MB"}, + {"GigaBytes", 565 * GB, "565GB"}, + {"TeraBytes", 396 * TB, "396TB"}, + } + for _, tt := range tests { got := formatBytes(tt.args) - So(got, ShouldEqual, tt.want) - }) - } + Expect(got, ShouldEqual, tt.want) + } + }) }) - Convey("should parse bytes from string", func() { - Convey("should parse Bytes", func() { - + Context("when parsing bytes from string", func() { + It("should parse Bytes", func() { got, err := Parse("5B") - So(err, ShouldBeNil) - So(got, ShouldEqual, 5) + Expect(err, ShouldBeNil) + Expect(got, ShouldEqual, 5) got, err = Parse("5") - So(err, ShouldBeNil) - So(got, ShouldEqual, 5) + Expect(err, ShouldBeNil) + Expect(got, ShouldEqual, 5) }) - Convey("should parse KiloBytes", func() { + It("should parse KiloBytes", func() { got, err := Parse("455KB") - So(err, ShouldBeNil) - So(got, ShouldEqual, 455*KB) + Expect(err, ShouldBeNil) + Expect(got, ShouldEqual, 455*KB) }) - Convey("should parse MegaBytes", func() { + It("should parse MegaBytes", func() { got, err := Parse("1023MB") - So(err, ShouldBeNil) - So(got, ShouldEqual, 1023*MB) + Expect(err, ShouldBeNil) + Expect(got, ShouldEqual, 1023*MB) }) - Convey("should parse GigaBytes", func() { + It("should parse GigaBytes", func() { got, err := Parse("565GB") - So(err, ShouldBeNil) - So(got, ShouldEqual, 565*GB) + Expect(err, ShouldBeNil) + Expect(got, ShouldEqual, 565*GB) }) - Convey("should parse TeraBytes", func() { + It("should parse TeraBytes", func() { got, err := Parse("396TB") - So(err, ShouldBeNil) - So(got, ShouldEqual, 396*TB) + Expect(err, ShouldBeNil) + Expect(got, ShouldEqual, 396*TB) }) }) }) diff --git a/pkg/version/version_test.go b/pkg/version/version_test.go index 7b81229..e4884e9 100644 --- a/pkg/version/version_test.go +++ b/pkg/version/version_test.go @@ -4,14 +4,14 @@ import ( "fmt" "testing" - . "github.com/smartystreets/goconvey/convey" + . "github.com/pgconfig/api/pkg/tests" ) func TestPretty(t *testing.T) { - Convey("Version", t, func() { - Convey("should print the version as expected", func() { + Describe("Version", t, func() { + It("should print the version as expected", func() { got := Pretty() - So(got, ShouldResemble, fmt.Sprintf("%s (%s)", Tag, Commit)) + Expect(got, ShouldResemble, fmt.Sprintf("%s (%s)", Tag, Commit)) }) }) } From d318ad3146b38f68ffc4ef09dd85974fa74b5e31 Mon Sep 17 00:00:00 2001 From: Sebastian Webber Date: Fri, 6 Feb 2026 02:34:52 -0300 Subject: [PATCH 4/7] ci: add workaround for go 1.25 covdata bug Fix "go: no such tool covdata" error when running tests with coverage in Go 1.25. This is a known issue where the covdata tool cannot be found with GOTOOLCHAIN=auto. Reference: https://github.com/golang/go/issues/75031 Signed-off-by: Sebastian Webber --- .github/workflows/cover.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/cover.yml b/.github/workflows/cover.yml index def4cef..210bec0 100644 --- a/.github/workflows/cover.yml +++ b/.github/workflows/cover.yml @@ -26,6 +26,8 @@ jobs: - uses: actions/setup-go@v5 with: go-version-file: go.mod + - name: Set Go toolchain (workaround for Go 1.25 covdata bug) + run: go env -w GOTOOLCHAIN=go1.25.1+auto - run: make test - name: Send coverage env: From b2288ac771734c2a4007fcb60180649ba0114da4 Mon Sep 17 00:00:00 2001 From: Sebastian Webber Date: Fri, 6 Feb 2026 02:35:27 -0300 Subject: [PATCH 5/7] test: convert pkg/category tests to bdd style Convert worker_test.go to use the new BDD-style test framework with Describe/Context/It structure. Maintain hybrid approach with compact table-driven tests inside It blocks for efficiency. Reduces test code from 136 to 53 lines while improving readability and maintaining all 27 assertions. Signed-off-by: Sebastian Webber --- pkg/category/worker_test.go | 157 +++++++++--------------------------- 1 file changed, 38 insertions(+), 119 deletions(-) diff --git a/pkg/category/worker_test.go b/pkg/category/worker_test.go index f8c740d..e73bb27 100644 --- a/pkg/category/worker_test.go +++ b/pkg/category/worker_test.go @@ -6,130 +6,49 @@ import ( "github.com/pgconfig/api/pkg/input" "github.com/pgconfig/api/pkg/input/bytes" "github.com/pgconfig/api/pkg/input/profile" + . "github.com/pgconfig/api/pkg/tests" ) func TestNewWorkerCfg(t *testing.T) { - tests := []struct { - name string - profile profile.Profile - totalCPU int - expectedMaxWorkerProcesses int - expectedMaxParallelWorkers int - expectedMaxParallelWorkerPerGather int - description string - }{ - { - name: "Desktop with 4 cores", - profile: profile.Desktop, - totalCPU: 4, - expectedMaxWorkerProcesses: 8, - expectedMaxParallelWorkers: 8, - expectedMaxParallelWorkerPerGather: 2, - description: "Small system uses minimum of 8 for worker processes", - }, - { - name: "Web with 8 cores", - profile: profile.Web, - totalCPU: 8, - expectedMaxWorkerProcesses: 8, - expectedMaxParallelWorkers: 8, - expectedMaxParallelWorkerPerGather: 2, - description: "Web keeps default parallel workers per gather", - }, - { - name: "OLTP with 16 cores", - profile: profile.OLTP, - totalCPU: 16, - expectedMaxWorkerProcesses: 16, - expectedMaxParallelWorkers: 16, - expectedMaxParallelWorkerPerGather: 2, - description: "OLTP scales workers with CPU but keeps per-gather at 2", - }, - { - name: "Mixed with 16 cores", - profile: profile.Mixed, - totalCPU: 16, - expectedMaxWorkerProcesses: 16, - expectedMaxParallelWorkers: 16, - expectedMaxParallelWorkerPerGather: 2, - description: "Mixed workload uses default parallel workers per gather", - }, - { - name: "DW with 8 cores", - profile: profile.DW, - totalCPU: 8, - expectedMaxWorkerProcesses: 8, - expectedMaxParallelWorkers: 8, - expectedMaxParallelWorkerPerGather: 4, - description: "DW uses CPU/2 for parallel workers per gather", - }, - { - name: "DW with 16 cores", - profile: profile.DW, - totalCPU: 16, - expectedMaxWorkerProcesses: 16, - expectedMaxParallelWorkers: 16, - expectedMaxParallelWorkerPerGather: 8, - description: "DW scales parallel workers per gather with CPU", - }, - { - name: "DW with 32 cores", - profile: profile.DW, - totalCPU: 32, - expectedMaxWorkerProcesses: 32, - expectedMaxParallelWorkers: 32, - expectedMaxParallelWorkerPerGather: 16, - description: "DW with many cores gets high parallelism", - }, - { - name: "DW with 2 cores ensures minimum", - profile: profile.DW, - totalCPU: 2, - expectedMaxWorkerProcesses: 8, - expectedMaxParallelWorkers: 8, - expectedMaxParallelWorkerPerGather: 2, - description: "DW with few cores still gets minimum values", - }, - { - name: "Large system with 64 cores", - profile: profile.OLTP, - totalCPU: 64, - expectedMaxWorkerProcesses: 64, - expectedMaxParallelWorkers: 64, - expectedMaxParallelWorkerPerGather: 2, - description: "Large OLTP system scales workers but not per-gather", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - in := input.Input{ - OS: "linux", - Arch: "amd64", - Profile: tt.profile, - TotalCPU: tt.totalCPU, - TotalRAM: 16 * bytes.GB, - MaxConnections: 100, - DiskType: "SSD", - PostgresVersion: 16.0, - } - - cfg := NewWorkerCfg(in) - - if cfg.MaxWorkerProcesses != tt.expectedMaxWorkerProcesses { - t.Errorf("%s: expected max_worker_processes = %d, got %d", - tt.description, tt.expectedMaxWorkerProcesses, cfg.MaxWorkerProcesses) - } - - if cfg.MaxParallelWorkers != tt.expectedMaxParallelWorkers { - t.Errorf("%s: expected max_parallel_workers = %d, got %d", - tt.description, tt.expectedMaxParallelWorkers, cfg.MaxParallelWorkers) + Describe("Worker configuration", t, func() { + It("should configure workers based on profile and CPU count", func() { + tests := []struct { + name string + profile profile.Profile + totalCPU int + expectedMaxWorkerProcesses int + expectedMaxParallelWorkers int + expectedMaxParallelWorkerPerGather int + }{ + {"Desktop with 4 cores", profile.Desktop, 4, 8, 8, 2}, + {"Web with 8 cores", profile.Web, 8, 8, 8, 2}, + {"OLTP with 16 cores", profile.OLTP, 16, 16, 16, 2}, + {"Mixed with 16 cores", profile.Mixed, 16, 16, 16, 2}, + {"DW with 8 cores", profile.DW, 8, 8, 8, 4}, + {"DW with 16 cores", profile.DW, 16, 16, 16, 8}, + {"DW with 32 cores", profile.DW, 32, 32, 32, 16}, + {"DW with 2 cores ensures minimum", profile.DW, 2, 8, 8, 2}, + {"Large system with 64 cores", profile.OLTP, 64, 64, 64, 2}, } - if cfg.MaxParallelWorkerPerGather != tt.expectedMaxParallelWorkerPerGather { - t.Errorf("%s: expected max_parallel_workers_per_gather = %d, got %d", - tt.description, tt.expectedMaxParallelWorkerPerGather, cfg.MaxParallelWorkerPerGather) + for _, tt := range tests { + in := input.Input{ + OS: "linux", + Arch: "amd64", + Profile: tt.profile, + TotalCPU: tt.totalCPU, + TotalRAM: 16 * bytes.GB, + MaxConnections: 100, + DiskType: "SSD", + PostgresVersion: 16.0, + } + + cfg := NewWorkerCfg(in) + + Expect(cfg.MaxWorkerProcesses, ShouldEqual, tt.expectedMaxWorkerProcesses) + Expect(cfg.MaxParallelWorkers, ShouldEqual, tt.expectedMaxParallelWorkers) + Expect(cfg.MaxParallelWorkerPerGather, ShouldEqual, tt.expectedMaxParallelWorkerPerGather) } }) - } + }) } From 927757e652191ac9740ce1893932211f8f93425a Mon Sep 17 00:00:00 2001 From: Sebastian Webber Date: Fri, 6 Feb 2026 02:37:03 -0300 Subject: [PATCH 6/7] test: convert pkg/format tests to bdd style Convert all three format tests (alter_test.go, conf_test.go, sg_config_test.go) to use BDD-style Describe/It structure for improved readability and consistency with project test standards. Signed-off-by: Sebastian Webber --- pkg/format/alter_test.go | 18 +++++++++++++----- pkg/format/conf_test.go | 17 ++++++++++++----- pkg/format/sg_config_test.go | 19 +++++++++++++------ 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/pkg/format/alter_test.go b/pkg/format/alter_test.go index 6e4cb95..56a8292 100644 --- a/pkg/format/alter_test.go +++ b/pkg/format/alter_test.go @@ -5,10 +5,13 @@ import ( "testing" "github.com/andreyvit/diff" + . "github.com/pgconfig/api/pkg/tests" ) func TestAlterSystem(t *testing.T) { - sample := ` + Describe("ALTER SYSTEM formatting", t, func() { + It("should format configuration as ALTER SYSTEM statements", func() { + expected := ` -- Memory Configuration ALTER SYSTEM SET shared_buffers TO '23GB'; ALTER SYSTEM SET effective_cache_size TO '70GB'; @@ -37,9 +40,14 @@ ALTER SYSTEM SET max_parallel_workers_per_gather TO '2'; ALTER SYSTEM SET max_parallel_workers TO '2'; ` - out := AlterSystem(sliceConfSample) + out := AlterSystem(sliceConfSample) - if a, e := strings.TrimSpace(sample), strings.TrimSpace(out); a != e { - t.Errorf("Result not as expected:\n%v", diff.LineDiff(e, a)) - } + actual := strings.TrimSpace(out) + expectedTrimmed := strings.TrimSpace(expected) + + if actual != expectedTrimmed { + t.Errorf("Result not as expected:\n%v", diff.LineDiff(expectedTrimmed, actual)) + } + }) + }) } diff --git a/pkg/format/conf_test.go b/pkg/format/conf_test.go index 1d38857..d94fb8b 100644 --- a/pkg/format/conf_test.go +++ b/pkg/format/conf_test.go @@ -6,6 +6,7 @@ import ( "github.com/andreyvit/diff" "github.com/pgconfig/api/pkg/category" + . "github.com/pgconfig/api/pkg/tests" ) var sliceConfSample = []category.SliceOutput{ @@ -47,7 +48,9 @@ var sliceConfSample = []category.SliceOutput{ {Name: "max_parallel_workers", Value: "2", Format: "int"}}}} func TestConfigFile(t *testing.T) { - sample := ` + Describe("Config file formatting", t, func() { + It("should format configuration as postgresql.conf format", func() { + expected := ` # Memory Configuration shared_buffers = 23GB effective_cache_size = 70GB @@ -76,10 +79,14 @@ max_parallel_workers_per_gather = 2 max_parallel_workers = 2 ` - out := ConfigFile(sliceConfSample) + out := ConfigFile(sliceConfSample) - if a, e := strings.TrimSpace(sample), strings.TrimSpace(out); a != e { - t.Errorf("Result not as expected:\n%v", diff.LineDiff(e, a)) - } + actual := strings.TrimSpace(out) + expectedTrimmed := strings.TrimSpace(expected) + if actual != expectedTrimmed { + t.Errorf("Result not as expected:\n%v", diff.LineDiff(expectedTrimmed, actual)) + } + }) + }) } diff --git a/pkg/format/sg_config_test.go b/pkg/format/sg_config_test.go index a7f16f3..45b78f7 100644 --- a/pkg/format/sg_config_test.go +++ b/pkg/format/sg_config_test.go @@ -5,11 +5,13 @@ import ( "testing" "github.com/andreyvit/diff" + . "github.com/pgconfig/api/pkg/tests" ) func TestSGConfigFile(t *testing.T) { - - sample := ` + Describe("StackGres config formatting", t, func() { + It("should format configuration as SGPostgresConfig YAML", func() { + expected := ` apiVersion: stackgres.io/v1 kind: SGPostgresConfig metadata: @@ -33,9 +35,14 @@ spec: work_mem: 965MB ` - out := SGConfigFile(sliceConfSample, "13") + out := SGConfigFile(sliceConfSample, "13") + + actual := strings.TrimSpace(out) + expectedTrimmed := strings.TrimSpace(expected) - if a, e := strings.TrimSpace(sample), strings.TrimSpace(out); a != e { - t.Errorf("Result not as expected:\n%v", diff.LineDiff(e, a)) - } + if actual != expectedTrimmed { + t.Errorf("Result not as expected:\n%v", diff.LineDiff(expectedTrimmed, actual)) + } + }) + }) } From 9aafa9039c59ef1270a90ebd3787d1edfd6d8ccb Mon Sep 17 00:00:00 2001 From: Sebastian Webber Date: Fri, 6 Feb 2026 02:43:00 -0300 Subject: [PATCH 7/7] test: convert pkg/rules tests to bdd style Convert all 7 rules test files to use BDD-style Describe/Context/It structure. Maintain hybrid approach with compact table-driven tests where appropriate for efficiency. Major improvements: - aio_test: 292 to 62 lines (230 lines removed) - compute_test: Consolidated into 2 focused tests - os_test: Better organization with Windows-specific Context - version_test: Clear Context groupings per PostgreSQL version All 148 assertions passing. Signed-off-by: Sebastian Webber --- pkg/rules/aio_test.go | 317 ++++++-------------------------------- pkg/rules/arch_test.go | 43 +++--- pkg/rules/compute_test.go | 212 ++++++++----------------- pkg/rules/os_test.go | 224 +++++++++++++-------------- pkg/rules/profile_test.go | 18 ++- pkg/rules/storage_test.go | 153 ++++++------------ pkg/rules/version_test.go | 154 ++++++++++-------- 7 files changed, 391 insertions(+), 730 deletions(-) diff --git a/pkg/rules/aio_test.go b/pkg/rules/aio_test.go index dc97b78..893c8ca 100644 --- a/pkg/rules/aio_test.go +++ b/pkg/rules/aio_test.go @@ -7,287 +7,56 @@ import ( "github.com/pgconfig/api/pkg/input" "github.com/pgconfig/api/pkg/input/bytes" "github.com/pgconfig/api/pkg/input/profile" - "github.com/stretchr/testify/assert" + . "github.com/pgconfig/api/pkg/tests" ) func Test_computeAIO(t *testing.T) { - type args struct { - in *input.Input - cfg *category.ExportCfg - } - tests := []struct { - name string - args args - want *category.ExportCfg - wantErr bool - }{ - { - name: "Desktop profile with 4 cores", - args: args{ - in: &input.Input{ - OS: "linux", - Arch: "amd64", - TotalRAM: 8 * bytes.GB, - TotalCPU: 4, - Profile: profile.Desktop, - DiskType: "SSD", - MaxConnections: 100, - PostgresVersion: 18.0, - }, - cfg: category.NewExportCfg(input.Input{}), - }, - want: func() *category.ExportCfg { - cfg := category.NewExportCfg(input.Input{}) - cfg.Storage.IOMethod = "worker" - cfg.Storage.IOWorkers = 2 // 4 * 0.1 = 0.4 -> ceil = 1, min 2 -> 2 - return cfg - }(), - }, - { - name: "DW profile with 16 cores HDD", - args: args{ - in: &input.Input{ - OS: "linux", - Arch: "amd64", - TotalRAM: 32 * bytes.GB, - TotalCPU: 16, - Profile: profile.DW, - DiskType: "HDD", - MaxConnections: 100, - PostgresVersion: 18.0, - }, - cfg: category.NewExportCfg(input.Input{}), - }, - want: func() *category.ExportCfg { - cfg := category.NewExportCfg(input.Input{}) - cfg.Storage.IOMethod = "worker" - // factor 0.4 + 0.1 for HDD = 0.5, 16 * 0.5 = 8, max workers = totalCPU (16), min 2 - cfg.Storage.IOWorkers = 8 - cfg.Storage.IOMaxCombineLimit = 128 - cfg.Storage.IOMaxConcurrency = 256 - return cfg - }(), - }, - { - name: "OLTP profile with 8 cores SSD", - args: args{ - in: &input.Input{ - OS: "linux", - Arch: "amd64", - TotalRAM: 16 * bytes.GB, - TotalCPU: 8, - Profile: profile.OLTP, - DiskType: "SSD", - MaxConnections: 100, - PostgresVersion: 18.0, - }, - cfg: category.NewExportCfg(input.Input{}), - }, - want: func() *category.ExportCfg { - cfg := category.NewExportCfg(input.Input{}) - cfg.Storage.IOMethod = "worker" - // factor 0.3, 8 * 0.3 = 2.4 -> ceil = 3 - cfg.Storage.IOWorkers = 3 - cfg.Storage.IOMaxConcurrency = 128 - return cfg - }(), - }, - { - name: "PostgreSQL version 17 should still have defaults but will be zeroed later", - args: args{ - in: &input.Input{ - OS: "linux", - Arch: "amd64", - TotalRAM: 8 * bytes.GB, - TotalCPU: 4, - Profile: profile.Web, - DiskType: "SSD", - MaxConnections: 100, - PostgresVersion: 17.0, - }, - cfg: category.NewExportCfg(input.Input{}), - }, - want: func() *category.ExportCfg { - cfg := category.NewExportCfg(input.Input{}) - cfg.Storage.IOMethod = "worker" - // computeAIO still sets io_workers, computeVersion will zero them - cfg.Storage.IOWorkers = 2 // 4 * 0.2 = 0.8 -> ceil =1, min2 ->2 - return cfg - }(), - }, - { - name: "Web profile should use factor 0.2", - args: args{ - in: &input.Input{ - OS: "linux", - Arch: "amd64", - TotalRAM: 16 * bytes.GB, - TotalCPU: 10, - Profile: profile.Web, - DiskType: "SSD", - MaxConnections: 100, - PostgresVersion: 18.0, - }, - cfg: category.NewExportCfg(input.Input{}), - }, - want: func() *category.ExportCfg { - cfg := category.NewExportCfg(input.Input{}) - cfg.Storage.IOMethod = "worker" - // 10 * 0.2 = 2 - cfg.Storage.IOWorkers = 2 - return cfg - }(), - }, - { - name: "Mixed profile should use factor 0.25", - args: args{ - in: &input.Input{ - OS: "linux", - Arch: "amd64", - TotalRAM: 16 * bytes.GB, - TotalCPU: 12, - Profile: profile.Mixed, - DiskType: "SSD", - MaxConnections: 100, - PostgresVersion: 18.0, - }, - cfg: category.NewExportCfg(input.Input{}), - }, - want: func() *category.ExportCfg { - cfg := category.NewExportCfg(input.Input{}) - cfg.Storage.IOMethod = "worker" - // 12 * 0.25 = 3 - cfg.Storage.IOWorkers = 3 - return cfg - }(), - }, - { - name: "Unknown profile should use default factor 0.25", - args: args{ - in: &input.Input{ - OS: "linux", - Arch: "amd64", - TotalRAM: 16 * bytes.GB, - TotalCPU: 12, - Profile: "unknown_profile", - DiskType: "SSD", - MaxConnections: 100, - PostgresVersion: 18.0, - }, - cfg: category.NewExportCfg(input.Input{}), - }, - want: func() *category.ExportCfg { - cfg := category.NewExportCfg(input.Input{}) - cfg.Storage.IOMethod = "worker" - // 12 * 0.25 = 3 - cfg.Storage.IOWorkers = 3 - return cfg - }(), - }, - { - name: "SAN disk type should behave like SSD", - args: args{ - in: &input.Input{ - OS: "linux", - Arch: "amd64", - TotalRAM: 16 * bytes.GB, - TotalCPU: 8, - Profile: profile.OLTP, - DiskType: "SAN", - MaxConnections: 100, - PostgresVersion: 18.0, - }, - cfg: category.NewExportCfg(input.Input{}), - }, - want: func() *category.ExportCfg { - cfg := category.NewExportCfg(input.Input{}) - cfg.Storage.IOMethod = "worker" - // OLTP factor 0.3, 8 * 0.3 = 2.4 -> ceil 3 - cfg.Storage.IOWorkers = 3 - cfg.Storage.IOMaxConcurrency = 128 - return cfg - }(), - }, - { - name: "Should limit workers to TotalCPU", - args: args{ - in: &input.Input{ + Describe("AIO (Asynchronous I/O) computation", t, func() { + It("should configure io_workers based on profile and CPU count", func() { + tests := []struct { + name string + profile profile.Profile + totalCPU int + diskType string + pgVersion float64 + expectedIOWorkers int + expectedIOMethod string + expectedCombineLimit int + expectedConcurrency int + }{ + {"Desktop with 4 cores", profile.Desktop, 4, "SSD", 18.0, 2, "worker", 16, 64}, + {"DW profile with 16 cores HDD", profile.DW, 16, "HDD", 18.0, 8, "worker", 128, 256}, + {"OLTP profile with 8 cores SSD", profile.OLTP, 8, "SSD", 18.0, 3, "worker", 16, 128}, + {"PostgreSQL version 17 still gets values", profile.Web, 4, "SSD", 17.0, 2, "worker", 16, 64}, + {"Web profile uses factor 0.2", profile.Web, 10, "SSD", 18.0, 2, "worker", 16, 64}, + {"Mixed profile uses factor 0.25", profile.Mixed, 12, "SSD", 18.0, 3, "worker", 16, 64}, + {"Unknown profile uses default factor 0.25", "unknown_profile", 12, "SSD", 18.0, 3, "worker", 16, 64}, + {"SAN disk type behaves like SSD", profile.OLTP, 8, "SAN", 18.0, 3, "worker", 16, 128}, + {"Should limit workers to TotalCPU", profile.DW, 2, "HDD", 18.0, 2, "worker", 128, 256}, + {"Should cap workers at TotalCPU when min exceeds it", profile.Web, 1, "SSD", 18.0, 1, "worker", 16, 64}, + } + + for _, tt := range tests { + in := &input.Input{ OS: "linux", Arch: "amd64", TotalRAM: 16 * bytes.GB, - TotalCPU: 2, - Profile: profile.DW, // factor 0.4 - DiskType: "HDD", // +0.1 = 0.5 + TotalCPU: tt.totalCPU, + Profile: tt.profile, + DiskType: tt.diskType, MaxConnections: 100, - PostgresVersion: 18.0, - }, - cfg: category.NewExportCfg(input.Input{}), - }, - want: func() *category.ExportCfg { + PostgresVersion: float32(tt.pgVersion), + } cfg := category.NewExportCfg(input.Input{}) - cfg.Storage.IOMethod = "worker" - // 2 * 0.5 = 1 -> min 2 -> 2. TotalCPU is 2. So workers = 2. - // Let's try a case where calculation > TotalCPU. - // Say TotalCPU = 2, Factor = 1.5 (impossible here but hypothetically) - // Actually with current factors max is 0.5. - // So workers will always be <= TotalCPU/2? No. - // If TotalCPU=2, 2*0.5=1 -> min 2. - // If TotalCPU=4, DW+HDD=0.5 -> 2. - // Wait, checking code: workers := ceil(TotalCPU * factor). - // If factor < 1, then workers < TotalCPU always. - // Except if ceil pushes it up? - // 4 * 0.9 = 3.6 -> 4. - // 4 * 1.1 = 4.4 -> 5. - // Current max factor is 0.4(DW) + 0.1(HDD) = 0.5. - // So workers will always be <= TotalCPU (since factor < 1). - // So the check `if workers > in.TotalCPU` might be unreachable with current factors? - // Let's verify. - // 1 core: 1 * 0.5 = 0.5 -> ceil 1 -> min 2. workers=2. TotalCPU=1. - // So workers(2) > TotalCPU(1). - // Aha! This case hits it. - cfg.Storage.IOWorkers = 2 // Calculated 2, but TotalCPU is 2. Wait. - cfg.Storage.IOMaxCombineLimit = 128 - cfg.Storage.IOMaxConcurrency = 256 - return cfg - }(), - }, - { - name: "Should cap workers at TotalCPU when calculated min exceeds it", - args: args{ - in: &input.Input{ - OS: "linux", - Arch: "amd64", - TotalRAM: 4 * bytes.GB, - TotalCPU: 1, // Single core - Profile: profile.Web, // Factor 0.2 - DiskType: "SSD", - MaxConnections: 100, - PostgresVersion: 18.0, - }, - cfg: category.NewExportCfg(input.Input{}), - }, - want: func() *category.ExportCfg { - cfg := category.NewExportCfg(input.Input{}) - cfg.Storage.IOMethod = "worker" - // 1 * 0.2 = 0.2 -> ceil 1. - // min workers check -> sets to 2. - // max workers check -> if 2 > 1 -> set to 1. - cfg.Storage.IOWorkers = 1 - return cfg - }(), - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := computeAIO(tt.args.in, tt.args.cfg) - if (err != nil) != tt.wantErr { - t.Errorf("computeAIO() error = %v, wantErr %v", err, tt.wantErr) - return + + got, err := computeAIO(in, cfg) + + Expect(err, ShouldBeNil) + Expect(got.Storage.IOMethod, ShouldEqual, tt.expectedIOMethod) + Expect(got.Storage.IOWorkers, ShouldEqual, tt.expectedIOWorkers) + Expect(got.Storage.IOMaxCombineLimit, ShouldEqual, tt.expectedCombineLimit) + Expect(got.Storage.IOMaxConcurrency, ShouldEqual, tt.expectedConcurrency) } - assert.Equal(t, tt.want.Storage.IOMethod, got.Storage.IOMethod, "io_method mismatch") - assert.Equal(t, tt.want.Storage.IOWorkers, got.Storage.IOWorkers, "io_workers mismatch") - assert.Equal(t, tt.want.Storage.IOMaxCombineLimit, got.Storage.IOMaxCombineLimit, "io_max_combine_limit mismatch") - assert.Equal(t, tt.want.Storage.IOMaxConcurrency, got.Storage.IOMaxConcurrency, "io_max_concurrency mismatch") }) - } -} \ No newline at end of file + }) +} diff --git a/pkg/rules/arch_test.go b/pkg/rules/arch_test.go index ba1a910..3470f6b 100644 --- a/pkg/rules/arch_test.go +++ b/pkg/rules/arch_test.go @@ -6,32 +6,33 @@ import ( "github.com/pgconfig/api/pkg/category" "github.com/pgconfig/api/pkg/input" "github.com/pgconfig/api/pkg/input/bytes" - - . "github.com/smartystreets/goconvey/convey" + . "github.com/pgconfig/api/pkg/tests" ) func Test_computeArch(t *testing.T) { - - Convey("Validations", t, func() { - Convey("Should thow an error when the arch is invalid", func() { - _, err := computeArch(&input.Input{Arch: "xpto-invalid-arch"}, nil) - So(err, ShouldNotBeNil) + Describe("Architecture validations", t, func() { + Context("when arch is invalid", func() { + It("should return an error", func() { + _, err := computeArch(&input.Input{Arch: "xpto-invalid-arch"}, nil) + Expect(err, ShouldNotBeNil) + }) }) - Convey("Should thow an error when the arch is 386 or i686 and has memory values over 4GiB", func() { - - similarArchs := []string{"386", "i686"} - - for _, newArch := range similarArchs { - in := fakeInput() - in.Arch = newArch - in.TotalRAM = 1 * bytes.TB - - out, _ := computeArch(in, category.NewExportCfg(*in)) - So(out.Memory.SharedBuffers, ShouldBeLessThanOrEqualTo, 4*bytes.GB) - So(out.Memory.WorkMem, ShouldBeLessThanOrEqualTo, 4*bytes.GB) - So(out.Memory.MaintenanceWorkMem, ShouldBeLessThanOrEqualTo, 4*bytes.GB) - } + Context("when arch is 386 or i686 with large memory", func() { + It("should limit memory values to 4GiB", func() { + similarArchs := []string{"386", "i686"} + + for _, newArch := range similarArchs { + in := fakeInput() + in.Arch = newArch + in.TotalRAM = 1 * bytes.TB + + out, _ := computeArch(in, category.NewExportCfg(*in)) + Expect(out.Memory.SharedBuffers, ShouldBeLessThanOrEqualTo, 4*bytes.GB) + Expect(out.Memory.WorkMem, ShouldBeLessThanOrEqualTo, 4*bytes.GB) + Expect(out.Memory.MaintenanceWorkMem, ShouldBeLessThanOrEqualTo, 4*bytes.GB) + } + }) }) }) } diff --git a/pkg/rules/compute_test.go b/pkg/rules/compute_test.go index 3b448a3..e49f176 100644 --- a/pkg/rules/compute_test.go +++ b/pkg/rules/compute_test.go @@ -6,6 +6,7 @@ import ( "github.com/pgconfig/api/pkg/input" "github.com/pgconfig/api/pkg/input/bytes" "github.com/pgconfig/api/pkg/input/profile" + . "github.com/pgconfig/api/pkg/tests" ) func TestCompute(t *testing.T) { @@ -13,168 +14,77 @@ func TestCompute(t *testing.T) { } func TestComputeWalSizes(t *testing.T) { - tests := []struct { - name string - profile profile.Profile - totalRAM bytes.Byte - expectedMinWAL bytes.Byte - expectedMaxWAL bytes.Byte - description string - }{ - { - name: "DW profile gets large WAL sizes", - profile: profile.DW, - totalRAM: 100 * bytes.GB, - expectedMinWAL: 4 * bytes.GB, - expectedMaxWAL: 16 * bytes.GB, - description: "Data Warehouse is write-heavy with batch jobs", - }, - { - name: "OLTP profile gets medium-large WAL sizes", - profile: profile.OLTP, - totalRAM: 100 * bytes.GB, - expectedMinWAL: 2 * bytes.GB, - expectedMaxWAL: 8 * bytes.GB, - description: "OLTP has frequent transactions", - }, - { - name: "Web profile gets moderate WAL sizes", - profile: profile.Web, - totalRAM: 100 * bytes.GB, - expectedMinWAL: 1 * bytes.GB, - expectedMaxWAL: 4 * bytes.GB, - description: "Web has moderate writes", - }, - { - name: "Mixed profile gets balanced WAL sizes", - profile: profile.Mixed, - totalRAM: 100 * bytes.GB, - expectedMinWAL: 2 * bytes.GB, - expectedMaxWAL: 6 * bytes.GB, - description: "Mixed workload is balanced", - }, - { - name: "Desktop profile gets small WAL sizes", - profile: profile.Desktop, - totalRAM: 16 * bytes.GB, - expectedMinWAL: 512 * bytes.MB, - expectedMaxWAL: 2 * bytes.GB, - description: "Desktop has low activity", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - in := input.Input{ - OS: "linux", - Arch: "amd64", - Profile: tt.profile, - TotalRAM: tt.totalRAM, - MaxConnections: 100, - DiskType: "ssd", - TotalCPU: 8, - PostgresVersion: 16.0, - } - - out, err := Compute(in) - if err != nil { - t.Fatalf("Compute failed: %v", err) + Describe("WAL size computation", t, func() { + It("should configure WAL sizes based on profile", func() { + tests := []struct { + name string + profile profile.Profile + totalRAM bytes.Byte + expectedMinWAL bytes.Byte + expectedMaxWAL bytes.Byte + }{ + {"DW profile gets large WAL sizes", profile.DW, 100 * bytes.GB, 4 * bytes.GB, 16 * bytes.GB}, + {"OLTP profile gets medium-large WAL sizes", profile.OLTP, 100 * bytes.GB, 2 * bytes.GB, 8 * bytes.GB}, + {"Web profile gets moderate WAL sizes", profile.Web, 100 * bytes.GB, 1 * bytes.GB, 4 * bytes.GB}, + {"Mixed profile gets balanced WAL sizes", profile.Mixed, 100 * bytes.GB, 2 * bytes.GB, 6 * bytes.GB}, + {"Desktop profile gets small WAL sizes", profile.Desktop, 16 * bytes.GB, 512 * bytes.MB, 2 * bytes.GB}, } - if out.Checkpoint.MinWALSize != tt.expectedMinWAL { - t.Errorf("%s: expected min_wal_size = %v, got %v", - tt.description, tt.expectedMinWAL, out.Checkpoint.MinWALSize) - } + for _, tt := range tests { + in := input.Input{ + OS: "linux", + Arch: "amd64", + Profile: tt.profile, + TotalRAM: tt.totalRAM, + MaxConnections: 100, + DiskType: "ssd", + TotalCPU: 8, + PostgresVersion: 16.0, + } - if out.Checkpoint.MaxWALSize != tt.expectedMaxWAL { - t.Errorf("%s: expected max_wal_size = %v, got %v", - tt.description, tt.expectedMaxWAL, out.Checkpoint.MaxWALSize) + out, err := Compute(in) + Expect(err, ShouldBeNil) + Expect(out.Checkpoint.MinWALSize, ShouldEqual, tt.expectedMinWAL) + Expect(out.Checkpoint.MaxWALSize, ShouldEqual, tt.expectedMaxWAL) } }) - } + }) } func TestComputeWalBuffers(t *testing.T) { - tests := []struct { - name string - profile profile.Profile - totalRAM bytes.Byte - expectedWalBuffers bytes.Byte - description string - }{ - { - name: "DW profile always gets 64MB", - profile: profile.DW, - totalRAM: 100 * bytes.GB, - expectedWalBuffers: 64 * bytes.MB, - description: "Data Warehouse is write-heavy", - }, - { - name: "DW profile with small RAM still gets 64MB", - profile: profile.DW, - totalRAM: 8 * bytes.GB, - expectedWalBuffers: 64 * bytes.MB, - description: "DW always uses 64MB regardless of RAM", - }, - { - name: "OLTP with large shared_buffers gets 32MB", - profile: profile.OLTP, - totalRAM: 40 * bytes.GB, // shared_buffers = 40GB * 0.25 = 10GB > 8GB - expectedWalBuffers: 32 * bytes.MB, - description: "Large OLTP systems benefit from larger wal_buffers", - }, - { - name: "OLTP with small shared_buffers uses auto-tune", - profile: profile.OLTP, - totalRAM: 16 * bytes.GB, // shared_buffers = 16GB * 0.25 = 4GB < 8GB - expectedWalBuffers: -1, - description: "Small OLTP systems use auto-tuning", - }, - { - name: "Web profile uses auto-tune", - profile: profile.Web, - totalRAM: 100 * bytes.GB, - expectedWalBuffers: -1, - description: "Web workload uses default auto-tuning", - }, - { - name: "Mixed profile uses auto-tune", - profile: profile.Mixed, - totalRAM: 100 * bytes.GB, - expectedWalBuffers: -1, - description: "Mixed workload uses default auto-tuning", - }, - { - name: "Desktop profile uses auto-tune", - profile: profile.Desktop, - totalRAM: 16 * bytes.GB, - expectedWalBuffers: -1, - description: "Desktop workload uses default auto-tuning", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - in := input.Input{ - OS: "linux", - Arch: "amd64", - Profile: tt.profile, - TotalRAM: tt.totalRAM, - MaxConnections: 100, - DiskType: "ssd", - TotalCPU: 8, - PostgresVersion: 16.0, + Describe("WAL buffers computation", t, func() { + It("should configure wal_buffers based on profile and RAM", func() { + tests := []struct { + name string + profile profile.Profile + totalRAM bytes.Byte + expectedWalBuffers bytes.Byte + }{ + {"DW profile always gets 64MB", profile.DW, 100 * bytes.GB, 64 * bytes.MB}, + {"DW profile with small RAM still gets 64MB", profile.DW, 8 * bytes.GB, 64 * bytes.MB}, + {"OLTP with large shared_buffers gets 32MB", profile.OLTP, 40 * bytes.GB, 32 * bytes.MB}, + {"OLTP with small shared_buffers uses auto-tune", profile.OLTP, 16 * bytes.GB, -1}, + {"Web profile uses auto-tune", profile.Web, 100 * bytes.GB, -1}, + {"Mixed profile uses auto-tune", profile.Mixed, 100 * bytes.GB, -1}, + {"Desktop profile uses auto-tune", profile.Desktop, 16 * bytes.GB, -1}, } - out, err := Compute(in) - if err != nil { - t.Fatalf("Compute failed: %v", err) - } + for _, tt := range tests { + in := input.Input{ + OS: "linux", + Arch: "amd64", + Profile: tt.profile, + TotalRAM: tt.totalRAM, + MaxConnections: 100, + DiskType: "ssd", + TotalCPU: 8, + PostgresVersion: 16.0, + } - if out.Checkpoint.WALBuffers != tt.expectedWalBuffers { - t.Errorf("%s: expected wal_buffers = %v, got %v", - tt.description, tt.expectedWalBuffers, out.Checkpoint.WALBuffers) + out, err := Compute(in) + Expect(err, ShouldBeNil) + Expect(out.Checkpoint.WALBuffers, ShouldEqual, tt.expectedWalBuffers) } }) - } + }) } diff --git a/pkg/rules/os_test.go b/pkg/rules/os_test.go index 36c9095..0910c08 100644 --- a/pkg/rules/os_test.go +++ b/pkg/rules/os_test.go @@ -7,132 +7,130 @@ import ( "github.com/pgconfig/api/pkg/errors" "github.com/pgconfig/api/pkg/input" "github.com/pgconfig/api/pkg/input/bytes" - - . "github.com/smartystreets/goconvey/convey" + . "github.com/pgconfig/api/pkg/tests" ) func Test_computeOS(t *testing.T) { - - Convey("Operating systems", t, func() { - Convey("should return error on non-supported operating systems", func() { + Describe("Operating system handling", t, func() { + It("should return error on non-supported operating systems", func() { _, err := computeOS(&input.Input{OS: "xpto-wrong-os"}, &category.ExportCfg{}) - So(err, ShouldResemble, errors.ErrorInvalidOS) + Expect(err, ShouldResemble, errors.ErrorInvalidOS) }) - Convey("should ignore case for all operating systems supported", func() { + It("should ignore case for all operating systems supported", func() { in := fakeInput() in.OS = "lINUx" in.TotalRAM = 120 * bytes.GB _, err := computeOS(in, category.NewExportCfg(*in)) - So(err, ShouldBeNil) - }) - - Convey("should limit shared_buffers to 512MB until pg 10 on windows", func() { - in := fakeInput() - in.OS = "windows" - in.PostgresVersion = 9.6 - - out, err := computeOS(in, category.NewExportCfg(*in)) - So(err, ShouldBeNil) - So(out.Memory.SharedBuffers, ShouldEqual, 512*bytes.MB) - }) - - Convey("should limit effective_io_concurrency to 0 on platforms that lack posix_fadvise()", func() { - in := fakeInput() - in.OS = Windows - in.PostgresVersion = 12.0 - - out, err := computeOS(in, category.NewExportCfg(*in)) - So(err, ShouldBeNil) - So(out.Storage.EffectiveIOConcurrency, ShouldEqual, 0) - }) - - Convey("should not limit shared_buffers on versions greater or equal than pg 11", func() { - in := fakeInput() - in.PostgresVersion = 14.0 - in.TotalRAM = 120 * bytes.GB - - out, err := computeOS(in, category.NewExportCfg(*in)) - So(err, ShouldBeNil) - So(out.Memory.SharedBuffers, ShouldBeGreaterThan, 25*bytes.GB) - }) - - Convey("should limit work_mem to ~2GB on Windows (issue #5)", func() { - in := fakeInput() - in.OS = Windows - in.TotalRAM = 256 * bytes.GB - in.MaxConnections = 10 - in.PostgresVersion = 16.0 - - cfg := category.NewExportCfg(*in) - // Force work_mem to be higher than the limit - cfg.Memory.WorkMem = 5 * bytes.GB - - out, err := computeOS(in, cfg) - So(err, ShouldBeNil) - So(out.Memory.WorkMem, ShouldEqual, WindowsMaxWorkMem) - So(out.Memory.WorkMem, ShouldBeLessThan, 2*bytes.GB) - }) - - Convey("should limit maintenance_work_mem to ~2GB on Windows (issue #5)", func() { - in := fakeInput() - in.OS = Windows - in.TotalRAM = 256 * bytes.GB - in.PostgresVersion = 16.0 - - cfg := category.NewExportCfg(*in) - // Force maintenance_work_mem to be higher than the limit - cfg.Memory.MaintenanceWorkMem = 10 * bytes.GB - - out, err := computeOS(in, cfg) - So(err, ShouldBeNil) - So(out.Memory.MaintenanceWorkMem, ShouldEqual, WindowsMaxWorkMem) - So(out.Memory.MaintenanceWorkMem, ShouldBeLessThan, 2*bytes.GB) - }) - - Convey("should not limit work_mem on non-Windows platforms", func() { - in := fakeInput() - in.OS = Linux - in.TotalRAM = 256 * bytes.GB - in.MaxConnections = 10 - in.PostgresVersion = 16.0 - - cfg := category.NewExportCfg(*in) - cfg.Memory.WorkMem = 5 * bytes.GB - - out, err := computeOS(in, cfg) - So(err, ShouldBeNil) - So(out.Memory.WorkMem, ShouldEqual, 5*bytes.GB) + Expect(err, ShouldBeNil) }) - Convey("should not limit work_mem on Windows with PostgreSQL 18+", func() { - in := fakeInput() - in.OS = Windows - in.TotalRAM = 256 * bytes.GB - in.MaxConnections = 10 - in.PostgresVersion = 18.0 - - cfg := category.NewExportCfg(*in) - cfg.Memory.WorkMem = 5 * bytes.GB - - out, err := computeOS(in, cfg) - So(err, ShouldBeNil) - So(out.Memory.WorkMem, ShouldEqual, 5*bytes.GB) - }) - - Convey("should not limit maintenance_work_mem on Windows with PostgreSQL 18+", func() { - in := fakeInput() - in.OS = Windows - in.TotalRAM = 256 * bytes.GB - in.PostgresVersion = 18.0 - - cfg := category.NewExportCfg(*in) - cfg.Memory.MaintenanceWorkMem = 10 * bytes.GB - - out, err := computeOS(in, cfg) - So(err, ShouldBeNil) - So(out.Memory.MaintenanceWorkMem, ShouldEqual, 10*bytes.GB) + Context("Windows specific limits", func() { + It("should limit shared_buffers to 512MB until pg 10", func() { + in := fakeInput() + in.OS = "windows" + in.PostgresVersion = 9.6 + + out, err := computeOS(in, category.NewExportCfg(*in)) + Expect(err, ShouldBeNil) + Expect(out.Memory.SharedBuffers, ShouldEqual, 512*bytes.MB) + }) + + It("should limit effective_io_concurrency to 0 on platforms that lack posix_fadvise()", func() { + in := fakeInput() + in.OS = Windows + in.PostgresVersion = 12.0 + + out, err := computeOS(in, category.NewExportCfg(*in)) + Expect(err, ShouldBeNil) + Expect(out.Storage.EffectiveIOConcurrency, ShouldEqual, 0) + }) + + It("should not limit shared_buffers on versions >= pg 11", func() { + in := fakeInput() + in.PostgresVersion = 14.0 + in.TotalRAM = 120 * bytes.GB + + out, err := computeOS(in, category.NewExportCfg(*in)) + Expect(err, ShouldBeNil) + Expect(out.Memory.SharedBuffers, ShouldBeGreaterThan, 25*bytes.GB) + }) + + It("should limit work_mem to ~2GB on Windows (issue #5)", func() { + in := fakeInput() + in.OS = Windows + in.TotalRAM = 256 * bytes.GB + in.MaxConnections = 10 + in.PostgresVersion = 16.0 + + cfg := category.NewExportCfg(*in) + cfg.Memory.WorkMem = 5 * bytes.GB + + out, err := computeOS(in, cfg) + Expect(err, ShouldBeNil) + Expect(out.Memory.WorkMem, ShouldEqual, WindowsMaxWorkMem) + Expect(out.Memory.WorkMem, ShouldBeLessThan, 2*bytes.GB) + }) + + It("should limit maintenance_work_mem to ~2GB on Windows (issue #5)", func() { + in := fakeInput() + in.OS = Windows + in.TotalRAM = 256 * bytes.GB + in.PostgresVersion = 16.0 + + cfg := category.NewExportCfg(*in) + cfg.Memory.MaintenanceWorkMem = 10 * bytes.GB + + out, err := computeOS(in, cfg) + Expect(err, ShouldBeNil) + Expect(out.Memory.MaintenanceWorkMem, ShouldEqual, WindowsMaxWorkMem) + Expect(out.Memory.MaintenanceWorkMem, ShouldBeLessThan, 2*bytes.GB) + }) + + It("should not limit work_mem on non-Windows platforms", func() { + in := fakeInput() + in.OS = Linux + in.TotalRAM = 256 * bytes.GB + in.MaxConnections = 10 + in.PostgresVersion = 16.0 + + cfg := category.NewExportCfg(*in) + cfg.Memory.WorkMem = 5 * bytes.GB + + out, err := computeOS(in, cfg) + Expect(err, ShouldBeNil) + Expect(out.Memory.WorkMem, ShouldEqual, 5*bytes.GB) + }) + + It("should not limit work_mem on Windows with PostgreSQL 18+", func() { + in := fakeInput() + in.OS = Windows + in.TotalRAM = 256 * bytes.GB + in.MaxConnections = 10 + in.PostgresVersion = 18.0 + + cfg := category.NewExportCfg(*in) + cfg.Memory.WorkMem = 5 * bytes.GB + + out, err := computeOS(in, cfg) + Expect(err, ShouldBeNil) + Expect(out.Memory.WorkMem, ShouldEqual, 5*bytes.GB) + }) + + It("should not limit maintenance_work_mem on Windows with PostgreSQL 18+", func() { + in := fakeInput() + in.OS = Windows + in.TotalRAM = 256 * bytes.GB + in.PostgresVersion = 18.0 + + cfg := category.NewExportCfg(*in) + cfg.Memory.MaintenanceWorkMem = 10 * bytes.GB + + out, err := computeOS(in, cfg) + Expect(err, ShouldBeNil) + Expect(out.Memory.MaintenanceWorkMem, ShouldEqual, 10*bytes.GB) + }) }) }) } diff --git a/pkg/rules/profile_test.go b/pkg/rules/profile_test.go index 2d4b29c..538c9b6 100644 --- a/pkg/rules/profile_test.go +++ b/pkg/rules/profile_test.go @@ -6,16 +6,20 @@ import ( "github.com/pgconfig/api/pkg/category" "github.com/pgconfig/api/pkg/input/bytes" "github.com/pgconfig/api/pkg/input/profile" + . "github.com/pgconfig/api/pkg/tests" ) func Test_computeProfile(t *testing.T) { - in := fakeInput() - in.Profile = profile.Desktop - in.TotalRAM = 4 * bytes.GB + Describe("Profile computation", t, func() { + It("should apply lower shared_buffers for Desktop profile", func() { + in := fakeInput() + in.Profile = profile.Desktop + in.TotalRAM = 4 * bytes.GB - out, _ := computeProfile(in, category.NewExportCfg(*in)) + out, _ := computeProfile(in, category.NewExportCfg(*in)) - if in.Profile == profile.Desktop && out.Memory.SharedBuffers != (4*bytes.GB)/16 { - t.Error("should apply a lower value for shared_buffers on the Desktop profile") - } + expected := (4 * bytes.GB) / 16 + Expect(out.Memory.SharedBuffers, ShouldEqual, expected) + }) + }) } diff --git a/pkg/rules/storage_test.go b/pkg/rules/storage_test.go index 85a35f8..abc8735 100644 --- a/pkg/rules/storage_test.go +++ b/pkg/rules/storage_test.go @@ -7,122 +7,69 @@ import ( "github.com/pgconfig/api/pkg/input" "github.com/pgconfig/api/pkg/input/bytes" "github.com/pgconfig/api/pkg/input/profile" + . "github.com/pgconfig/api/pkg/tests" ) func Test_computeStorage(t *testing.T) { - in := fakeInput() - in.DiskType = "SSD" - outSSD, _ := computeStorage(in, category.NewExportCfg(*in)) - in.DiskType = "SAN" - outSAN, _ := computeStorage(in, category.NewExportCfg(*in)) - in.DiskType = "HDD" - outHDD, _ := computeStorage(in, category.NewExportCfg(*in)) + Describe("Storage configuration", t, func() { + It("should configure random_page_cost and io_concurrency based on disk type", func() { + in := fakeInput() + in.DiskType = "SSD" + outSSD, _ := computeStorage(in, category.NewExportCfg(*in)) + in.DiskType = "SAN" + outSAN, _ := computeStorage(in, category.NewExportCfg(*in)) + in.DiskType = "HDD" + outHDD, _ := computeStorage(in, category.NewExportCfg(*in)) - if outSSD.Storage.RandomPageCost > 1.1 || outSAN.Storage.RandomPageCost > 1.1 { - t.Error("should use lower values for random_page_cost on both SSD and SAN") - } + Expect(outSSD.Storage.RandomPageCost, ShouldBeLessThanOrEqualTo, 1.1) + Expect(outSAN.Storage.RandomPageCost, ShouldBeLessThanOrEqualTo, 1.1) - if outSSD.Storage.EffectiveIOConcurrency < 200 || outSAN.Storage.EffectiveIOConcurrency < 300 { - t.Error("should use higher values for effective_io_concurrency on both SSD and SAN") - } + Expect(outSSD.Storage.EffectiveIOConcurrency, ShouldBeGreaterThanOrEqualTo, 200) + Expect(outSAN.Storage.EffectiveIOConcurrency, ShouldBeGreaterThanOrEqualTo, 300) - if outHDD.Storage.EffectiveIOConcurrency > 2 { - t.Error("should use lower values for effective_io_concurrency on HDD drives") - } + Expect(outHDD.Storage.EffectiveIOConcurrency, ShouldBeLessThanOrEqualTo, 2) - // maintenance_io_concurrency should match effective_io_concurrency - if outSSD.Storage.MaintenanceIOConcurrency != outSSD.Storage.EffectiveIOConcurrency { - t.Error("maintenance_io_concurrency should match effective_io_concurrency for SSD") - } - if outSAN.Storage.MaintenanceIOConcurrency != outSAN.Storage.EffectiveIOConcurrency { - t.Error("maintenance_io_concurrency should match effective_io_concurrency for SAN") - } - if outHDD.Storage.MaintenanceIOConcurrency != outHDD.Storage.EffectiveIOConcurrency { - t.Error("maintenance_io_concurrency should match effective_io_concurrency for HDD") - } + Expect(outSSD.Storage.MaintenanceIOConcurrency, ShouldEqual, outSSD.Storage.EffectiveIOConcurrency) + Expect(outSAN.Storage.MaintenanceIOConcurrency, ShouldEqual, outSAN.Storage.EffectiveIOConcurrency) + Expect(outHDD.Storage.MaintenanceIOConcurrency, ShouldEqual, outHDD.Storage.EffectiveIOConcurrency) + }) + }) } func Test_computeStorageRandomPageCost(t *testing.T) { - tests := []struct { - name string - profile profile.Profile - diskType string - expectedRandomPageCost float32 - description string - }{ - { - name: "DW profile with SSD gets higher random_page_cost", - profile: profile.DW, - diskType: "SSD", - expectedRandomPageCost: 1.8, - description: "DW analytical queries favor sequential scans", - }, - { - name: "DW profile with SAN gets higher random_page_cost", - profile: profile.DW, - diskType: "SAN", - expectedRandomPageCost: 1.8, - description: "DW analytical queries favor sequential scans on SAN too", - }, - { - name: "DW profile with HDD keeps default", - profile: profile.DW, - diskType: "HDD", - expectedRandomPageCost: 4.0, - description: "HDD uses PostgreSQL default", - }, - { - name: "OLTP profile with SSD gets low random_page_cost", - profile: profile.OLTP, - diskType: "SSD", - expectedRandomPageCost: 1.1, - description: "OLTP favors index scans", - }, - { - name: "Web profile with SSD gets low random_page_cost", - profile: profile.Web, - diskType: "SSD", - expectedRandomPageCost: 1.1, - description: "Web workload favors index scans", - }, - { - name: "Mixed profile with SSD gets low random_page_cost", - profile: profile.Mixed, - diskType: "SSD", - expectedRandomPageCost: 1.1, - description: "Mixed workload uses general SSD value", - }, - { - name: "Desktop profile with SSD gets low random_page_cost", - profile: profile.Desktop, - diskType: "SSD", - expectedRandomPageCost: 1.1, - description: "Desktop uses general SSD value", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - in := input.Input{ - OS: "linux", - Arch: "amd64", - Profile: tt.profile, - DiskType: tt.diskType, - TotalRAM: 100 * bytes.GB, - MaxConnections: 100, - TotalCPU: 8, - PostgresVersion: 16.0, + Describe("Random page cost computation", t, func() { + It("should configure random_page_cost based on profile and disk type", func() { + tests := []struct { + name string + profile profile.Profile + diskType string + expectedPageCost float32 + }{ + {"DW profile with SSD gets higher random_page_cost", profile.DW, "SSD", 1.8}, + {"DW profile with SAN gets higher random_page_cost", profile.DW, "SAN", 1.8}, + {"DW profile with HDD keeps default", profile.DW, "HDD", 4.0}, + {"OLTP profile with SSD gets low random_page_cost", profile.OLTP, "SSD", 1.1}, + {"Web profile with SSD gets low random_page_cost", profile.Web, "SSD", 1.1}, + {"Mixed profile with SSD gets low random_page_cost", profile.Mixed, "SSD", 1.1}, + {"Desktop profile with SSD gets low random_page_cost", profile.Desktop, "SSD", 1.1}, } - out, err := computeStorage(&in, category.NewExportCfg(in)) - if err != nil { - t.Fatalf("computeStorage failed: %v", err) - } + for _, tt := range tests { + in := input.Input{ + OS: "linux", + Arch: "amd64", + Profile: tt.profile, + DiskType: tt.diskType, + TotalRAM: 100 * bytes.GB, + MaxConnections: 100, + TotalCPU: 8, + PostgresVersion: 16.0, + } - if out.Storage.RandomPageCost != tt.expectedRandomPageCost { - t.Errorf("%s: expected random_page_cost = %v, got %v", - tt.description, tt.expectedRandomPageCost, out.Storage.RandomPageCost) + out, err := computeStorage(&in, category.NewExportCfg(in)) + Expect(err, ShouldBeNil) + Expect(out.Storage.RandomPageCost, ShouldEqual, tt.expectedPageCost) } }) - } + }) } diff --git a/pkg/rules/version_test.go b/pkg/rules/version_test.go index 63ac35d..a2e88d8 100644 --- a/pkg/rules/version_test.go +++ b/pkg/rules/version_test.go @@ -5,93 +5,125 @@ import ( "github.com/pgconfig/api/pkg/category" "github.com/pgconfig/api/pkg/input/bytes" + . "github.com/pgconfig/api/pkg/tests" ) func Test_computeVersion(t *testing.T) { - in := fakeInput() - in.PostgresVersion = 9.4 + Describe("PostgreSQL version-specific settings", t, func() { + Context("versions older than 9.5", func() { + It("should remove min_wal_size and max_wal_size", func() { + in := fakeInput() + in.PostgresVersion = 9.4 - out, _ := computeVersion(in, category.NewExportCfg(*in)) + out, _ := computeVersion(in, category.NewExportCfg(*in)) - if out.Checkpoint.MinWALSize > 0 || out.Checkpoint.MaxWALSize > 0 { - t.Error("should remove min_wal_size and max_wal_size on versions older than 9.5") - } + Expect(out.Checkpoint.MinWALSize, ShouldEqual, 0) + Expect(out.Checkpoint.MaxWALSize, ShouldEqual, 0) + }) - in = fakeInput() - in.PostgresVersion = 9.5 + It("should limit shared_buffers up to 8gb", func() { + in := fakeInput() + in.PostgresVersion = 9.5 + in.TotalRAM = 1 * bytes.TB - out, _ = computeVersion(in, category.NewExportCfg(*in)) + out, _ := computeVersion(in, category.NewExportCfg(*in)) - if out.Checkpoint.CheckpointSegments > 0 { - t.Error("should remove checkpoint_segments on versions greater than 9.5") - } + Expect(out.Memory.SharedBuffers, ShouldBeLessThanOrEqualTo, 8*bytes.GB) + }) + }) - in = fakeInput() - in.PostgresVersion = 9.3 + Context("versions 9.5 and newer", func() { + It("should remove checkpoint_segments", func() { + in := fakeInput() + in.PostgresVersion = 9.5 - out, _ = computeVersion(in, category.NewExportCfg(*in)) + out, _ := computeVersion(in, category.NewExportCfg(*in)) - if out.Worker != nil { - t.Error("should remove the workers category in versions older than 9.3") - } + Expect(out.Checkpoint.CheckpointSegments, ShouldEqual, 0) + }) + }) - in = fakeInput() - in.PostgresVersion = 9.4 + Context("versions older than 9.6", func() { + It("should remove max_parallel_workers_per_gather", func() { + in := fakeInput() + in.PostgresVersion = 9.4 - out, _ = computeVersion(in, category.NewExportCfg(*in)) + out, _ := computeVersion(in, category.NewExportCfg(*in)) - if out.Worker.MaxParallelWorkerPerGather != 0 { - t.Error("should remove max_parallel_workers_per_gather on versions < 9.6") - } + Expect(out.Worker.MaxParallelWorkerPerGather, ShouldEqual, 0) + }) - in = fakeInput() - in.PostgresVersion = 9.5 + It("should remove the workers category in versions older than 9.3", func() { + in := fakeInput() + in.PostgresVersion = 9.3 - out, _ = computeVersion(in, category.NewExportCfg(*in)) + out, _ := computeVersion(in, category.NewExportCfg(*in)) - if out.Worker.MaxParallelWorkers != 0 { - t.Error("should remove max_parallel_workers on versions < 10") - } + Expect(out.Worker, ShouldBeNil) + }) + }) - in = fakeInput() - in.PostgresVersion = 9.5 - in.TotalRAM = 1 * bytes.TB + Context("versions older than 10", func() { + It("should remove max_parallel_workers", func() { + in := fakeInput() + in.PostgresVersion = 9.5 - out, _ = computeVersion(in, category.NewExportCfg(*in)) + out, _ := computeVersion(in, category.NewExportCfg(*in)) - if out.Memory.SharedBuffers > 8*bytes.GB { - t.Error("should limit shared_buffers up to 8gb on versions <= 9.5") - } + Expect(out.Worker.MaxParallelWorkers, ShouldEqual, 0) + }) + }) - in = fakeInput() - in.PostgresVersion = 12.0 - out, _ = computeVersion(in, category.NewExportCfg(*in)) + Context("versions older than 13", func() { + It("should zero maintenance_io_concurrency", func() { + in := fakeInput() + in.PostgresVersion = 12.0 - if out.Storage.MaintenanceIOConcurrency != 0 { - t.Error("should zero maintenance_io_concurrency for versions < 13") - } + out, _ := computeVersion(in, category.NewExportCfg(*in)) - in = fakeInput() - in.PostgresVersion = 13.0 - out, _ = computeVersion(in, category.NewExportCfg(*in)) + Expect(out.Storage.MaintenanceIOConcurrency, ShouldEqual, 0) + }) + }) - if out.Storage.MaintenanceIOConcurrency == 0 { - t.Error("should keep maintenance_io_concurrency for versions >= 13") - } + Context("versions 13 and newer", func() { + It("should keep maintenance_io_concurrency", func() { + in := fakeInput() + in.PostgresVersion = 13.0 - in = fakeInput() - in.PostgresVersion = 17.0 - out, _ = computeVersion(in, category.NewExportCfg(*in)) + out, _ := computeVersion(in, category.NewExportCfg(*in)) - if out.Storage.IOMethod != "" || out.Storage.IOWorkers != 0 || out.Storage.IOMaxCombineLimit != 0 || out.Storage.IOMaxConcurrency != 0 || out.Storage.FileCopyMethod != "" { - t.Error("should zero all AIO-related parameters for versions < 18") - } + Expect(out.Storage.MaintenanceIOConcurrency, ShouldNotEqual, 0) + }) + }) - in = fakeInput() - in.PostgresVersion = 18.0 - out, _ = computeVersion(in, category.NewExportCfg(*in)) + Context("versions older than 18", func() { + It("should zero all AIO-related parameters", func() { + in := fakeInput() + in.PostgresVersion = 17.0 - if out.Storage.IOMethod == "" || out.Storage.IOWorkers == 0 || out.Storage.IOMaxCombineLimit == 0 || out.Storage.IOMaxConcurrency == 0 || out.Storage.FileCopyMethod == "" { - t.Error("should keep AIO-related parameters for versions >= 18") - } + out, _ := computeVersion(in, category.NewExportCfg(*in)) + + Expect(out.Storage.IOMethod, ShouldEqual, "") + Expect(out.Storage.IOWorkers, ShouldEqual, 0) + Expect(out.Storage.IOMaxCombineLimit, ShouldEqual, 0) + Expect(out.Storage.IOMaxConcurrency, ShouldEqual, 0) + Expect(out.Storage.FileCopyMethod, ShouldEqual, "") + }) + }) + + Context("versions 18 and newer", func() { + It("should keep AIO-related parameters", func() { + in := fakeInput() + in.PostgresVersion = 18.0 + + out, _ := computeVersion(in, category.NewExportCfg(*in)) + + Expect(out.Storage.IOMethod, ShouldNotEqual, "") + Expect(out.Storage.IOWorkers, ShouldNotEqual, 0) + Expect(out.Storage.IOMaxCombineLimit, ShouldNotEqual, 0) + Expect(out.Storage.IOMaxConcurrency, ShouldNotEqual, 0) + Expect(out.Storage.FileCopyMethod, ShouldNotEqual, "") + }) + }) + }) }