From 425f607c18bc673772fdb3d78af08f304489635a Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 11 Nov 2025 17:12:27 +0100 Subject: [PATCH 1/6] starting to add some sanity checks for Ruby platform to give feedback when it cannot work --- internal/platform/platform.go | 5 ++ internal/platform/ruby.go | 103 ++++++++++++++++++++++ internal/platform/ruby_test.go | 147 +++++++++++++++++++++++++++---- internal/runner/runner_test.go | 5 ++ internal/version/version.go | 129 +++++++++++++++++++++++++++ internal/version/version_test.go | 90 +++++++++++++++++++ 6 files changed, 463 insertions(+), 16 deletions(-) create mode 100644 internal/version/version.go create mode 100644 internal/version/version_test.go diff --git a/internal/platform/platform.go b/internal/platform/platform.go index adb7eea..87b7317 100644 --- a/internal/platform/platform.go +++ b/internal/platform/platform.go @@ -11,6 +11,7 @@ type Platform interface { Name() string CreateTagsMap() (map[string]string, error) DetectFramework() (framework.Framework, error) + SanityCheck() error } // PlatformDetector defines interface for detecting platforms - needed to allow mocking in tests @@ -35,6 +36,10 @@ func DetectPlatform() (Platform, error) { return nil, fmt.Errorf("unsupported platform: %s", platformName) } + if err := platform.SanityCheck(); err != nil { + return nil, fmt.Errorf("sanity check failed for platform %s: %w", platform.Name(), err) + } + return platform, nil } diff --git a/internal/platform/ruby.go b/internal/platform/ruby.go index 972420d..0b55326 100644 --- a/internal/platform/ruby.go +++ b/internal/platform/ruby.go @@ -7,11 +7,14 @@ import ( "fmt" "maps" "os" + "path/filepath" + "strings" "github.com/DataDog/ddtest/internal/constants" "github.com/DataDog/ddtest/internal/ext" "github.com/DataDog/ddtest/internal/framework" "github.com/DataDog/ddtest/internal/settings" + "github.com/DataDog/ddtest/internal/version" ) //go:embed scripts/ruby_env.rb @@ -19,11 +22,17 @@ var rubyEnvScript string type Ruby struct { executor ext.CommandExecutor + rootDir string } func NewRuby() *Ruby { + wd, err := os.Getwd() + if err != nil { + wd = "." + } return &Ruby{ executor: &ext.DefaultCommandExecutor{}, + rootDir: wd, } } @@ -80,3 +89,97 @@ func (r *Ruby) DetectFramework() (framework.Framework, error) { return nil, fmt.Errorf("framework '%s' is not supported by platform 'ruby'", frameworkName) } } + +func (r *Ruby) SanityCheck() error { + const ( + requiredGemName = "datadog-ci" + minimumGemVersion = "1.23.0" + gemfileName = "Gemfile" + ) + + gemfilePath := r.resolvePath(gemfileName) + if _, err := os.Stat(gemfilePath); err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("gemfile not found at %s", gemfilePath) + } + return fmt.Errorf("failed to stat gemfile at %s: %w", gemfilePath, err) + } + + args := []string{"info", requiredGemName} + env := map[string]string{ + "BUNDLE_GEMFILE": gemfilePath, + } + + output, err := r.executor.CombinedOutput(context.Background(), "bundle", args, env) + if err != nil { + message := strings.TrimSpace(string(output)) + if message == "" { + return fmt.Errorf("bundle info datadog-ci command failed: %w", err) + } + return fmt.Errorf("bundle info datadog-ci command failed: %s", message) + } + + requiredVersion, err := version.Parse(minimumGemVersion) + if err != nil { + return err + } + + gemVersion, err := parseBundlerInfoVersion(string(output), requiredGemName) + if err != nil { + return err + } + + if gemVersion.Compare(requiredVersion) < 0 { + return fmt.Errorf("datadog-ci gem version %s is lower than required >= %s", gemVersion.String(), requiredVersion.String()) + } + + return nil +} + +func (r *Ruby) resolvePath(name string) string { + root := r.rootDir + if root == "" || root == "." { + return name + } + return filepath.Join(root, name) +} + +func parseBundlerInfoVersion(output, gemName string) (version.Version, error) { + lines := strings.Split(output, "\n") + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed == "" { + continue + } + + if !strings.Contains(trimmed, gemName) { + continue + } + + start := strings.Index(trimmed, "(") + end := strings.Index(trimmed, ")") + if start == -1 || end == -1 || end <= start+1 { + continue + } + + versionToken := strings.TrimSpace(trimmed[start+1 : end]) + if versionToken == "" { + continue + } + + fields := strings.Fields(versionToken) + versionString := fields[0] + if !version.IsValid(versionString) { + return version.Version{}, fmt.Errorf("unexpected version format in bundle info output: %q", versionToken) + } + + parsed, err := version.Parse(versionString) + if err != nil { + return version.Version{}, fmt.Errorf("failed to parse version from bundle info output: %w", err) + } + + return parsed, nil + } + + return version.Version{}, fmt.Errorf("unable to find datadog-ci gem version in bundle info output") +} diff --git a/internal/platform/ruby_test.go b/internal/platform/ruby_test.go index a3e31ab..68b6651 100644 --- a/internal/platform/ruby_test.go +++ b/internal/platform/ruby_test.go @@ -5,32 +5,53 @@ import ( "encoding/json" "os" "os/exec" + "path/filepath" "strings" "testing" "github.com/DataDog/ddtest/internal/constants" + "github.com/DataDog/ddtest/internal/ext" "github.com/DataDog/ddtest/internal/settings" "github.com/spf13/viper" ) type mockCommandExecutor struct { - output []byte - err error - onExecution func(name string, args []string) + runErr error + combinedOutput []byte + combinedOutputErr error + onRun func(name string, args []string, envMap map[string]string) + onCombinedOutput func(name string, args []string, envMap map[string]string) } func (m *mockCommandExecutor) CombinedOutput(ctx context.Context, name string, args []string, envMap map[string]string) ([]byte, error) { - if m.onExecution != nil { - m.onExecution(name, args) + if m.onCombinedOutput != nil { + m.onCombinedOutput(name, args, envMap) } - return m.output, m.err + return m.combinedOutput, m.combinedOutputErr } func (m *mockCommandExecutor) Run(ctx context.Context, name string, args []string, envMap map[string]string) error { - if m.onExecution != nil { - m.onExecution(name, args) + if m.onRun != nil { + m.onRun(name, args, envMap) } - return m.err + return m.runErr +} + +func writeTestFile(t *testing.T, dir, name, content string) { + t.Helper() + path := filepath.Join(dir, name) + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write %s: %v", path, err) + } +} + +func newRubyForDir(dir string, executor ext.CommandExecutor) *Ruby { + ruby := NewRuby() + ruby.rootDir = dir + if executor != nil { + ruby.executor = executor + } + return ruby } func TestRuby_Name(t *testing.T) { @@ -43,6 +64,103 @@ func TestRuby_Name(t *testing.T) { } } +func TestRuby_SanityCheck_Passes(t *testing.T) { + tempDir := t.TempDir() + writeTestFile(t, tempDir, "Gemfile", "source \"https://rubygems.org\"\n\ngem \"datadog-ci\"\n") + + mockExecutor := &mockCommandExecutor{ + combinedOutput: []byte(" * datadog-ci (1.23.1 9d54a15)\n"), + onCombinedOutput: func(name string, args []string, envMap map[string]string) { + if name != "bundle" { + t.Fatalf("expected command 'bundle', got %q", name) + } + if len(args) != 2 || args[0] != "info" || args[1] != "datadog-ci" { + t.Fatalf("unexpected args: %v", args) + } + if envMap["BUNDLE_GEMFILE"] != filepath.Join(tempDir, "Gemfile") { + t.Fatalf("expected BUNDLE_GEMFILE to be %s, got %s", filepath.Join(tempDir, "Gemfile"), envMap["BUNDLE_GEMFILE"]) + } + }, + } + + ruby := newRubyForDir(tempDir, mockExecutor) + if err := ruby.SanityCheck(); err != nil { + t.Fatalf("SanityCheck() unexpected error: %v", err) + } +} + +func TestRuby_SanityCheck_FailsWhenGemfileMissing(t *testing.T) { + tempDir := t.TempDir() + + ruby := newRubyForDir(tempDir, nil) + err := ruby.SanityCheck() + if err == nil { + t.Fatal("SanityCheck() expected error when Gemfile is missing") + } + + if !strings.Contains(err.Error(), "gemfile") { + t.Fatalf("expected error to mention gemfile, got: %v", err) + } +} + +func TestRuby_SanityCheck_FailsWhenBundleInfoFails(t *testing.T) { + tempDir := t.TempDir() + writeTestFile(t, tempDir, "Gemfile", "source \"https://rubygems.org\"\n\ngem \"datadog-ci\"\n") + + mockExecutor := &mockCommandExecutor{ + combinedOutput: []byte("Could not find gem 'datadog-ci'."), + combinedOutputErr: &exec.ExitError{}, + } + + ruby := newRubyForDir(tempDir, mockExecutor) + err := ruby.SanityCheck() + if err == nil { + t.Fatal("SanityCheck() expected error when bundle info fails") + } + + if !strings.Contains(err.Error(), "Could not find gem") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestRuby_SanityCheck_FailsWhenVersionTooLow(t *testing.T) { + tempDir := t.TempDir() + writeTestFile(t, tempDir, "Gemfile", "source \"https://rubygems.org\"\n\ngem \"datadog-ci\"\n") + + mockExecutor := &mockCommandExecutor{ + combinedOutput: []byte(" * datadog-ci (1.22.5)\n"), + } + + ruby := newRubyForDir(tempDir, mockExecutor) + err := ruby.SanityCheck() + if err == nil { + t.Fatal("SanityCheck() expected error for outdated datadog-ci version") + } + + if !strings.Contains(err.Error(), "1.22.5") { + t.Fatalf("expected error to mention detected version, got: %v", err) + } +} + +func TestRuby_SanityCheck_FailsWhenVersionNotFound(t *testing.T) { + tempDir := t.TempDir() + writeTestFile(t, tempDir, "Gemfile", "source \"https://rubygems.org\"\n\ngem \"datadog-ci\"\n") + + mockExecutor := &mockCommandExecutor{ + combinedOutput: []byte(" * datadog-ci\n Summary: Datadog Test Optimization for your ruby application\n"), + } + + ruby := newRubyForDir(tempDir, mockExecutor) + err := ruby.SanityCheck() + if err == nil { + t.Fatal("SanityCheck() expected error when version is not found") + } + + if !strings.Contains(err.Error(), "unable to find datadog-ci gem version") { + t.Fatalf("unexpected error: %v", err) + } +} + func TestRuby_DetectFramework_RSpec(t *testing.T) { viper.Reset() viper.Set("framework", "rspec") @@ -140,8 +258,7 @@ func TestRuby_CreateTagsMap_Success(t *testing.T) { } mockExecutor := &mockCommandExecutor{ - err: nil, - onExecution: func(name string, args []string) { + onRun: func(name string, args []string, envMap map[string]string) { // Verify the command is correct if name != "bundle" { t.Errorf("expected command to be 'bundle', got %q", name) @@ -209,9 +326,8 @@ func TestRuby_CreateTagsMap_CommandFailure(t *testing.T) { }() mockExecutor := &mockCommandExecutor{ - output: []byte("bundle: command not found"), - err: &exec.ExitError{}, - onExecution: func(name string, args []string) { + runErr: &exec.ExitError{}, + onRun: func(name string, args []string, envMap map[string]string) { // Command fails, don't create any file }, } @@ -243,8 +359,7 @@ func TestRuby_CreateTagsMap_InvalidJSON(t *testing.T) { invalidJSON := `{invalid json}` mockExecutor := &mockCommandExecutor{ - err: nil, - onExecution: func(name string, args []string) { + onRun: func(name string, args []string, envMap map[string]string) { // Get the temp file path from the last argument if len(args) < 5 { t.Errorf("expected at least 5 args, got %d", len(args)) diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index cab050d..55605fa 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -41,6 +41,7 @@ type MockPlatform struct { TagsErr error Framework framework.Framework FrameworkErr error + SanityErr error } func (m *MockPlatform) Name() string { @@ -55,6 +56,10 @@ func (m *MockPlatform) DetectFramework() (framework.Framework, error) { return m.Framework, m.FrameworkErr } +func (m *MockPlatform) SanityCheck() error { + return m.SanityErr +} + // MockFramework mocks a testing framework type MockFramework struct { FrameworkName string diff --git a/internal/version/version.go b/internal/version/version.go new file mode 100644 index 0000000..5225a77 --- /dev/null +++ b/internal/version/version.go @@ -0,0 +1,129 @@ +package version + +import ( + "fmt" + "strconv" + "strings" +) + +// Version represents a semantic-like version composed of dot-separated integer components. +type Version struct { + raw string + components []int +} + +// Parse converts a raw string into a Version, ensuring it only contains numeric dot-separated components. +func Parse(raw string) (Version, error) { + trimmed := strings.TrimSpace(raw) + if trimmed == "" { + return Version{}, fmt.Errorf("version string is empty") + } + + parts := strings.Split(trimmed, ".") + components := make([]int, len(parts)) + + for i, part := range parts { + if part == "" { + return Version{}, fmt.Errorf("invalid version component %q", part) + } + + for _, r := range part { + if r < '0' || r > '9' { + return Version{}, fmt.Errorf("invalid version component %q", part) + } + } + + value, err := strconv.Atoi(part) + if err != nil { + return Version{}, fmt.Errorf("invalid version component %q: %w", part, err) + } + components[i] = value + } + + return Version{ + raw: strings.Join(parts, "."), + components: components, + }, nil +} + +// MustParse converts a raw string into a Version, panicking if parsing fails. +func MustParse(raw string) Version { + v, err := Parse(raw) + if err != nil { + panic(err) + } + return v +} + +// IsValid reports whether the provided string can be parsed into a Version. +func IsValid(raw string) bool { + _, err := Parse(raw) + return err == nil +} + +// Compare returns 1 if the receiver is greater than other, -1 if less, and 0 if equal. +func (v Version) Compare(other Version) int { + maxLen := max(len(v.components), len(other.components)) + + for i := 0; i < maxLen; i++ { + var a, b int + if i < len(v.components) { + a = v.components[i] + } + if i < len(other.components) { + b = other.components[i] + } + + if a > b { + return 1 + } + if a < b { + return -1 + } + } + + return 0 +} + +// String returns the normalized string representation of the version. +func (v Version) String() string { + if v.raw != "" { + return v.raw + } + + if len(v.components) == 0 { + return "" + } + + strParts := make([]string, len(v.components)) + for i, component := range v.components { + strParts[i] = strconv.Itoa(component) + } + + return strings.Join(strParts, ".") +} + +// Components returns a copy of the underlying version components. +func (v Version) Components() []int { + if len(v.components) == 0 { + return nil + } + result := make([]int, len(v.components)) + copy(result, v.components) + return result +} + +// CompareStrings parses the provided strings as versions and compares them. +func CompareStrings(a, b string) (int, error) { + vA, err := Parse(a) + if err != nil { + return 0, fmt.Errorf("invalid version %q: %w", a, err) + } + + vB, err := Parse(b) + if err != nil { + return 0, fmt.Errorf("invalid version %q: %w", b, err) + } + + return vA.Compare(vB), nil +} diff --git a/internal/version/version_test.go b/internal/version/version_test.go new file mode 100644 index 0000000..c3d7047 --- /dev/null +++ b/internal/version/version_test.go @@ -0,0 +1,90 @@ +package version + +import "testing" + +func TestParseValid(t *testing.T) { + input := "1.23.4" + v, err := Parse(input) + if err != nil { + t.Fatalf("Parse(%q) returned error: %v", input, err) + } + + if v.String() != input { + t.Fatalf("expected String() to return %q, got %q", input, v.String()) + } + + expectedComponents := []int{1, 23, 4} + actual := v.Components() + if len(actual) != len(expectedComponents) { + t.Fatalf("expected %d components, got %d", len(expectedComponents), len(actual)) + } + + for i, expected := range expectedComponents { + if actual[i] != expected { + t.Fatalf("expected component %d to be %d, got %d", i, expected, actual[i]) + } + } +} + +func TestParseInvalid(t *testing.T) { + cases := []string{ + "", + " ", + "1..2", + "1.2.a", + "1.-2", + } + + for _, input := range cases { + if _, err := Parse(input); err == nil { + t.Fatalf("expected Parse(%q) to fail", input) + } + } +} + +func TestIsValid(t *testing.T) { + if !IsValid("1.0.0") { + t.Fatal("expected 1.0.0 to be valid") + } + + if IsValid("1.0.beta") { + t.Fatal("expected 1.0.beta to be invalid") + } +} + +func TestCompare(t *testing.T) { + v1 := MustParse("1.2.3") + v2 := MustParse("1.2.3") + v3 := MustParse("1.3.0") + v4 := MustParse("1.2") + + if v1.Compare(v2) != 0 { + t.Fatal("expected equal versions to compare as 0") + } + + if v1.Compare(v3) >= 0 { + t.Fatal("expected v1 < v3") + } + + if v3.Compare(v1) <= 0 { + t.Fatal("expected v3 > v1") + } + + if v1.Compare(v4) <= 0 { + t.Fatal("expected v1 > v4 when compared with shorter version") + } +} + +func TestCompareStrings(t *testing.T) { + cmp, err := CompareStrings("1.2.3", "1.2.4") + if err != nil { + t.Fatalf("CompareStrings returned error: %v", err) + } + if cmp >= 0 { + t.Fatal("expected 1.2.3 < 1.2.4") + } + + if _, err := CompareStrings("1.2", "1.a"); err == nil { + t.Fatal("expected invalid version comparison to fail") + } +} From 4c0114ce065557643b2ad9c9446a284083020735 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 12 Nov 2025 09:38:01 +0100 Subject: [PATCH 2/6] remove check for Gemfile in the current folder when doing sanity check for Ruby --- internal/platform/ruby.go | 39 ++-------------- internal/platform/ruby_test.go | 83 ++++++---------------------------- 2 files changed, 19 insertions(+), 103 deletions(-) diff --git a/internal/platform/ruby.go b/internal/platform/ruby.go index 0b55326..fa42648 100644 --- a/internal/platform/ruby.go +++ b/internal/platform/ruby.go @@ -7,7 +7,6 @@ import ( "fmt" "maps" "os" - "path/filepath" "strings" "github.com/DataDog/ddtest/internal/constants" @@ -22,17 +21,11 @@ var rubyEnvScript string type Ruby struct { executor ext.CommandExecutor - rootDir string } func NewRuby() *Ruby { - wd, err := os.Getwd() - if err != nil { - wd = "." - } return &Ruby{ executor: &ext.DefaultCommandExecutor{}, - rootDir: wd, } } @@ -91,26 +84,11 @@ func (r *Ruby) DetectFramework() (framework.Framework, error) { } func (r *Ruby) SanityCheck() error { - const ( - requiredGemName = "datadog-ci" - minimumGemVersion = "1.23.0" - gemfileName = "Gemfile" - ) - - gemfilePath := r.resolvePath(gemfileName) - if _, err := os.Stat(gemfilePath); err != nil { - if os.IsNotExist(err) { - return fmt.Errorf("gemfile not found at %s", gemfilePath) - } - return fmt.Errorf("failed to stat gemfile at %s: %w", gemfilePath, err) - } + requiredGemName := "datadog-ci" + minimumGemVersion := "1.23.0" args := []string{"info", requiredGemName} - env := map[string]string{ - "BUNDLE_GEMFILE": gemfilePath, - } - - output, err := r.executor.CombinedOutput(context.Background(), "bundle", args, env) + output, err := r.executor.CombinedOutput(context.Background(), "bundle", args, nil) if err != nil { message := strings.TrimSpace(string(output)) if message == "" { @@ -136,17 +114,8 @@ func (r *Ruby) SanityCheck() error { return nil } -func (r *Ruby) resolvePath(name string) string { - root := r.rootDir - if root == "" || root == "." { - return name - } - return filepath.Join(root, name) -} - func parseBundlerInfoVersion(output, gemName string) (version.Version, error) { - lines := strings.Split(output, "\n") - for _, line := range lines { + for line := range strings.SplitSeq(output, "\n") { trimmed := strings.TrimSpace(line) if trimmed == "" { continue diff --git a/internal/platform/ruby_test.go b/internal/platform/ruby_test.go index 68b6651..dad5061 100644 --- a/internal/platform/ruby_test.go +++ b/internal/platform/ruby_test.go @@ -5,12 +5,10 @@ import ( "encoding/json" "os" "os/exec" - "path/filepath" "strings" "testing" "github.com/DataDog/ddtest/internal/constants" - "github.com/DataDog/ddtest/internal/ext" "github.com/DataDog/ddtest/internal/settings" "github.com/spf13/viper" ) @@ -37,23 +35,6 @@ func (m *mockCommandExecutor) Run(ctx context.Context, name string, args []strin return m.runErr } -func writeTestFile(t *testing.T, dir, name, content string) { - t.Helper() - path := filepath.Join(dir, name) - if err := os.WriteFile(path, []byte(content), 0o644); err != nil { - t.Fatalf("failed to write %s: %v", path, err) - } -} - -func newRubyForDir(dir string, executor ext.CommandExecutor) *Ruby { - ruby := NewRuby() - ruby.rootDir = dir - if executor != nil { - ruby.executor = executor - } - return ruby -} - func TestRuby_Name(t *testing.T) { ruby := NewRuby() expected := "ruby" @@ -65,9 +46,6 @@ func TestRuby_Name(t *testing.T) { } func TestRuby_SanityCheck_Passes(t *testing.T) { - tempDir := t.TempDir() - writeTestFile(t, tempDir, "Gemfile", "source \"https://rubygems.org\"\n\ngem \"datadog-ci\"\n") - mockExecutor := &mockCommandExecutor{ combinedOutput: []byte(" * datadog-ci (1.23.1 9d54a15)\n"), onCombinedOutput: func(name string, args []string, envMap map[string]string) { @@ -77,42 +55,24 @@ func TestRuby_SanityCheck_Passes(t *testing.T) { if len(args) != 2 || args[0] != "info" || args[1] != "datadog-ci" { t.Fatalf("unexpected args: %v", args) } - if envMap["BUNDLE_GEMFILE"] != filepath.Join(tempDir, "Gemfile") { - t.Fatalf("expected BUNDLE_GEMFILE to be %s, got %s", filepath.Join(tempDir, "Gemfile"), envMap["BUNDLE_GEMFILE"]) - } }, } - ruby := newRubyForDir(tempDir, mockExecutor) + ruby := NewRuby() + ruby.executor = mockExecutor if err := ruby.SanityCheck(); err != nil { t.Fatalf("SanityCheck() unexpected error: %v", err) } } -func TestRuby_SanityCheck_FailsWhenGemfileMissing(t *testing.T) { - tempDir := t.TempDir() - - ruby := newRubyForDir(tempDir, nil) - err := ruby.SanityCheck() - if err == nil { - t.Fatal("SanityCheck() expected error when Gemfile is missing") - } - - if !strings.Contains(err.Error(), "gemfile") { - t.Fatalf("expected error to mention gemfile, got: %v", err) - } -} - func TestRuby_SanityCheck_FailsWhenBundleInfoFails(t *testing.T) { - tempDir := t.TempDir() - writeTestFile(t, tempDir, "Gemfile", "source \"https://rubygems.org\"\n\ngem \"datadog-ci\"\n") - mockExecutor := &mockCommandExecutor{ combinedOutput: []byte("Could not find gem 'datadog-ci'."), combinedOutputErr: &exec.ExitError{}, } - ruby := newRubyForDir(tempDir, mockExecutor) + ruby := NewRuby() + ruby.executor = mockExecutor err := ruby.SanityCheck() if err == nil { t.Fatal("SanityCheck() expected error when bundle info fails") @@ -124,14 +84,12 @@ func TestRuby_SanityCheck_FailsWhenBundleInfoFails(t *testing.T) { } func TestRuby_SanityCheck_FailsWhenVersionTooLow(t *testing.T) { - tempDir := t.TempDir() - writeTestFile(t, tempDir, "Gemfile", "source \"https://rubygems.org\"\n\ngem \"datadog-ci\"\n") - mockExecutor := &mockCommandExecutor{ combinedOutput: []byte(" * datadog-ci (1.22.5)\n"), } - ruby := newRubyForDir(tempDir, mockExecutor) + ruby := NewRuby() + ruby.executor = mockExecutor err := ruby.SanityCheck() if err == nil { t.Fatal("SanityCheck() expected error for outdated datadog-ci version") @@ -143,14 +101,12 @@ func TestRuby_SanityCheck_FailsWhenVersionTooLow(t *testing.T) { } func TestRuby_SanityCheck_FailsWhenVersionNotFound(t *testing.T) { - tempDir := t.TempDir() - writeTestFile(t, tempDir, "Gemfile", "source \"https://rubygems.org\"\n\ngem \"datadog-ci\"\n") - mockExecutor := &mockCommandExecutor{ combinedOutput: []byte(" * datadog-ci\n Summary: Datadog Test Optimization for your ruby application\n"), } - ruby := newRubyForDir(tempDir, mockExecutor) + ruby := NewRuby() + ruby.executor = mockExecutor err := ruby.SanityCheck() if err == nil { t.Fatal("SanityCheck() expected error when version is not found") @@ -420,24 +376,15 @@ func TestDetectPlatform_Ruby(t *testing.T) { viper.Set("platform", "ruby") platform, err := DetectPlatform() - if err != nil { - t.Fatalf("DetectPlatform failed: %v", err) - } - - if platform == nil { - t.Error("expected platform to be non-nil") - } - - if platform.Name() != "ruby" { - t.Errorf("expected platform name to be 'ruby', got %q", platform.Name()) + if err == nil { + t.Errorf("expected error for SanityCheck failure, but got platform: %v", platform) + } else if platform != nil { + t.Errorf("expected nil platform for SanityCheck failure, but got platform: %v", platform) } - // Verify it's the correct type and has executor - rubyPlatform, ok := platform.(*Ruby) - if !ok { - t.Error("expected platform to be *Ruby") - } else if rubyPlatform.executor == nil { - t.Error("expected Ruby platform to have executor") + expectedError := "sanity check failed for platform ruby: bundle info datadog-ci command failed: Could not locate Gemfile" + if err.Error() != expectedError { + t.Errorf("expected error %q, got %q", expectedError, err.Error()) } } From 8496ebea3b435a3e9844442067021b7b7f2d7f81 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 12 Nov 2025 09:55:16 +0100 Subject: [PATCH 3/6] ruby - move gem name and version to constants --- internal/platform/ruby.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/platform/ruby.go b/internal/platform/ruby.go index fa42648..e92ded3 100644 --- a/internal/platform/ruby.go +++ b/internal/platform/ruby.go @@ -19,6 +19,11 @@ import ( //go:embed scripts/ruby_env.rb var rubyEnvScript string +const ( + requiredGemName = "datadog-ci" + requiredGemMinVersion = "1.23.0" +) + type Ruby struct { executor ext.CommandExecutor } @@ -84,9 +89,6 @@ func (r *Ruby) DetectFramework() (framework.Framework, error) { } func (r *Ruby) SanityCheck() error { - requiredGemName := "datadog-ci" - minimumGemVersion := "1.23.0" - args := []string{"info", requiredGemName} output, err := r.executor.CombinedOutput(context.Background(), "bundle", args, nil) if err != nil { @@ -97,7 +99,7 @@ func (r *Ruby) SanityCheck() error { return fmt.Errorf("bundle info datadog-ci command failed: %s", message) } - requiredVersion, err := version.Parse(minimumGemVersion) + requiredVersion, err := version.Parse(requiredGemMinVersion) if err != nil { return err } From 27c997bada0ef25118e16b98b573c125587c0a2e Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 12 Nov 2025 09:59:53 +0100 Subject: [PATCH 4/6] add info on minimal supported library version to README --- README.md | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 282e940..f939b25 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,10 @@ Currently supported languages and frameworks: - Ruby (RSpec, Minitest) +DDTest requires that your project is correctly set up for Datadog Test Optimization with the native library for your language. Minimum supported library versions: + +- Ruby: `datadog-ci` gem **1.23.0** or higher + ## Installation ### From Source @@ -112,15 +116,15 @@ In CI‑node mode, DDTest also fans out across local CPUs on that node and furth ### Settings (flags and environment variables) -| CLI flag | Environment variable | Default | What it does | -| ------------------- | --------------------------------------------- | ---------: | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `--platform` | `DD_TEST_OPTIMIZATION_RUNNER_PLATFORM` | `ruby` | Language/platform (currently supported values: `ruby`). | -| `--framework` | `DD_TEST_OPTIMIZATION_RUNNER_FRAMEWORK` | `rspec` | Test framework (currently supported values: `rspec`, `minitest`). | -| `--min-parallelism` | `DD_TEST_OPTIMIZATION_RUNNER_MIN_PARALLELISM` | vCPU count | Minimum workers to use for the split. | -| `--max-parallelism` | `DD_TEST_OPTIMIZATION_RUNNER_MAX_PARALLELISM` | vCPU count | Maximum workers to use for the split. | -| `--ci-node` | `DD_TEST_OPTIMIZATION_RUNNER_CI_NODE` | `-1` (off) | Restrict this run to the slice assigned to node **N** (0‑indexed). Also parallelizes within the node across its CPUs. | -| `--worker-env` | `DD_TEST_OPTIMIZATION_RUNNER_WORKER_ENV` | `""` | Template env vars per local worker (e.g., isolate DBs): `--worker-env "DATABASE_NAME_TEST=app_test{{nodeIndex}}"`. | -| `--command` | `DD_TEST_OPTIMIZATION_RUNNER_COMMAND` | `""` | Override the default test command used by the framework. When provided, takes precedence over auto-detection (e.g., `--command "bundle exec custom-rspec"`). | +| CLI flag | Environment variable | Default | What it does | +| ------------------- | --------------------------------------------- | ---------: | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `--platform` | `DD_TEST_OPTIMIZATION_RUNNER_PLATFORM` | `ruby` | Language/platform (currently supported values: `ruby`). | +| `--framework` | `DD_TEST_OPTIMIZATION_RUNNER_FRAMEWORK` | `rspec` | Test framework (currently supported values: `rspec`, `minitest`). | +| `--min-parallelism` | `DD_TEST_OPTIMIZATION_RUNNER_MIN_PARALLELISM` | vCPU count | Minimum workers to use for the split. | +| `--max-parallelism` | `DD_TEST_OPTIMIZATION_RUNNER_MAX_PARALLELISM` | vCPU count | Maximum workers to use for the split. | +| `--ci-node` | `DD_TEST_OPTIMIZATION_RUNNER_CI_NODE` | `-1` (off) | Restrict this run to the slice assigned to node **N** (0‑indexed). Also parallelizes within the node across its CPUs. | +| `--worker-env` | `DD_TEST_OPTIMIZATION_RUNNER_WORKER_ENV` | `""` | Template env vars per local worker (e.g., isolate DBs): `--worker-env "DATABASE_NAME_TEST=app_test{{nodeIndex}}"`. | +| `--command` | `DD_TEST_OPTIMIZATION_RUNNER_COMMAND` | `""` | Override the default test command used by the framework. When provided, takes precedence over auto-detection (e.g., `--command "bundle exec custom-rspec"`). | | `--tests-location` | `DD_TEST_OPTIMIZATION_RUNNER_TESTS_LOCATION` | `""` | Custom glob pattern to discover test files (e.g., `--tests-location "custom/spec/**/*_spec.rb"`). Defaults to `spec/**/*_spec.rb` for RSpec, `test/**/*_test.rb` for Minitest. | #### Note about the `--command` flag From 6e7abfdc716dcc360dc015fb89a069cf5b42c1fc Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 12 Nov 2025 10:54:37 +0100 Subject: [PATCH 5/6] make version parsing more robust --- internal/version/version.go | 104 ++++++++++++-- internal/version/version_test.go | 232 ++++++++++++++++++++++++++----- 2 files changed, 294 insertions(+), 42 deletions(-) diff --git a/internal/version/version.go b/internal/version/version.go index 5225a77..1a812b4 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -2,35 +2,78 @@ package version import ( "fmt" + "regexp" "strconv" "strings" ) +// validIdentifierRegex matches valid pre-release and build metadata identifiers. +// They must contain only alphanumerics, hyphens, and dots. +var validIdentifierRegex = regexp.MustCompile(`^[a-zA-Z0-9.\-]+$`) + +// numericComponentRegex matches numeric version components (digits only). +var numericComponentRegex = regexp.MustCompile(`^[0-9]+$`) + // Version represents a semantic-like version composed of dot-separated integer components. +// It also supports pre-release identifiers (after '-') and build metadata (after '+'). type Version struct { raw string components []int + preRelease string + buildMeta string } -// Parse converts a raw string into a Version, ensuring it only contains numeric dot-separated components. +// Parse converts a raw string into a Version, supporting numeric dot-separated components, +// pre-release identifiers (after '-'), and build metadata (after '+'). +// Examples: "1.2.3", "1.0.0-alpha", "1.0.0-beta.2", "1.0.0+build", "1.0.0-rc.1+build.123" func Parse(raw string) (Version, error) { trimmed := strings.TrimSpace(raw) if trimmed == "" { return Version{}, fmt.Errorf("version string is empty") } - parts := strings.Split(trimmed, ".") - components := make([]int, len(parts)) + // Split by '+' to separate build metadata + var buildMeta string + mainPart := trimmed + if idx := strings.Index(trimmed, "+"); idx >= 0 { + mainPart = trimmed[:idx] + buildMeta = trimmed[idx+1:] + if buildMeta == "" { + return Version{}, fmt.Errorf("build metadata cannot be empty after '+'") + } + if !validIdentifierRegex.MatchString(buildMeta) { + return Version{}, fmt.Errorf("invalid build metadata %q", buildMeta) + } + } + // Split by '-' to separate pre-release identifier + var preRelease string + versionPart := mainPart + if idx := strings.Index(mainPart, "-"); idx >= 0 { + versionPart = mainPart[:idx] + preRelease = mainPart[idx+1:] + if preRelease == "" { + return Version{}, fmt.Errorf("pre-release identifier cannot be empty after '-'") + } + if !validIdentifierRegex.MatchString(preRelease) { + return Version{}, fmt.Errorf("invalid pre-release identifier %q", preRelease) + } + } + + // Parse the numeric version components + parts := strings.Split(versionPart, ".") + if len(parts) == 0 { + return Version{}, fmt.Errorf("version string must have at least one numeric component") + } + + components := make([]int, len(parts)) for i, part := range parts { if part == "" { return Version{}, fmt.Errorf("invalid version component %q", part) } - for _, r := range part { - if r < '0' || r > '9' { - return Version{}, fmt.Errorf("invalid version component %q", part) - } + if !numericComponentRegex.MatchString(part) { + return Version{}, fmt.Errorf("invalid version component %q", part) } value, err := strconv.Atoi(part) @@ -41,8 +84,10 @@ func Parse(raw string) (Version, error) { } return Version{ - raw: strings.Join(parts, "."), + raw: trimmed, components: components, + preRelease: preRelease, + buildMeta: buildMeta, }, nil } @@ -62,7 +107,10 @@ func IsValid(raw string) bool { } // Compare returns 1 if the receiver is greater than other, -1 if less, and 0 if equal. +// Build metadata is ignored in comparisons. Pre-release versions have lower precedence +// than normal versions (e.g., 1.0.0-alpha < 1.0.0). func (v Version) Compare(other Version) int { + // First compare numeric components maxLen := max(len(v.components), len(other.components)) for i := 0; i < maxLen; i++ { @@ -82,6 +130,24 @@ func (v Version) Compare(other Version) int { } } + // If numeric components are equal, compare pre-release identifiers + // No pre-release (normal version) > has pre-release (pre-release version) + if v.preRelease == "" && other.preRelease != "" { + return 1 + } + if v.preRelease != "" && other.preRelease == "" { + return -1 + } + if v.preRelease != "" && other.preRelease != "" { + if v.preRelease > other.preRelease { + return 1 + } + if v.preRelease < other.preRelease { + return -1 + } + } + + // Build metadata is ignored in comparisons return 0 } @@ -100,7 +166,15 @@ func (v Version) String() string { strParts[i] = strconv.Itoa(component) } - return strings.Join(strParts, ".") + result := strings.Join(strParts, ".") + if v.preRelease != "" { + result += "-" + v.preRelease + } + if v.buildMeta != "" { + result += "+" + v.buildMeta + } + + return result } // Components returns a copy of the underlying version components. @@ -113,6 +187,18 @@ func (v Version) Components() []int { return result } +// PreRelease returns the pre-release identifier (the part after '-' and before '+'). +// Returns empty string if there is no pre-release identifier. +func (v Version) PreRelease() string { + return v.preRelease +} + +// BuildMeta returns the build metadata (the part after '+'). +// Returns empty string if there is no build metadata. +func (v Version) BuildMeta() string { + return v.buildMeta +} + // CompareStrings parses the provided strings as versions and compares them. func CompareStrings(a, b string) (int, error) { vA, err := Parse(a) diff --git a/internal/version/version_test.go b/internal/version/version_test.go index c3d7047..512d8ac 100644 --- a/internal/version/version_test.go +++ b/internal/version/version_test.go @@ -3,26 +3,100 @@ package version import "testing" func TestParseValid(t *testing.T) { - input := "1.23.4" - v, err := Parse(input) - if err != nil { - t.Fatalf("Parse(%q) returned error: %v", input, err) - } + tests := []struct { + input string + expectedComponents []int + expectedPreRelease string + expectedBuildMeta string + expectedString string + }{ + // Standard semantic versioning + {"1.23.4", []int{1, 23, 4}, "", "", "1.23.4"}, + {"1.0.0", []int{1, 0, 0}, "", "", "1.0.0"}, + {"0.9.8", []int{0, 9, 8}, "", "", "0.9.8"}, + {"2.1.3", []int{2, 1, 3}, "", "", "2.1.3"}, - if v.String() != input { - t.Fatalf("expected String() to return %q, got %q", input, v.String()) - } + // Pre-release versions + {"1.0.0-alpha", []int{1, 0, 0}, "alpha", "", "1.0.0-alpha"}, + {"1.0.0-beta", []int{1, 0, 0}, "beta", "", "1.0.0-beta"}, + {"1.0.0-beta.2", []int{1, 0, 0}, "beta.2", "", "1.0.0-beta.2"}, + {"2.1.3-rc.1", []int{2, 1, 3}, "rc.1", "", "2.1.3-rc.1"}, + {"3.0.0-alpha.fah2345", []int{3, 0, 0}, "alpha.fah2345", "", "3.0.0-alpha.fah2345"}, + {"3.0-beta1", []int{3, 0}, "beta1", "", "3.0-beta1"}, + {"1.0.0-alpha.1.11.28", []int{1, 0, 0}, "alpha.1.11.28", "", "1.0.0-alpha.1.11.28"}, + + // Build metadata + {"1.0.0+20130313144700", []int{1, 0, 0}, "", "20130313144700", "1.0.0+20130313144700"}, + {"2.1.3+exp.sha.5114f85", []int{2, 1, 3}, "", "exp.sha.5114f85", "2.1.3+exp.sha.5114f85"}, + {"1.0.0+build.123", []int{1, 0, 0}, "", "build.123", "1.0.0+build.123"}, + + // Pre-release with build metadata + {"1.0.0-alpha+001", []int{1, 0, 0}, "alpha", "001", "1.0.0-alpha+001"}, + {"1.0.0-beta.2+exp.sha.5114f85", []int{1, 0, 0}, "beta.2", "exp.sha.5114f85", "1.0.0-beta.2+exp.sha.5114f85"}, + {"3.0.0-alpha.fah2345+build.123", []int{3, 0, 0}, "alpha.fah2345", "build.123", "3.0.0-alpha.fah2345+build.123"}, + + // Date-based versions + {"2025.11.12", []int{2025, 11, 12}, "", "", "2025.11.12"}, + {"2025.11.12.1", []int{2025, 11, 12, 1}, "", "", "2025.11.12.1"}, + {"2025.11.12-alpha", []int{2025, 11, 12}, "alpha", "", "2025.11.12-alpha"}, + + // Short versions + {"1.0", []int{1, 0}, "", "", "1.0"}, + {"2", []int{2}, "", "", "2"}, + {"10.5-rc1", []int{10, 5}, "rc1", "", "10.5-rc1"}, - expectedComponents := []int{1, 23, 4} - actual := v.Components() - if len(actual) != len(expectedComponents) { - t.Fatalf("expected %d components, got %d", len(expectedComponents), len(actual)) + // Complex examples + {"1.0.0-x.7.z.92", []int{1, 0, 0}, "x.7.z.92", "", "1.0.0-x.7.z.92"}, + {"1.0.0-alpha.beta", []int{1, 0, 0}, "alpha.beta", "", "1.0.0-alpha.beta"}, + {"1.0.0-rc.1.2.3", []int{1, 0, 0}, "rc.1.2.3", "", "1.0.0-rc.1.2.3"}, + {"1.2.3-DEV-SNAPSHOT", []int{1, 2, 3}, "DEV-SNAPSHOT", "", "1.2.3-DEV-SNAPSHOT"}, + {"1.2.3-SNAPSHOT-123", []int{1, 2, 3}, "SNAPSHOT-123", "", "1.2.3-SNAPSHOT-123"}, + + // Ruby/Rails style versions + {"2.7.6", []int{2, 7, 6}, "", "", "2.7.6"}, + {"3.1.0", []int{3, 1, 0}, "", "", "3.1.0"}, + {"7.0.4.3", []int{7, 0, 4, 3}, "", "", "7.0.4.3"}, + + // Python style versions + {"3.10.0", []int{3, 10, 0}, "", "", "3.10.0"}, + {"3.11.5", []int{3, 11, 5}, "", "", "3.11.5"}, + {"2.7.18", []int{2, 7, 18}, "", "", "2.7.18"}, + + // Node.js style versions + {"18.12.1", []int{18, 12, 1}, "", "", "18.12.1"}, + {"20.0.0", []int{20, 0, 0}, "", "", "20.0.0"}, } - for i, expected := range expectedComponents { - if actual[i] != expected { - t.Fatalf("expected component %d to be %d, got %d", i, expected, actual[i]) - } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + v, err := Parse(tt.input) + if err != nil { + t.Fatalf("Parse(%q) returned error: %v", tt.input, err) + } + + if v.String() != tt.expectedString { + t.Fatalf("expected String() to return %q, got %q", tt.expectedString, v.String()) + } + + actual := v.Components() + if len(actual) != len(tt.expectedComponents) { + t.Fatalf("expected %d components, got %d", len(tt.expectedComponents), len(actual)) + } + + for i, expected := range tt.expectedComponents { + if actual[i] != expected { + t.Fatalf("expected component %d to be %d, got %d", i, expected, actual[i]) + } + } + + if v.PreRelease() != tt.expectedPreRelease { + t.Fatalf("expected PreRelease() to return %q, got %q", tt.expectedPreRelease, v.PreRelease()) + } + + if v.BuildMeta() != tt.expectedBuildMeta { + t.Fatalf("expected BuildMeta() to return %q, got %q", tt.expectedBuildMeta, v.BuildMeta()) + } + }) } } @@ -33,12 +107,24 @@ func TestParseInvalid(t *testing.T) { "1..2", "1.2.a", "1.-2", + "1.0.0-", // Empty pre-release + "1.0.0+", // Empty build metadata + "1.0.0+build+", // Trailing plus in build metadata + "1.0.0-alpha@beta", // Invalid character in pre-release + "1.0.0+build#123", // Invalid character in build metadata + "1.0.0-beta$1", // Invalid character in pre-release + ".1.2", // Leading dot + "1.2.", // Trailing dot + "a.b.c", // Non-numeric components + "-1.0.0", // Negative version } for _, input := range cases { - if _, err := Parse(input); err == nil { - t.Fatalf("expected Parse(%q) to fail", input) - } + t.Run(input, func(t *testing.T) { + if _, err := Parse(input); err == nil { + t.Fatalf("expected Parse(%q) to fail", input) + } + }) } } @@ -53,25 +139,105 @@ func TestIsValid(t *testing.T) { } func TestCompare(t *testing.T) { - v1 := MustParse("1.2.3") - v2 := MustParse("1.2.3") - v3 := MustParse("1.3.0") - v4 := MustParse("1.2") + tests := []struct { + name string + v1 string + v2 string + expected int // -1 if v1 < v2, 0 if v1 == v2, 1 if v1 > v2 + }{ + // Equal versions + {"equal standard versions", "1.2.3", "1.2.3", 0}, + {"equal short versions", "1.0", "1.0", 0}, + {"equal single component", "2", "2", 0}, + {"equal with pre-release", "1.0.0-alpha", "1.0.0-alpha", 0}, + {"equal with build metadata", "1.0.0+build1", "1.0.0+build2", 0}, // Build metadata ignored + {"equal pre-release and build", "1.0.0-beta+build1", "1.0.0-beta+build2", 0}, - if v1.Compare(v2) != 0 { - t.Fatal("expected equal versions to compare as 0") - } + // Major version differences + {"major version less", "1.0.0", "2.0.0", -1}, + {"major version greater", "2.0.0", "1.0.0", 1}, + {"major version much greater", "10.0.0", "2.0.0", 1}, - if v1.Compare(v3) >= 0 { - t.Fatal("expected v1 < v3") - } + // Minor version differences + {"minor version less", "1.2.0", "1.3.0", -1}, + {"minor version greater", "1.3.0", "1.2.0", 1}, + {"minor version much greater", "1.10.0", "1.2.0", 1}, + + // Patch version differences + {"patch version less", "1.2.3", "1.2.4", -1}, + {"patch version greater", "1.2.4", "1.2.3", 1}, + {"patch version much greater", "1.2.10", "1.2.3", 1}, + + // Short vs long versions + {"short less than long", "1.2", "1.2.3", -1}, + {"long greater than short", "1.2.3", "1.2", 1}, + {"single equal to double with zero", "1", "1.0", 0}, // Missing components treated as 0 + {"double equal to single with zero", "1.0", "1", 0}, // Missing components treated as 0 + {"triple equal to double with zero", "1.0.0", "1.0", 0}, // Missing components treated as 0 + + // Pre-release versions + {"release greater than pre-release", "1.0.0", "1.0.0-alpha", 1}, + {"pre-release less than release", "1.0.0-alpha", "1.0.0", -1}, + {"pre-release alpha less than beta", "1.0.0-alpha", "1.0.0-beta", -1}, + {"pre-release beta greater than alpha", "1.0.0-beta", "1.0.0-alpha", 1}, + {"pre-release rc greater than beta", "1.0.0-rc", "1.0.0-beta", 1}, + {"pre-release with version less", "1.0.0-alpha.1", "1.0.0-alpha.2", -1}, + {"pre-release with version greater", "1.0.0-beta.2", "1.0.0-beta.1", 1}, + {"pre-release complex less", "1.0.0-alpha.beta", "1.0.0-alpha.gamma", -1}, + + // Build metadata (should be ignored) + {"build metadata ignored equal", "1.0.0+build1", "1.0.0+build999", 0}, + {"build metadata ignored with pre-release", "1.0.0-alpha+b1", "1.0.0-alpha+b2", 0}, - if v3.Compare(v1) <= 0 { - t.Fatal("expected v3 > v1") + // Date-based versions + {"date versions less", "2025.11.12", "2025.11.13", -1}, + {"date versions greater", "2025.12.01", "2025.11.30", 1}, + {"date versions equal", "2025.11.12", "2025.11.12", 0}, + + // Mixed length comparisons + {"four component less", "7.0.4.3", "7.0.5.1", -1}, + {"four component greater", "7.0.5.0", "7.0.4.9", 1}, + {"three vs four components", "7.0.4", "7.0.4.1", -1}, + + // Edge cases + {"zero major less", "0.9.0", "1.0.0", -1}, + {"zero minor less", "1.0.0", "1.1.0", -1}, + {"large version numbers", "100.200.300", "100.200.301", -1}, + {"very different lengths", "1", "2.0.0.0", -1}, + + // Real-world version comparisons + {"ruby 2.7.6 vs 3.0.0", "2.7.6", "3.0.0", -1}, + {"python 3.10.0 vs 3.11.0", "3.10.0", "3.11.0", -1}, + {"node 18.12.1 vs 20.0.0", "18.12.1", "20.0.0", -1}, + {"rails 7.0.4.3 vs 7.0.5", "7.0.4.3", "7.0.5", -1}, + + // Pre-release ordering + {"alpha before beta before rc", "1.0.0-alpha", "1.0.0-beta", -1}, + {"beta before rc", "1.0.0-beta", "1.0.0-rc", -1}, + {"dev snapshot versions", "1.2.3-SNAPSHOT", "1.2.3", -1}, } - if v1.Compare(v4) <= 0 { - t.Fatal("expected v1 > v4 when compared with shorter version") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v1 := MustParse(tt.v1) + v2 := MustParse(tt.v2) + + result := v1.Compare(v2) + + if result != tt.expected { + t.Fatalf("Compare(%q, %q) = %d, expected %d", tt.v1, tt.v2, result, tt.expected) + } + + // Verify symmetry: if v1 < v2, then v2 > v1 + if tt.expected != 0 { + reverseResult := v2.Compare(v1) + expectedReverse := -tt.expected + if reverseResult != expectedReverse { + t.Fatalf("Symmetry check failed: Compare(%q, %q) = %d, but Compare(%q, %q) = %d (expected %d)", + tt.v1, tt.v2, result, tt.v2, tt.v1, reverseResult, expectedReverse) + } + } + }) } } From 3eeb5ec1f63e92e0e706b9be73ea078ef79a5c9c Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 12 Nov 2025 11:47:35 +0100 Subject: [PATCH 6/6] relax assertion --- internal/platform/ruby_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/platform/ruby_test.go b/internal/platform/ruby_test.go index dad5061..163a039 100644 --- a/internal/platform/ruby_test.go +++ b/internal/platform/ruby_test.go @@ -382,9 +382,9 @@ func TestDetectPlatform_Ruby(t *testing.T) { t.Errorf("expected nil platform for SanityCheck failure, but got platform: %v", platform) } - expectedError := "sanity check failed for platform ruby: bundle info datadog-ci command failed: Could not locate Gemfile" - if err.Error() != expectedError { - t.Errorf("expected error %q, got %q", expectedError, err.Error()) + expectedErrorPrefix := "sanity check failed for platform ruby: bundle info datadog-ci command failed" + if !strings.Contains(err.Error(), expectedErrorPrefix) { + t.Errorf("expected error to contain %q, got %q", expectedErrorPrefix, err.Error()) } }