From e01ce6880d9d6b4e384a07c7be8205d8ae77c81b Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Fri, 30 Jan 2026 12:43:45 +0100 Subject: [PATCH 1/2] set min-parallelism and max-parallelism to the numer of logical CPUs by default --- internal/cmd/cmd.go | 7 +++-- internal/runner/runner_test.go | 27 ++++++++++++++++++++ internal/settings/settings.go | 11 ++++++-- internal/settings/settings_test.go | 41 +++++++++++++++++++++--------- 4 files changed, 70 insertions(+), 16 deletions(-) diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go index 0e2b1a2..689a3cc 100644 --- a/internal/cmd/cmd.go +++ b/internal/cmd/cmd.go @@ -13,6 +13,9 @@ import ( "github.com/spf13/viper" ) +// defaultParallelism stores the computed default at init time for CLI flags +var defaultParallelism = settings.DefaultParallelism() + var rootCmd = &cobra.Command{ Use: "ddtest", Short: "A test runner from Datadog", @@ -54,8 +57,8 @@ var runCmd = &cobra.Command{ func init() { rootCmd.PersistentFlags().String("platform", "ruby", "Platform that runs tests") rootCmd.PersistentFlags().String("framework", "rspec", "Test framework to use") - rootCmd.PersistentFlags().Int("min-parallelism", 1, "Minimum number of parallel test processes") - rootCmd.PersistentFlags().Int("max-parallelism", 1, "Maximum number of parallel test processes") + rootCmd.PersistentFlags().Int("min-parallelism", defaultParallelism, "Minimum number of parallel test processes (default: number of CPUs)") + rootCmd.PersistentFlags().Int("max-parallelism", defaultParallelism, "Maximum number of parallel test processes (default: number of CPUs)") rootCmd.PersistentFlags().String("worker-env", "", "Worker environment configuration") rootCmd.PersistentFlags().String("command", "", "Test command that ddtest should wrap") rootCmd.PersistentFlags().String("tests-location", "", "Glob pattern used to discover test files") diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index c68045c..44519c2 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -242,6 +242,15 @@ func TestTestRunner_Setup_WithParallelRunners(t *testing.T) { // Create .testoptimization directory _ = os.MkdirAll(constants.PlanDirectory, 0755) + // Set parallelism to 1 to test single runner behavior + _ = os.Setenv("DD_TEST_OPTIMIZATION_RUNNER_MIN_PARALLELISM", "1") + _ = os.Setenv("DD_TEST_OPTIMIZATION_RUNNER_MAX_PARALLELISM", "1") + defer func() { + _ = os.Unsetenv("DD_TEST_OPTIMIZATION_RUNNER_MIN_PARALLELISM") + _ = os.Unsetenv("DD_TEST_OPTIMIZATION_RUNNER_MAX_PARALLELISM") + }() + settings.Init() + // Setup mocks for a test with 40% skippable percentage mockFramework := &MockFramework{ FrameworkName: "rspec", @@ -299,6 +308,15 @@ func TestTestRunner_Setup_WithCIProvider(t *testing.T) { // Create .testoptimization directory _ = os.MkdirAll(constants.PlanDirectory, 0755) + // Set parallelism to 1 to test single runner behavior + _ = os.Setenv("DD_TEST_OPTIMIZATION_RUNNER_MIN_PARALLELISM", "1") + _ = os.Setenv("DD_TEST_OPTIMIZATION_RUNNER_MAX_PARALLELISM", "1") + defer func() { + _ = os.Unsetenv("DD_TEST_OPTIMIZATION_RUNNER_MIN_PARALLELISM") + _ = os.Unsetenv("DD_TEST_OPTIMIZATION_RUNNER_MAX_PARALLELISM") + }() + settings.Init() + // Setup mocks for test with CI provider mockFramework := &MockFramework{ FrameworkName: "rspec", @@ -455,6 +473,15 @@ func TestTestRunner_Setup_WithTestSplit(t *testing.T) { // Create .testoptimization directory _ = os.MkdirAll(constants.PlanDirectory, 0755) + // Set parallelism to 1 to test single runner behavior + _ = os.Setenv("DD_TEST_OPTIMIZATION_RUNNER_MIN_PARALLELISM", "1") + _ = os.Setenv("DD_TEST_OPTIMIZATION_RUNNER_MAX_PARALLELISM", "1") + defer func() { + _ = os.Unsetenv("DD_TEST_OPTIMIZATION_RUNNER_MIN_PARALLELISM") + _ = os.Unsetenv("DD_TEST_OPTIMIZATION_RUNNER_MAX_PARALLELISM") + }() + settings.Init() + // Setup mocks for single runner scenario mockFramework := &MockFramework{ FrameworkName: "rspec", diff --git a/internal/settings/settings.go b/internal/settings/settings.go index 0e07688..3e20ebf 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -4,11 +4,18 @@ import ( "encoding/json" "fmt" "os" + "runtime" "strings" "github.com/spf13/viper" ) +// DefaultParallelism returns the default parallelism value, which is the number +// of available CPUs. This respects container CPU limits in cloud CI environments. +func DefaultParallelism() int { + return runtime.NumCPU() +} + type Config struct { Platform string `mapstructure:"platform"` Framework string `mapstructure:"framework"` @@ -42,8 +49,8 @@ func Init() { func setDefaults() { viper.SetDefault("platform", "ruby") viper.SetDefault("framework", "rspec") - viper.SetDefault("min_parallelism", 1) - viper.SetDefault("max_parallelism", 1) + viper.SetDefault("min_parallelism", DefaultParallelism()) + viper.SetDefault("max_parallelism", DefaultParallelism()) viper.SetDefault("worker_env", "") viper.SetDefault("ci_node", -1) viper.SetDefault("command", "") diff --git a/internal/settings/settings_test.go b/internal/settings/settings_test.go index 78b4657..735c414 100644 --- a/internal/settings/settings_test.go +++ b/internal/settings/settings_test.go @@ -2,11 +2,24 @@ package settings import ( "os" + "runtime" "testing" "github.com/spf13/viper" ) +func TestDefaultParallelism(t *testing.T) { + result := DefaultParallelism() + expected := runtime.NumCPU() + + if result != expected { + t.Errorf("expected DefaultParallelism() to return %d (runtime.NumCPU()), got %d", expected, result) + } + if result < 1 { + t.Errorf("expected DefaultParallelism() to be at least 1, got %d", result) + } +} + func TestInit(t *testing.T) { // Clear any existing config config = nil @@ -25,11 +38,12 @@ func TestInit(t *testing.T) { if config.Framework != "rspec" { t.Errorf("expected default framework to be 'rspec', got %q", config.Framework) } - if config.MinParallelism != 1 { - t.Errorf("expected default min_parallelism to be 1, got %d", config.MinParallelism) + expectedParallelism := runtime.NumCPU() + if config.MinParallelism != expectedParallelism { + t.Errorf("expected default min_parallelism to be %d (CPU count), got %d", expectedParallelism, config.MinParallelism) } - if config.MaxParallelism != 1 { - t.Errorf("expected default max_parallelism to be 1, got %d", config.MaxParallelism) + if config.MaxParallelism != expectedParallelism { + t.Errorf("expected default max_parallelism to be %d (CPU count), got %d", expectedParallelism, config.MaxParallelism) } if config.WorkerEnv != "" { t.Errorf("expected default worker_env to be empty, got %q", config.WorkerEnv) @@ -59,11 +73,12 @@ func TestSetDefaults(t *testing.T) { if viper.GetString("framework") != "rspec" { t.Errorf("expected default framework to be 'rspec', got %q", viper.GetString("framework")) } - if viper.GetInt("min_parallelism") != 1 { - t.Errorf("expected default min_parallelism to be 1, got %d", viper.GetInt("min_parallelism")) + expectedParallelism := runtime.NumCPU() + if viper.GetInt("min_parallelism") != expectedParallelism { + t.Errorf("expected default min_parallelism to be %d (CPU count), got %d", expectedParallelism, viper.GetInt("min_parallelism")) } - if viper.GetInt("max_parallelism") != 1 { - t.Errorf("expected default max_parallelism to be 1, got %d", viper.GetInt("max_parallelism")) + if viper.GetInt("max_parallelism") != expectedParallelism { + t.Errorf("expected default max_parallelism to be %d (CPU count), got %d", expectedParallelism, viper.GetInt("max_parallelism")) } if viper.GetString("worker_env") != "" { t.Errorf("expected default worker_env to be empty, got %q", viper.GetString("worker_env")) @@ -199,9 +214,10 @@ func TestGetMinParallelism(t *testing.T) { config = nil viper.Reset() + expectedParallelism := runtime.NumCPU() minParallelism := GetMinParallelism() - if minParallelism != 1 { - t.Errorf("expected min_parallelism to be 1, got %d", minParallelism) + if minParallelism != expectedParallelism { + t.Errorf("expected min_parallelism to be %d (CPU count), got %d", expectedParallelism, minParallelism) } // Test with custom value @@ -217,9 +233,10 @@ func TestGetMaxParallelism(t *testing.T) { config = nil viper.Reset() + expectedParallelism := runtime.NumCPU() maxParallelism := GetMaxParallelism() - if maxParallelism != 1 { - t.Errorf("expected max_parallelism to be 1, got %d", maxParallelism) + if maxParallelism != expectedParallelism { + t.Errorf("expected max_parallelism to be %d (CPU count), got %d", expectedParallelism, maxParallelism) } // Test with custom value From 448719034c889b36eb40467709e51eabcf4e2d42 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Fri, 30 Jan 2026 12:54:10 +0100 Subject: [PATCH 2/2] fix: clamp min_parallelism to max when max < min When a user only sets --max-parallelism to a lower value (e.g., 1), they should get that value, not the default min (which is now NumCPU). Previously, if max < min, the code returned min, effectively ignoring the user's cap. Now we clamp min to max instead, ensuring the user's explicit max setting takes precedence. --- internal/runner/parallelism.go | 4 ++-- internal/runner/parallelism_test.go | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/runner/parallelism.go b/internal/runner/parallelism.go index 0102cc8..e364d3a 100644 --- a/internal/runner/parallelism.go +++ b/internal/runner/parallelism.go @@ -24,9 +24,9 @@ func calculateParallelRunnersWithParams(skippablePercentage float64, minParallel } if maxParallelism < minParallelism { - slog.Warn("max_parallelism is less than min_parallelism, using min_parallelism", + slog.Warn("max_parallelism is less than min_parallelism, clamping min to max", "max_parallelism", maxParallelism, "min_parallelism", minParallelism) - return minParallelism + minParallelism = maxParallelism } percentage := math.Max(0.0, math.Min(100.0, skippablePercentage)) // Clamp to [0, 100] diff --git a/internal/runner/parallelism_test.go b/internal/runner/parallelism_test.go index 22860d6..14c0747 100644 --- a/internal/runner/parallelism_test.go +++ b/internal/runner/parallelism_test.go @@ -50,8 +50,10 @@ func TestCalculateParallelRunners_MinParallelismLessThanOne(t *testing.T) { } func TestCalculateParallelRunners_MaxLessThanMin(t *testing.T) { + // When max < min, min is clamped to max. This ensures that a user who only + // sets --max-parallelism to a lower value gets the expected behavior. result := testCalculateParallelRunners(50.0, 5, 3) // max < min - expected := 5 // Should return min_parallelism + expected := 3 // Should clamp min to max and return max if result != expected { t.Errorf("calculateParallelRunners(50.0) = %d, expected %d when max < min", result, expected) }