diff --git a/README.md b/README.md index c452f28..181cb88 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,7 @@ parallelism details, see [Running DDTest](docs/running.md). | `--command` | Override the default test command used by Ruby frameworks. For pytest, use `PYTEST_ADDOPTS` for pytest flags. | | `--min-parallelism` | Minimum CI node or worker count DDTest considers when planning. | | `--max-parallelism` | Maximum CI node or worker count DDTest considers when planning. | +| `--target-time` | Target wall time DDTest tries to satisfy when selecting parallelism. | | `--ci-node` | Run only the files assigned to CI node **N**. | | `--tests-location` | Override the default test file discovery glob. | | `--tests-exclude-pattern` | Exclude matching test files from discovery. | diff --git a/docs/running.md b/docs/running.md index ca64ea0..a506a76 100644 --- a/docs/running.md +++ b/docs/running.md @@ -133,3 +133,10 @@ decrease it to prefer faster wall time. Use duration values such as `25s`, `1m`, or `1500ms`; set `0s` to disable this overhead bias. When scores tie, DDTest prefers fewer CI nodes or workers, then lower wall time, then lower imbalance between workers. + +Set `--target-time` to make DDTest first choose among splits whose expected +wall time is at or below that target. Use the same duration format, such as +`10m`, `300s`, or `1500ms`; the default `0s` disables the target. If no split +within `--min-parallelism` and `--max-parallelism` can meet the target, DDTest +logs a warning and selects the split with the lowest expected wall time, +ignoring CI job overhead, to get as close as possible to the target. diff --git a/docs/settings.md b/docs/settings.md index 96047d6..7a26fe1 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -11,6 +11,7 @@ CLI flags take precedence over environment variables. | `--min-parallelism` | `DD_TEST_OPTIMIZATION_RUNNER_MIN_PARALLELISM` | | physical CPU count | Minimum count DDTest considers when planning. Interpret it as CI nodes in CI-node mode, or workers in a single-node run. | | `--max-parallelism` | `DD_TEST_OPTIMIZATION_RUNNER_MAX_PARALLELISM` | | physical CPU count | Maximum count DDTest considers when planning. Interpret it as CI nodes in CI-node mode, or workers in a single-node run. | | `--ci-job-overhead` | `DD_TEST_OPTIMIZATION_RUNNER_CI_JOB_OVERHEAD` | | `25s` | Modeled overhead for adding one more CI node. Accepts durations such as `25s`, `1m`, `1500ms`, or `0s` to disable this bias. Increase it to use fewer CI nodes; decrease it to prefer faster wall time. | +| `--target-time` | `DD_TEST_OPTIMIZATION_RUNNER_TARGET_TIME` | | `0s` | Target wall time for the selected split. Accepts durations such as `10m`, `300s`, `1500ms`, or `0s` to disable the target. DDTest first considers splits at or below this wall time; if none are possible within the min/max parallelism range, it warns and selects the split with the lowest expected wall time, ignoring CI job overhead, to get as close as possible to the target. | | `--ci-node` | `DD_TEST_OPTIMIZATION_RUNNER_CI_NODE` | | `-1` (off) | Restrict this run to files assigned to CI node **N** (0-indexed). | | `--ci-node-workers` | `DD_TEST_OPTIMIZATION_RUNNER_CI_NODE_WORKERS` | | `1` | Number of workers to start on this CI node. Use a positive integer, or `ncpu` to use the node's available physical CPU cores. | | `--worker-env` | `DD_TEST_OPTIMIZATION_RUNNER_WORKER_ENV` | | `""` | Template env vars per worker: `--worker-env "DATABASE_NAME_TEST=app_test{{nodeIndex}}_{{workerIndex}}"`. `{{nodeIndex}}` is the CI node index (`0` for single-node runs); `{{workerIndex}}` is the worker process index within that CI node. | diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go index ed9d365..085cdb7 100644 --- a/internal/cmd/cmd.go +++ b/internal/cmd/cmd.go @@ -64,6 +64,7 @@ var rootPersistentFlagBindings = []persistentFlagBinding{ {configKey: "min_parallelism", flagName: "min-parallelism"}, {configKey: "max_parallelism", flagName: "max-parallelism"}, {configKey: "parallel_runner_overhead", flagName: "ci-job-overhead"}, + {configKey: "target_time", flagName: "target-time"}, {configKey: "worker_env", flagName: "worker-env"}, {configKey: "ci_node", flagName: "ci-node"}, {configKey: "ci_node_workers", flagName: "ci-node-workers"}, @@ -85,6 +86,7 @@ func init() { rootCmd.PersistentFlags().Int("min-parallelism", defaultParallelism, "Minimum number of parallel test processes (default: number of physical CPUs)") rootCmd.PersistentFlags().Int("max-parallelism", defaultParallelism, "Maximum number of parallel test processes (default: number of physical CPUs)") rootCmd.PersistentFlags().String("ci-job-overhead", settings.DefaultParallelRunnerOverhead().String(), "Modeled overhead for adding one more CI job / parallel runner (for example, 25s, 1m, 1500ms, or 0s to disable the bias). Increase it to use fewer CI jobs; decrease it to prefer faster wall time") + rootCmd.PersistentFlags().String("target-time", settings.DefaultTargetTime().String(), "Target wall time for selected CI job / parallel runner split (for example, 10m, 300s, 1500ms, or 0s to disable the target)") rootCmd.PersistentFlags().String("worker-env", "", "Worker environment configuration") rootCmd.PersistentFlags().Int("ci-node", -1, "CI node index to run (0-indexed; default: -1 disables CI-node mode)") rootCmd.PersistentFlags().String("ci-node-workers", "1", `Number of parallel workers per CI node (positive integer or "ncpu"; default: 1)`) diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index eea5226..1612c36 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -92,6 +92,12 @@ func TestRootCommandFlags(t *testing.T) { return } + targetTimeFlag := rootCmd.PersistentFlags().Lookup("target-time") + if targetTimeFlag == nil { + t.Error("target-time flag should be defined") + return + } + // Check default values if platformFlag.DefValue != "ruby" { t.Errorf("expected platform default to be 'ruby', got %q", platformFlag.DefValue) @@ -141,6 +147,11 @@ func TestRootCommandFlags(t *testing.T) { if parallelRunnerOverheadFlag.DefValue != expectedParallelRunnerOverhead { t.Errorf("expected ci-job-overhead default to be %q, got %q", expectedParallelRunnerOverhead, parallelRunnerOverheadFlag.DefValue) } + + expectedTargetTime := settings.DefaultTargetTime().String() + if targetTimeFlag.DefValue != expectedTargetTime { + t.Errorf("expected target-time default to be %q, got %q", expectedTargetTime, targetTimeFlag.DefValue) + } } func TestCommandHierarchy(t *testing.T) { @@ -380,6 +391,9 @@ func TestFlagBinding(t *testing.T) { if err := rootCmd.PersistentFlags().Set("ci-job-overhead", "30s"); err != nil { t.Fatalf("Error setting ci-job-overhead flag: %v", err) } + if err := rootCmd.PersistentFlags().Set("target-time", "10m"); err != nil { + t.Fatalf("Error setting target-time flag: %v", err) + } // Check that viper picks up the flag values if viper.GetString("platform") != "python" { @@ -418,6 +432,9 @@ func TestFlagBinding(t *testing.T) { if viper.GetString("parallel_runner_overhead") != "30s" { t.Errorf("expected viper parallel_runner_overhead to be '30s', got %q", viper.GetString("parallel_runner_overhead")) } + if viper.GetString("target_time") != "10m" { + t.Errorf("expected viper target_time to be '10m', got %q", viper.GetString("target_time")) + } } func TestBindPersistentFlags(t *testing.T) { diff --git a/internal/planner/parallelism.go b/internal/planner/parallelism.go index 20e695c..c2d1cac 100644 --- a/internal/planner/parallelism.go +++ b/internal/planner/parallelism.go @@ -7,15 +7,23 @@ import ( // calculateParallelRunnerSplit determines the selected runner split by // estimating candidates between the configured min and max parallelism. -func calculateParallelRunnerSplit(testFileWeights map[string]int, minParallelism, maxParallelism int, parallelRunnerOverhead time.Duration) splitScore { +func calculateParallelRunnerSplit(testFileWeights map[string]int, minParallelism, maxParallelism int, parallelRunnerOverhead, targetTime time.Duration) splitScore { + return calculateParallelRunnerSplitSelection(testFileWeights, minParallelism, maxParallelism, parallelRunnerOverhead, targetTime).selected +} + +func calculateParallelRunnerSplitSelection(testFileWeights map[string]int, minParallelism, maxParallelism int, parallelRunnerOverhead, targetTime time.Duration) splitSelection { files := sortedWeightedTestFiles(testFileWeights) - selector := splitSelector{parallelRunnerOverhead: parallelRunnerOverhead} + selector := splitSelector{ + parallelRunnerOverhead: parallelRunnerOverhead, + targetTime: targetTime, + } // maxParallelism could be 0 or negative! if maxParallelism <= 1 { score := scoreSortedWeightedRunnerSplit(files, 1) selector.logCandidate(score) - return score + selector.maybeWarnTargetTimeUnreachable(score, 1, 1) + return selector.selection(score, score, []splitScore{score}) } if minParallelism < 1 { @@ -32,22 +40,43 @@ func calculateParallelRunnerSplit(testFileWeights map[string]int, minParallelism if len(files) == 0 { score := scoreSortedWeightedRunnerSplit(files, minParallelism) selector.logCandidate(score) - return score + selector.maybeWarnTargetTimeUnreachable(score, minParallelism, maxParallelism) + return selector.selection(score, score, []splitScore{score}) } candidateMax := maxUsefulParallelism(minParallelism, maxParallelism, len(files)) - best := scoreSortedWeightedRunnerSplit(files, minParallelism) - selector.logCandidate(best) + bestWithoutTarget := scoreSortedWeightedRunnerSplit(files, minParallelism) + lowestWallTime := bestWithoutTarget + bestWithinTarget := bestWithoutTarget + targetBestFound := selector.meetsTargetTime(bestWithoutTarget) + candidates := []splitScore{bestWithoutTarget} + selector.logCandidate(bestWithoutTarget) for parallelRunners := minParallelism + 1; parallelRunners <= candidateMax; parallelRunners++ { score := scoreSortedWeightedRunnerSplit(files, parallelRunners) + candidates = append(candidates, score) selector.logCandidate(score) - if selector.better(score, best) { - best = score + if selector.better(score, bestWithoutTarget) { + bestWithoutTarget = score + } + if selector.betterWallTime(score, lowestWallTime) { + lowestWallTime = score } + if selector.meetsTargetTime(score) && (!targetBestFound || selector.better(score, bestWithinTarget)) { + bestWithinTarget = score + targetBestFound = true + } + } + + if targetBestFound { + return selector.selection(bestWithinTarget, bestWithoutTarget, candidates) + } + if selector.targetTime <= 0 { + return selector.selection(bestWithoutTarget, bestWithoutTarget, candidates) } - return best + selector.maybeWarnTargetTimeUnreachable(lowestWallTime, minParallelism, maxParallelism) + return selector.selection(lowestWallTime, bestWithoutTarget, candidates) } func maxUsefulParallelism(minParallelism, maxParallelism, filesCount int) int { @@ -62,6 +91,16 @@ func maxUsefulParallelism(minParallelism, maxParallelism, filesCount int) int { type splitSelector struct { parallelRunnerOverhead time.Duration + targetTime time.Duration +} + +type splitSelection struct { + selected splitScore + bestWithoutTarget splitScore + candidates []splitScore + parallelRunnerOverhead time.Duration + targetTime time.Duration + available bool } func (s splitSelector) better(candidate, currentBest splitScore) bool { @@ -79,6 +118,16 @@ func (s splitSelector) better(candidate, currentBest splitScore) bool { return candidate.imbalance < currentBest.imbalance } +func (s splitSelector) betterWallTime(candidate, currentBest splitScore) bool { + if candidate.wallTime != currentBest.wallTime { + return candidate.wallTime < currentBest.wallTime + } + if candidate.parallelRunners != currentBest.parallelRunners { + return candidate.parallelRunners < currentBest.parallelRunners + } + return candidate.imbalance < currentBest.imbalance +} + // selectionScore models each candidate as wallTime + runners * overhead. When // scores tie, the selector intentionally prefers fewer runners before comparing // wall time and imbalance. @@ -93,6 +142,34 @@ func (s splitSelector) parallelRunnerOverheadMillis() int { return int(s.parallelRunnerOverhead / time.Millisecond) } +func (s splitSelector) selection(selected, bestWithoutTarget splitScore, candidates []splitScore) splitSelection { + return splitSelection{ + selected: selected, + bestWithoutTarget: bestWithoutTarget, + candidates: candidates, + parallelRunnerOverhead: s.parallelRunnerOverhead, + targetTime: s.targetTime, + available: true, + } +} + +func (s splitSelector) meetsTargetTime(score splitScore) bool { + return s.targetTime > 0 && score.wallTimeDuration() <= s.targetTime +} + +func (s splitSelector) maybeWarnTargetTimeUnreachable(best splitScore, minParallelism, maxParallelism int) { + if s.targetTime <= 0 || s.meetsTargetTime(best) { + return + } + + slog.Warn("No parallel runner split meets target time; selecting split with lowest expected wall time", + "targetTime", s.targetTime, + "minParallelism", minParallelism, + "maxParallelism", maxParallelism, + "selectedParallelRunners", best.parallelRunners, + "selectedExpectedWallTime", best.wallTimeDuration()) +} + func (s splitSelector) logCandidate(score splitScore) { slog.Debug("Considered parallel runner split", "parallelRunners", score.parallelRunners, @@ -100,5 +177,7 @@ func (s splitSelector) logCandidate(score splitScore) { "imbalance", score.imbalanceDuration(), "expectedTotalRuntime", score.totalRuntimeDuration(), "parallelRunnerOverhead", s.parallelRunnerOverhead, + "targetTime", s.targetTime, + "meetsTargetTime", s.meetsTargetTime(score), "selectionScore", time.Duration(s.selectionScore(score))*time.Millisecond) } diff --git a/internal/planner/parallelism_test.go b/internal/planner/parallelism_test.go index d9d10d1..76f125c 100644 --- a/internal/planner/parallelism_test.go +++ b/internal/planner/parallelism_test.go @@ -14,7 +14,7 @@ import ( const testParallelRunnerOverhead = 25 * time.Second func testCalculateParallelRunners(testFileWeights map[string]int, minParallelism, maxParallelism int) int { - return calculateParallelRunnerSplit(testFileWeights, minParallelism, maxParallelism, testParallelRunnerOverhead).parallelRunners + return calculateParallelRunnerSplit(testFileWeights, minParallelism, maxParallelism, testParallelRunnerOverhead, 0).parallelRunners } func TestCalculateParallelRunners_MaxParallelismIsOne(t *testing.T) { @@ -144,7 +144,7 @@ func TestCalculateParallelRunnerSplit_ReturnsSelectedScore(t *testing.T) { "test5.rb": 6, } - result := calculateParallelRunnerSplit(testFileWeights, 3, 4, testParallelRunnerOverhead) + result := calculateParallelRunnerSplit(testFileWeights, 3, 4, testParallelRunnerOverhead, 0) expected := splitScore{ parallelRunners: 3, wallTime: 12, @@ -201,7 +201,7 @@ func TestCalculateParallelRunnerSplit_RealGitHubActionsArtifacts(t *testing.T) { ) } - result := calculateParallelRunnerSplit(fixture.TestFileWeights, 1, 8, testParallelRunnerOverhead) + result := calculateParallelRunnerSplit(fixture.TestFileWeights, 1, 8, testParallelRunnerOverhead, 0) if result.parallelRunners != tt.expectedParallelRunners { t.Fatalf( "calculateParallelRunnerSplit() = %d runners, expected %d; artifact selected %d before this fix", @@ -236,7 +236,7 @@ func TestCalculateParallelRunnerSplit_ParallelRunnerOverheadTunesFanout(t *testi for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := calculateParallelRunnerSplit(fixture.TestFileWeights, 1, 8, tt.parallelRunnerOverhead) + result := calculateParallelRunnerSplit(fixture.TestFileWeights, 1, 8, tt.parallelRunnerOverhead, 0) if result.parallelRunners != tt.expectedParallelRunners { t.Errorf("calculateParallelRunnerSplit() = %d runners, expected %d", result.parallelRunners, tt.expectedParallelRunners) } @@ -244,6 +244,59 @@ func TestCalculateParallelRunnerSplit_ParallelRunnerOverheadTunesFanout(t *testi } } +func TestCalculateParallelRunnerSplit_TargetTimeFiltersCandidateSplits(t *testing.T) { + testFileWeights := map[string]int{ + "test1.rb": 60_000, + "test2.rb": 60_000, + "test3.rb": 60_000, + "test4.rb": 60_000, + } + minParallelism := 1 + maxParallelism := 4 + parallelRunnerOverhead := 5 * time.Minute + targetTime := 2 * time.Minute + + result := calculateParallelRunnerSplit(testFileWeights, minParallelism, maxParallelism, parallelRunnerOverhead, targetTime) + if result.parallelRunners != 2 { + t.Errorf("calculateParallelRunnerSplit() = %d runners, expected 2 to satisfy target time with the lowest selection score", result.parallelRunners) + } + if result.wallTimeDuration() > targetTime { + t.Errorf("calculateParallelRunnerSplit() wall time = %s, expected at or below %s", result.wallTimeDuration(), targetTime) + } +} + +func TestCalculateParallelRunnerSplit_TargetTimeImpossibleWarnsAndSelectsLowestWallTime(t *testing.T) { + logs := captureLogs(t) + testFileWeights := map[string]int{ + "test1.rb": 60_000, + "test2.rb": 60_000, + "test3.rb": 60_000, + "test4.rb": 60_000, + } + minParallelism := 1 + maxParallelism := 4 + parallelRunnerOverhead := 5 * time.Minute + targetTime := 59 * time.Second + + result := calculateParallelRunnerSplitSelection(testFileWeights, minParallelism, maxParallelism, parallelRunnerOverhead, targetTime) + if result.selected.parallelRunners != 4 { + t.Errorf("calculateParallelRunnerSplit() = %d runners, expected 4 from fallback lowest wall time split", result.selected.parallelRunners) + } + if result.selected.wallTimeDuration() != time.Minute { + t.Errorf("calculateParallelRunnerSplit() wall time = %s, expected 1m0s", result.selected.wallTimeDuration()) + } + if result.bestWithoutTarget.parallelRunners != 1 { + t.Errorf("calculateParallelRunnerSplit() without target = %d runners, expected 1 from best selection score", result.bestWithoutTarget.parallelRunners) + } + + logOutput := logs.String() + if !strings.Contains(logOutput, "No parallel runner split meets target time") || + !strings.Contains(logOutput, "targetTime=59s") || + !strings.Contains(logOutput, "selectedExpectedWallTime=1m0s") { + t.Errorf("expected target-time warning with fallback details, got logs: %s", logOutput) + } +} + type splitSelectionFixture struct { SkippablePercentage float64 `json:"skippablePercentage"` CurrentParallelRunners int `json:"currentParallelRunners"` @@ -276,7 +329,7 @@ func TestCalculateParallelRunnerSplit_LogsCandidateSplits(t *testing.T) { "test3.rb": 10, } - _ = calculateParallelRunnerSplit(testFileWeights, 1, 3, testParallelRunnerOverhead) + _ = calculateParallelRunnerSplit(testFileWeights, 1, 3, testParallelRunnerOverhead, 0) logOutput := logs.String() if strings.Count(logOutput, "Considered parallel runner split") != 3 || @@ -298,6 +351,6 @@ func BenchmarkCalculateParallelRunners20000TestFiles(b *testing.B) { b.ResetTimer() for range b.N { - _ = calculateParallelRunnerSplit(testFileWeights, 1, 256, testParallelRunnerOverhead) + _ = calculateParallelRunnerSplit(testFileWeights, 1, 256, testParallelRunnerOverhead, 0) } } diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 34d2230..33ad4e5 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -209,12 +209,14 @@ func (tp *TestPlanner) Plan(ctx context.Context) error { return fmt.Errorf("failed to write skippable percentage: %w", err) } - parallelRunnerSplit := calculateParallelRunnerSplit( + parallelRunnerSelection := calculateParallelRunnerSplitSelection( tp.testFileWeights, settings.GetMinParallelism(), settings.GetMaxParallelism(), settings.GetParallelRunnerOverhead(), + settings.GetTargetTime(), ) + parallelRunnerSplit := parallelRunnerSelection.selected parallelRunners := parallelRunnerSplit.parallelRunners runnersContent := fmt.Sprintf("%d", parallelRunners) if err := writePlanFile(constants.ParallelRunnersOutputPath, []byte(runnersContent)); err != nil { @@ -237,7 +239,7 @@ func (tp *TestPlanner) Plan(ctx context.Context) error { } if settings.GetReportEnabled() { - printPlanReport(tp.reportWriter, tp, parallelRunnerSplit) + printPlanReport(tp.reportWriter, tp, parallelRunnerSelection) } tp.planLoaded = true diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index b0b59e2..beb16b8 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -749,9 +749,9 @@ func TestTestPlanner_Setup_WithParallelRunners(t *testing.T) { report := reportOutput.String() for _, expectedReportLine := range []string{ "Planning\n", - " Runner split\n", - " Estimated runtime:", - " Runners: 1", + " Split selection\n", + " Estimated runtime with TIA:", + " Selected: 1 runner", " Expected wall time:", " Imbalance:", } { diff --git a/internal/planner/report.go b/internal/planner/report.go index a521cda..d4c232b 100644 --- a/internal/planner/report.go +++ b/internal/planner/report.go @@ -14,8 +14,8 @@ import ( "github.com/DataDog/ddtest/internal/settings" ) -func printPlanReport(w io.Writer, tp *TestPlanner, split splitScore) { - printPlanReportData(w, tp.newPlanReportData(split)) +func printPlanReport(w io.Writer, tp *TestPlanner, selection splitSelection) { + printPlanReportData(w, tp.newPlanReportData(selection.selected).withSplitSelection(selection)) } func printPlanReportData(w io.Writer, report PlanReportData) { @@ -138,6 +138,7 @@ func defaultDDTestSettings() settings.Config { MinParallelism: settings.DefaultParallelism(), MaxParallelism: settings.DefaultParallelism(), ParallelRunnerOverhead: settings.DefaultParallelRunnerOverhead(), + TargetTime: settings.DefaultTargetTime(), CiNode: -1, CiNodeWorkers: 1, TestSkippingLevel: settings.TestSkippingLevelTest, @@ -177,7 +178,7 @@ func printPlanningReport(w io.Writer, report PlanReportData) { printDurationEstimatesPlanningReport(w, report.Planning.Durations) printSkippingPlanningReport(w, report.DatadogSettings, report.Skippables, report.ManagedFlakyTests, report.Planning.Skipping) printRunSetPlanningReport(w, report.Planning) - printRunnerSplitPlanningReport(w, report.Planning, report.Split) + printRunnerSplitPlanningReport(w, report) } func printLongSeparateRunnerSuitesReport(w io.Writer, suites []testSuiteTimingReport) { @@ -379,9 +380,12 @@ func printRunSetPlanningReport(w io.Writer, planning planningReport) { reportFprintf(w, " Estimated time saved: %.2f%%\n", planning.EstimatedTimeSaved) } -func printRunnerSplitPlanningReport(w io.Writer, planning planningReport, split splitScore) { +func printRunnerSplitPlanningReport(w io.Writer, report PlanReportData) { + planning := report.Planning + selection := effectiveSplitSelection(report) + split := selection.selected if split.parallelRunners <= 0 { - reportFprintln(w, " Runner split: not available") + reportFprintln(w, " Split selection: not available") return } @@ -390,12 +394,209 @@ func printRunnerSplitPlanningReport(w io.Writer, planning planningReport, split fullDuration = formatDuration(planning.Durations.ExpectedFullDuration) } - reportFprintln(w, " Runner split") - reportFprintf(w, " Full runtime: %s\n", fullDuration) - reportFprintf(w, " Estimated runtime: %s\n", formatDuration(split.totalRuntimeDuration())) - reportFprintf(w, " Runners: %s\n", formatCount(split.parallelRunners)) + reportFprintln(w, " Split selection") + reportFprintf(w, " Full test suite time without TIA: %s\n", fullDuration) + reportFprintf(w, " Estimated runtime with TIA: %s\n", formatDuration(split.totalRuntimeDuration())) + reportFprintf(w, " Selected: %s\n", formatRunnerCount(split.parallelRunners)) + if selection.available { + reportFprintf(w, " Reason: %s\n", formatSplitSelectionReason(selection)) + printTargetTimeReport(w, selection) + } reportFprintf(w, " Expected wall time: %s\n", formatDuration(split.wallTimeDuration())) + if selection.available { + reportFprintf(w, " Modeled CI overhead: %s (%s x configured CI job overhead %s)\n", + formatDuration(selection.overheadDuration(split)), + formatRunnerCount(split.parallelRunners), + formatDuration(selection.parallelRunnerOverhead)) + reportFprintf(w, " Selection score: %s (wall time + modeled CI overhead)\n", formatDuration(selection.scoreDuration(split))) + } reportFprintf(w, " Imbalance: %s\n", formatDuration(split.imbalanceDuration())) + if selection.available && selection.targetTime > 0 { + printWithoutTargetTimeReport(w, selection) + } + if selection.available && len(selection.candidates) > 0 { + printSplitCandidatesReport(w, selection) + } +} + +func effectiveSplitSelection(report PlanReportData) splitSelection { + if report.SplitSelection.available { + return report.SplitSelection + } + return splitSelection{ + selected: report.Split, + bestWithoutTarget: report.Split, + } +} + +func formatSplitSelectionReason(selection splitSelection) string { + if selection.targetTime <= 0 { + return "lowest selection score" + } + if selection.meetsTargetTime(selection.selected) { + if sameSplitScore(selection.selected, selection.bestWithoutTarget) { + return "lowest selection score; target time satisfied" + } + return "lowest selection score among splits that meet target time" + } + return "no split met target time; selected lowest wall time to get closest to target" +} + +func printTargetTimeReport(w io.Writer, selection splitSelection) { + if selection.targetTime <= 0 { + return + } + if selection.meetsTargetTime(selection.selected) { + reportFprintf(w, " Target time: %s, satisfied\n", formatDuration(selection.targetTime)) + return + } + reportFprintf(w, " Target time: %s, not reachable (best wall time %s)\n", + formatDuration(selection.targetTime), + formatDuration(selection.bestWallTime())) +} + +func printWithoutTargetTimeReport(w io.Writer, selection splitSelection) { + best := selection.bestWithoutTarget + if sameSplitScore(selection.selected, best) { + reportFprintln(w) + reportFprintln(w, " Without target time: same as selected") + return + } + + reportFprintln(w) + reportFprintf(w, " Without target time: %s (wall %s, overhead %s, score %s)\n", + formatRunnerCount(best.parallelRunners), + formatDuration(best.wallTimeDuration()), + formatDuration(selection.overheadDuration(best)), + formatDuration(selection.scoreDuration(best))) + reportFprintf(w, " Selected vs without target: %s\n", formatSelectedVsWithoutTarget(selection)) +} + +func printSplitCandidatesReport(w io.Writer, selection splitSelection) { + candidates := selection.sortedCandidatesByScore() + limit := 5 + if len(candidates) < limit { + limit = len(candidates) + } + + reportFprintln(w) + if len(candidates) > limit { + reportFprintf(w, " Candidates (best %d of %d by score)\n", limit, len(candidates)) + } else { + reportFprintln(w, " Candidates") + } + for _, candidate := range candidates[:limit] { + reason := formatSplitCandidateReason(selection, candidate) + if reason != "" { + reason = ", " + reason + } + reportFprintf(w, " %s: wall %s, overhead %s, score %s%s\n", + formatRunnerCount(candidate.parallelRunners), + formatDuration(candidate.wallTimeDuration()), + formatDuration(selection.overheadDuration(candidate)), + formatDuration(selection.scoreDuration(candidate)), + reason) + } +} + +func formatSplitCandidateReason(selection splitSelection, candidate splitScore) string { + parts := make([]string, 0, 3) + if selection.targetTime > 0 { + if selection.meetsTargetTime(candidate) { + parts = append(parts, "met target") + } else { + parts = append(parts, "missed target by "+formatDuration(candidate.wallTimeDuration()-selection.targetTime)) + } + } + if selection.targetTime > 0 && sameSplitScore(candidate, selection.bestWithoutTarget) && !sameSplitScore(candidate, selection.selected) { + parts = append(parts, "would choose without target time") + } + if sameSplitScore(candidate, selection.selected) { + parts = append(parts, "selected") + } + return strings.Join(parts, "; ") +} + +func formatSelectedVsWithoutTarget(selection splitSelection) string { + return formatWallTimeDifference(selection.selected, selection.bestWithoutTarget) + ", " + + formatOverheadDifference(selection.overheadDuration(selection.selected), selection.overheadDuration(selection.bestWithoutTarget)) +} + +func formatWallTimeDifference(selected, withoutTarget splitScore) string { + switch { + case selected.wallTime < withoutTarget.wallTime: + return formatDuration(withoutTarget.wallTimeDuration()-selected.wallTimeDuration()) + " faster wall time" + case selected.wallTime > withoutTarget.wallTime: + return formatDuration(selected.wallTimeDuration()-withoutTarget.wallTimeDuration()) + " slower wall time" + default: + return "same wall time" + } +} + +func formatOverheadDifference(selected, withoutTarget time.Duration) string { + switch { + case selected > withoutTarget: + return formatDuration(selected-withoutTarget) + " more CI overhead" + case selected < withoutTarget: + return formatDuration(withoutTarget-selected) + " less CI overhead" + default: + return "same CI overhead" + } +} + +func formatRunnerCount(count int) string { + return formatCountWithUnit(count, "runner", "runners") +} + +func sameSplitScore(a, b splitScore) bool { + return a.parallelRunners == b.parallelRunners && + a.wallTime == b.wallTime && + a.imbalance == b.imbalance && + a.totalRuntime == b.totalRuntime +} + +func (s splitSelection) overheadDuration(score splitScore) time.Duration { + if s.parallelRunnerOverhead <= 0 { + return 0 + } + return time.Duration(score.parallelRunners) * s.parallelRunnerOverhead +} + +func (s splitSelection) scoreDuration(score splitScore) time.Duration { + return score.wallTimeDuration() + s.overheadDuration(score) +} + +func (s splitSelection) meetsTargetTime(score splitScore) bool { + return s.targetTime > 0 && score.wallTimeDuration() <= s.targetTime +} + +func (s splitSelection) bestWallTime() time.Duration { + if len(s.candidates) == 0 { + return s.selected.wallTimeDuration() + } + best := s.candidates[0].wallTimeDuration() + for _, candidate := range s.candidates[1:] { + if candidate.wallTimeDuration() < best { + best = candidate.wallTimeDuration() + } + } + return best +} + +func (s splitSelection) sortedCandidatesByScore() []splitScore { + candidates := slices.Clone(s.candidates) + selector := splitSelector{parallelRunnerOverhead: s.parallelRunnerOverhead} + slices.SortFunc(candidates, func(a, b splitScore) int { + switch { + case selector.better(a, b): + return -1 + case selector.better(b, a): + return 1 + default: + return 0 + } + }) + return candidates } func formatAppliedTIASkippables(datadogSettings datadogSettingsReport, skippables skippablesReport, skipping skippingApplicationReport) string { diff --git a/internal/planner/report_data.go b/internal/planner/report_data.go index 20d05b6..55908a9 100644 --- a/internal/planner/report_data.go +++ b/internal/planner/report_data.go @@ -241,6 +241,7 @@ type PlanReportData struct { LongSeparateRunnerSuites []testSuiteTimingReport SlowestTestSuitesOverall []testSuiteTimingReport Split splitScore + SplitSelection splitSelection } type planningReportStats struct { @@ -272,6 +273,11 @@ func (tp *TestPlanner) newPlanReportData(split splitScore) PlanReportData { return addBackendDataReports(report, tp.optimizationClient) } +func (report PlanReportData) withSplitSelection(selection splitSelection) PlanReportData { + report.SplitSelection = selection + return report +} + func (tp *TestPlanner) recordDiscoveryReport(mode discoveryMode, cache discoveryCacheResult, duration time.Duration) { tp.reportStats.discoveryMode = mode tp.reportStats.discoveryCache = cache diff --git a/internal/planner/report_test.go b/internal/planner/report_test.go index 654085d..0fc83ac 100644 --- a/internal/planner/report_test.go +++ b/internal/planner/report_test.go @@ -44,6 +44,7 @@ func TestPrintPlanReport_AllData(t *testing.T) { MinParallelism: minParallelism, MaxParallelism: maxParallelism, ParallelRunnerOverhead: 30 * time.Second, + TargetTime: 5 * time.Minute, WorkerEnv: "RAILS_ENV=test;DATABASE_PASSWORD=secret", CiNode: 0, CiNodeWorkers: 2, @@ -132,6 +133,49 @@ func TestPrintPlanReport_AllData(t *testing.T) { imbalance: 11000, totalRuntime: 1426000, }, + SplitSelection: splitSelection{ + selected: splitScore{ + parallelRunners: 6, + wallTime: 252000, + imbalance: 11000, + totalRuntime: 1426000, + }, + bestWithoutTarget: splitScore{ + parallelRunners: 4, + wallTime: 305000, + imbalance: 20000, + totalRuntime: 1426000, + }, + candidates: []splitScore{ + { + parallelRunners: 1, + wallTime: 1426000, + imbalance: 0, + totalRuntime: 1426000, + }, + { + parallelRunners: 4, + wallTime: 305000, + imbalance: 20000, + totalRuntime: 1426000, + }, + { + parallelRunners: 5, + wallTime: 290000, + imbalance: 15000, + totalRuntime: 1426000, + }, + { + parallelRunners: 6, + wallTime: 252000, + imbalance: 11000, + totalRuntime: 1426000, + }, + }, + parallelRunnerOverhead: 30 * time.Second, + targetTime: 5 * time.Minute, + available: true, + }, LongSeparateRunnerSuites: []testSuiteTimingReport{ { Runner: 0, @@ -180,6 +224,7 @@ DDTest settings Min parallelism: %s Max parallelism: %s CI job overhead: 30s + Target time: 5m0s Worker env: DATABASE_PASSWORD, RAILS_ENV CI node: 0 CI node workers: 2 @@ -229,13 +274,26 @@ Planning Run set Test files to run: 524 Estimated time saved: 38.40%% - Runner split - Full runtime: 37m12s - Estimated runtime: 23m46s - Runners: 6 + Split selection + Full test suite time without TIA: 37m12s + Estimated runtime with TIA: 23m46s + Selected: 6 runners + Reason: lowest selection score among splits that meet target time + Target time: 5m0s, satisfied Expected wall time: 4m12s + Modeled CI overhead: 3m0s (6 runners x configured CI job overhead 30s) + Selection score: 7m12s (wall time + modeled CI overhead) Imbalance: 11s + Without target time: 4 runners (wall 5m5s, overhead 2m0s, score 7m5s) + Selected vs without target: 53s faster wall time, 1m0s more CI overhead + + Candidates + 4 runners: wall 5m5s, overhead 2m0s, score 7m5s, missed target by 5s; would choose without target time + 6 runners: wall 4m12s, overhead 3m0s, score 7m12s, met target; selected + 5 runners: wall 4m50s, overhead 2m30s, score 7m20s, met target + 1 runner: wall 23m46s, overhead 30s, score 24m16s, missed target by 18m46s + Slow suites on dedicated runners ATTENTION: 1 dedicated runner 1. runner 0, rspec / Checkout::Slow (spec/slow_spec.rb): historical duration 2m0s, estimated runtime 1m40s @@ -381,10 +439,10 @@ Planning Run set Test files to run: 18 Estimated time saved: 25.00% - Runner split - Full runtime: not available - Estimated runtime: 3m30s - Runners: 3 + Split selection + Full test suite time without TIA: not available + Estimated runtime with TIA: 3m30s + Selected: 3 runners Expected wall time: 1m30s Imbalance: 500ms @@ -436,8 +494,53 @@ func TestPrintPlanReport_MissingSettingsAndData(t *testing.T) { if !strings.Contains(report, " Run set: not available") { t.Errorf("expected missing run set message, got:\n%s", report) } - if !strings.Contains(report, " Runner split: not available") { - t.Errorf("expected missing runner split message, got:\n%s", report) + if !strings.Contains(report, " Split selection: not available") { + t.Errorf("expected missing split selection message, got:\n%s", report) + } +} + +func TestPrintSplitCandidatesReport_LimitsAndSortsByScore(t *testing.T) { + selection := splitSelection{ + selected: splitScore{parallelRunners: 2, wallTime: 80_000, totalRuntime: 100_000}, + parallelRunnerOverhead: 10 * time.Second, + available: true, + candidates: []splitScore{ + {parallelRunners: 6, wallTime: 59_000, totalRuntime: 100_000}, + {parallelRunners: 5, wallTime: 60_000, totalRuntime: 100_000}, + {parallelRunners: 1, wallTime: 100_000, totalRuntime: 100_000}, + {parallelRunners: 4, wallTime: 65_000, totalRuntime: 100_000}, + {parallelRunners: 3, wallTime: 70_000, totalRuntime: 100_000}, + {parallelRunners: 2, wallTime: 80_000, totalRuntime: 100_000}, + }, + } + + var output strings.Builder + printSplitCandidatesReport(&output, selection) + + report := output.String() + for _, want := range []string{ + "Candidates (best 5 of 6 by score)", + "2 runners: wall 1m20s, overhead 20s, score 1m40s, selected", + "3 runners: wall 1m10s, overhead 30s, score 1m40s", + "4 runners: wall 1m5s, overhead 40s, score 1m45s", + "1 runner: wall 1m40s, overhead 10s, score 1m50s", + "5 runners: wall 1m0s, overhead 50s, score 1m50s", + } { + if !strings.Contains(report, want) { + t.Fatalf("expected candidates report to contain %q, got:\n%s", want, report) + } + } + if strings.Contains(report, "6 runners:") { + t.Fatalf("expected candidates report to omit sixth candidate, got:\n%s", report) + } + orderedCandidates := []string{"2 runners:", "3 runners:", "4 runners:", "1 runner:", "5 runners:"} + previousIndex := -1 + for _, candidate := range orderedCandidates { + index := strings.Index(report, candidate) + if index <= previousIndex { + t.Fatalf("expected candidates sorted by score, got:\n%s", report) + } + previousIndex = index } } @@ -463,6 +566,7 @@ func TestPrintDDTestSettingsReport_AllSupportedSettings(t *testing.T) { config.MinParallelism++ config.MaxParallelism += 2 config.ParallelRunnerOverhead += time.Second + config.TargetTime = 12 * time.Minute config.CiNode = 0 config.CiNodeWorkers = 2 config.WorkerEnv = "TOKEN=secret" @@ -498,6 +602,7 @@ func TestPrintDDTestSettingsReport_AllSupportedSettings(t *testing.T) { "Min parallelism", "Max parallelism", "CI job overhead", + "Target time", "Worker env", "CI node", "CI node workers", diff --git a/internal/settings/settings.go b/internal/settings/settings.go index ef6a50c..07a5567 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -16,6 +16,7 @@ import ( const ( defaultCiNodeWorkers = 1 defaultParallelRunnerOverhead = 25 * time.Second + defaultTargetTime = 0 * time.Second ncpuCiNodeWorkers = "ncpu" envPrefix = "DD_TEST_OPTIMIZATION_RUNNER" platformEnv = "DD_TEST_OPTIMIZATION_RUNNER_PLATFORM" @@ -23,6 +24,7 @@ const ( minParallelismEnv = "DD_TEST_OPTIMIZATION_RUNNER_MIN_PARALLELISM" maxParallelismEnv = "DD_TEST_OPTIMIZATION_RUNNER_MAX_PARALLELISM" parallelRunnerOverheadEnv = "DD_TEST_OPTIMIZATION_RUNNER_CI_JOB_OVERHEAD" + targetTimeEnv = "DD_TEST_OPTIMIZATION_RUNNER_TARGET_TIME" workerEnv = "DD_TEST_OPTIMIZATION_RUNNER_WORKER_ENV" ciNodeEnv = "DD_TEST_OPTIMIZATION_RUNNER_CI_NODE" ciNodeWorkersEnv = "DD_TEST_OPTIMIZATION_RUNNER_CI_NODE_WORKERS" @@ -58,6 +60,11 @@ func DefaultParallelRunnerOverhead() time.Duration { return defaultParallelRunnerOverhead } +// DefaultTargetTime returns the default target wall time for a runner split. +func DefaultTargetTime() time.Duration { + return defaultTargetTime +} + // PhysicalCPUCount returns the number of physical CPU cores available to this process. // // It starts from runtime.GOMAXPROCS(0), which is the number of logical CPUs the @@ -112,6 +119,7 @@ type Config struct { MinParallelism int `mapstructure:"min_parallelism"` MaxParallelism int `mapstructure:"max_parallelism"` ParallelRunnerOverhead time.Duration `mapstructure:"parallel_runner_overhead"` + TargetTime time.Duration `mapstructure:"target_time"` WorkerEnv string `mapstructure:"worker_env"` CiNode int `mapstructure:"ci_node"` CiNodeWorkers int `mapstructure:"ci_node_workers"` @@ -138,6 +146,10 @@ func Init() { fmt.Fprintf(os.Stderr, "Error binding parallel runner overhead env: %v\n", err) os.Exit(1) } + if err := viper.BindEnv("target_time", targetTimeEnv); err != nil { + fmt.Fprintf(os.Stderr, "Error binding target time env: %v\n", err) + os.Exit(1) + } if err := viper.BindEnv("tests_location", testsLocationEnv, knapsackTestFilePatternEnv); err != nil { fmt.Fprintf(os.Stderr, "Error binding tests location env: %v\n", err) os.Exit(1) @@ -175,12 +187,26 @@ func Init() { os.Exit(1) } viper.Set("ci_node_workers", ciNodeWorkers) - parallelRunnerOverhead, err := ParseParallelRunnerOverhead(viper.GetString("parallel_runner_overhead")) + parallelRunnerOverhead, err := ParseNonNegativeDurationSetting( + viper.GetString("parallel_runner_overhead"), + defaultParallelRunnerOverhead, + "ci-job-overhead", + ) if err != nil { fmt.Fprintf(os.Stderr, "Error loading config: %v\n", err) os.Exit(1) } viper.Set("parallel_runner_overhead", parallelRunnerOverhead) + targetTime, err := ParseNonNegativeDurationSetting( + viper.GetString("target_time"), + defaultTargetTime, + "target-time", + ) + if err != nil { + fmt.Fprintf(os.Stderr, "Error loading config: %v\n", err) + os.Exit(1) + } + viper.Set("target_time", targetTime) viper.Set("test_skipping_mode", NormalizeTestSkippingLevel(TestSkippingLevel(viper.GetString("test_skipping_mode")))) config = &Config{} @@ -196,6 +222,7 @@ func setDefaults() { viper.SetDefault("min_parallelism", DefaultParallelism()) viper.SetDefault("max_parallelism", DefaultParallelism()) viper.SetDefault("parallel_runner_overhead", defaultParallelRunnerOverhead.String()) + viper.SetDefault("target_time", defaultTargetTime.String()) viper.SetDefault("worker_env", "") viper.SetDefault("ci_node", -1) viper.SetDefault("ci_node_workers", strconv.Itoa(defaultCiNodeWorkers)) @@ -225,23 +252,22 @@ func (level TestSkippingLevel) String() string { return string(level) } -// ParseParallelRunnerOverhead resolves the modeled overhead for adding another -// parallel runner. It accepts Go duration strings such as "25s", "1m", -// "1500ms", or "0s" to disable the runner-overhead bias. -func ParseParallelRunnerOverhead(value string) (time.Duration, error) { +// ParseNonNegativeDurationSetting resolves a duration setting from a Go +// duration string such as "25s", "1m", "1500ms", or "0s". +func ParseNonNegativeDurationSetting(value string, defaultValue time.Duration, settingName string) (time.Duration, error) { normalized := strings.TrimSpace(value) if normalized == "" { - return defaultParallelRunnerOverhead, nil + return defaultValue, nil } - overhead, err := time.ParseDuration(normalized) + duration, err := time.ParseDuration(normalized) if err != nil { - return 0, fmt.Errorf("ci-job-overhead must be a duration like %q, %q, %q, or %q, got %q", "25s", "1m", "1500ms", "0s", value) + return 0, fmt.Errorf("%s must be a duration like %q, %q, %q, or %q, got %q", settingName, "25s", "1m", "1500ms", "0s", value) } - if overhead < 0 { - return 0, fmt.Errorf("ci-job-overhead must be non-negative, got %q", value) + if duration < 0 { + return 0, fmt.Errorf("%s must be non-negative, got %q", settingName, value) } - return overhead, nil + return duration, nil } // ParseCiNodeWorkers resolves the ci_node_workers setting from either a positive integer @@ -292,6 +318,10 @@ func GetParallelRunnerOverhead() time.Duration { return Get().ParallelRunnerOverhead } +func GetTargetTime() time.Duration { + return Get().TargetTime +} + func GetWorkerEnv() string { return Get().WorkerEnv } diff --git a/internal/settings/settings_test.go b/internal/settings/settings_test.go index 5563134..325dfbe 100644 --- a/internal/settings/settings_test.go +++ b/internal/settings/settings_test.go @@ -2,6 +2,7 @@ package settings import ( "os" + "strings" "testing" "time" @@ -9,6 +10,7 @@ import ( ) const expectedDefaultParallelRunnerOverhead = 25 * time.Second +const expectedDefaultTargetTime = 0 * time.Second func TestDefaultParallelism(t *testing.T) { result := DefaultParallelism() @@ -28,6 +30,12 @@ func TestDefaultParallelRunnerOverhead(t *testing.T) { } } +func TestDefaultTargetTime(t *testing.T) { + if DefaultTargetTime() != expectedDefaultTargetTime { + t.Errorf("expected default target time to be %s, got %s", expectedDefaultTargetTime, DefaultTargetTime()) + } +} + func TestPhysicalCPUCount(t *testing.T) { result := PhysicalCPUCount() if result < 1 { @@ -131,6 +139,9 @@ func TestInit(t *testing.T) { if config.ParallelRunnerOverhead != expectedDefaultParallelRunnerOverhead { t.Errorf("expected default parallel_runner_overhead to be %s, got %s", expectedDefaultParallelRunnerOverhead, config.ParallelRunnerOverhead) } + if config.TargetTime != expectedDefaultTargetTime { + t.Errorf("expected default target_time to be %s, got %s", expectedDefaultTargetTime, config.TargetTime) + } if config.WorkerEnv != "" { t.Errorf("expected default worker_env to be empty, got %q", config.WorkerEnv) } @@ -190,6 +201,9 @@ func TestSetDefaults(t *testing.T) { if viper.GetString("parallel_runner_overhead") != expectedDefaultParallelRunnerOverhead.String() { t.Errorf("expected default parallel_runner_overhead to be %s, got %q", expectedDefaultParallelRunnerOverhead, viper.GetString("parallel_runner_overhead")) } + if viper.GetString("target_time") != expectedDefaultTargetTime.String() { + t.Errorf("expected default target_time to be %s, got %q", expectedDefaultTargetTime, viper.GetString("target_time")) + } if viper.GetString("worker_env") != "" { t.Errorf("expected default worker_env to be empty, got %q", viper.GetString("worker_env")) } @@ -293,6 +307,7 @@ func TestEnvironmentVariables(t *testing.T) { _ = os.Setenv(minParallelismEnv, "2") _ = os.Setenv(maxParallelismEnv, "8") _ = os.Setenv(parallelRunnerOverheadEnv, "40s") + _ = os.Setenv(targetTimeEnv, "10m") _ = os.Setenv(workerEnv, "RAILS_DB=my_project_dev_{{nodeIndex}}") _ = os.Setenv(ciNodeEnv, "5") _ = os.Setenv(ciNodeWorkersEnv, "4") @@ -311,6 +326,7 @@ func TestEnvironmentVariables(t *testing.T) { _ = os.Unsetenv(minParallelismEnv) _ = os.Unsetenv(maxParallelismEnv) _ = os.Unsetenv(parallelRunnerOverheadEnv) + _ = os.Unsetenv(targetTimeEnv) _ = os.Unsetenv(workerEnv) _ = os.Unsetenv(ciNodeEnv) _ = os.Unsetenv(ciNodeWorkersEnv) @@ -342,6 +358,9 @@ func TestEnvironmentVariables(t *testing.T) { if config.ParallelRunnerOverhead != 40*time.Second { t.Errorf("expected parallel_runner_overhead from env var to be 40s, got %s", config.ParallelRunnerOverhead) } + if config.TargetTime != 10*time.Minute { + t.Errorf("expected target_time from env var to be 10m, got %s", config.TargetTime) + } if config.WorkerEnv != "RAILS_DB=my_project_dev_{{nodeIndex}}" { t.Errorf("expected worker_env from env var to be 'RAILS_DB=my_project_dev_{{nodeIndex}}', got %q", config.WorkerEnv) } @@ -434,6 +453,22 @@ func TestGetParallelRunnerOverhead(t *testing.T) { } } +func TestGetTargetTime(t *testing.T) { + config = nil + viper.Reset() + + targetTime := GetTargetTime() + if targetTime != expectedDefaultTargetTime { + t.Errorf("expected target_time to be %s, got %s", expectedDefaultTargetTime, targetTime) + } + + config = &Config{TargetTime: 12 * time.Minute} + targetTime = GetTargetTime() + if targetTime != 12*time.Minute { + t.Errorf("expected target_time to be 12m, got %s", targetTime) + } +} + func TestGetWorkerEnv(t *testing.T) { // Test with defaults config = nil @@ -901,62 +936,83 @@ func TestParseCiNodeWorkers(t *testing.T) { } } -func TestParseParallelRunnerOverhead(t *testing.T) { +func TestParseNonNegativeDurationSetting(t *testing.T) { tests := []struct { - name string - value string - expected time.Duration - expectErr bool + name string + value string + defaultValue time.Duration + settingName string + expected time.Duration + expectErr bool }{ { - name: "empty value uses default", - value: "", - expected: expectedDefaultParallelRunnerOverhead, + name: "empty overhead value uses overhead default", + value: "", + defaultValue: expectedDefaultParallelRunnerOverhead, + settingName: "ci-job-overhead", + expected: expectedDefaultParallelRunnerOverhead, }, { - name: "seconds", - value: "25s", - expected: expectedDefaultParallelRunnerOverhead, + name: "empty target value uses target default", + value: "", + defaultValue: expectedDefaultTargetTime, + settingName: "target-time", + expected: expectedDefaultTargetTime, }, { - name: "minutes", - value: "1m", - expected: time.Minute, + name: "seconds", + value: "25s", + settingName: "ci-job-overhead", + expected: expectedDefaultParallelRunnerOverhead, }, { - name: "milliseconds", - value: "1500ms", - expected: 1500 * time.Millisecond, + name: "minutes", + value: "1m", + settingName: "target-time", + expected: time.Minute, }, { - name: "zero disables bias", - value: "0s", - expected: 0, + name: "milliseconds", + value: "1500ms", + settingName: "target-time", + expected: 1500 * time.Millisecond, }, { - name: "rejects negative values", - value: "-1s", - expectErr: true, + name: "zero disables duration setting", + value: "0s", + settingName: "target-time", + expected: 0, }, { - name: "rejects plain integers", - value: "25", - expectErr: true, + name: "rejects negative values", + value: "-1s", + settingName: "target-time", + expectErr: true, }, { - name: "rejects unknown strings", - value: "many", - expectErr: true, + name: "rejects plain integers", + value: "25", + settingName: "ci-job-overhead", + expectErr: true, + }, + { + name: "rejects unknown strings", + value: "fast", + settingName: "target-time", + expectErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := ParseParallelRunnerOverhead(tt.value) + result, err := ParseNonNegativeDurationSetting(tt.value, tt.defaultValue, tt.settingName) if tt.expectErr { if err == nil { t.Fatalf("expected error, got nil") } + if !strings.Contains(err.Error(), tt.settingName) { + t.Fatalf("expected error to contain setting name %q, got %v", tt.settingName, err) + } return }