From b77bbcad639ca84b3ce69857bc9d54e21b607943 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sat, 25 Jul 2026 15:09:34 +0300 Subject: [PATCH 1/2] Add --workload-path to aggregate, for parity with run add_workload_source() offers a workload either as a repository or as a local path, and run, list and info all use it. aggregate defined only --workload-repository, so a workload developed locally and benchmarked with run --workload-path could not be aggregated afterwards: solr-orbit: error: unrecognized arguments: --workload-path=... aggregate now uses the same helper, and goes through configure_workload_params so that a path is normalised and validated the way it is everywhere else -- that is also what rejects --workload-revision together with --workload-path. Since the path sets workload.path, which is what makes the loader choose SimpleWorkloadRepository, aggregate() no longer names a repository when one was given. The fixture supplied no workload_path, so the aggregator saw a Mock attribute that happens to be truthy; it is now explicit, and the two new tests cover each mode. --- solrorbit/aggregator.py | 5 ++++- solrorbit/benchmark.py | 6 ++---- tests/aggregator_test.py | 33 +++++++++++++++++++++++++++++++-- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/solrorbit/aggregator.py b/solrorbit/aggregator.py index 7f099314..f4296cdc 100644 --- a/solrorbit/aggregator.py +++ b/solrorbit/aggregator.py @@ -272,7 +272,10 @@ def aggregate(self) -> None: if self.test_run_compatibility_check(): self.test_run = self.test_store.find_by_test_run_id(list(self.test_runs.keys())[0]) self.test_procedure_name = self.test_run.test_procedure - self.config.add(config.Scope.applicationOverride, "workload", "repository.name", self.args.workload_repository) + # a workload given as a path is already configured; naming a repository too would send the + # loader looking for the workload in that repository instead + if not self.args.workload_path: + self.config.add(config.Scope.applicationOverride, "workload", "repository.name", self.args.workload_repository) self.config.add(config.Scope.applicationOverride, "workload", "workload.name", self.test_run.workload) self.loaded_workload = workload.load_workload(self.config) for id in self.test_runs.keys(): diff --git a/solrorbit/benchmark.py b/solrorbit/benchmark.py index 5834a0c9..e85a142b 100644 --- a/solrorbit/benchmark.py +++ b/solrorbit/benchmark.py @@ -353,10 +353,7 @@ def add_workload_source(subparser): "--results-file", help="Write the aggregated results to the provided file.", default="") - aggregate_parser.add_argument( - "--workload-repository", - help="Define the repository from where solr-orbit will load workloads (default: default).", - default="default") + add_workload_source(aggregate_parser) download_parser = subparsers.add_parser("download", help="Downloads an artifact") download_parser.add_argument( @@ -1213,6 +1210,7 @@ def dispatch_sub_command(arg_parser, args, cfg): cfg.add(config.Scope.applicationOverride, "reporting", "percentiles", args.percentiles) publisher.compare(cfg, args.baseline, args.contender) elif sub_command == "aggregate": + configure_workload_params(arg_parser, args, cfg, command_requires_workload=False) test_runs_dict = prepare_test_runs_dict(args, cfg) aggregator_instance = aggregator.Aggregator(cfg, test_runs_dict, args) aggregator_instance.aggregate() diff --git a/tests/aggregator_test.py b/tests/aggregator_test.py index 7bc046ad..ea391056 100644 --- a/tests/aggregator_test.py +++ b/tests/aggregator_test.py @@ -1,4 +1,4 @@ -from unittest.mock import Mock +from unittest.mock import Mock, patch import pytest from solrorbit import config from solrorbit.aggregator import Aggregator, AggregatedResults @@ -21,7 +21,8 @@ def mock_args(): return Mock( results_file="", test_run_id="", - workload_repository="default" + workload_repository="default", + workload_path=None ) @pytest.fixture @@ -120,6 +121,34 @@ def test_calculate_weighted_average(aggregator): assert result["latency"]["avg"] == 16 # (10*2 + 20*3) / (2+3) assert result["latency"]["unit"] == "ms" +def test_aggregate_names_the_workload_repository(aggregator): + aggregator.test_store.find_by_test_run_id.side_effect = None + aggregator.test_store.find_by_test_run_id.return_value = Mock( + results={}, workload="workload1", test_procedure="test_proc1") + + with patch("solrorbit.workload.load_workload"), patch.object(aggregator, "build_aggregated_results"), \ + patch("solrorbit.aggregator.FileTestRunStore"): + aggregator.aggregate() + + aggregator.config.add.assert_any_call(config.Scope.applicationOverride, "workload", + "repository.name", "default") + +def test_aggregate_leaves_a_workload_path_alone(aggregator): + # a workload given as --workload-path is loaded from that path; naming a repository as well would + # send the loader to the repository instead + aggregator.args.workload_path = "/path/to/geonames" + aggregator.test_store.find_by_test_run_id.side_effect = None + aggregator.test_store.find_by_test_run_id.return_value = Mock( + results={}, workload="workload1", test_procedure="test_proc1") + + with patch("solrorbit.workload.load_workload"), patch.object(aggregator, "build_aggregated_results"), \ + patch("solrorbit.aggregator.FileTestRunStore"): + aggregator.aggregate() + + repository_calls = [call for call in aggregator.config.add.call_args_list + if call.args[2] == "repository.name"] + assert repository_calls == [] + def test_calculate_rsd(aggregator): values = [1, 2, 3, 4, 5] rsd = aggregator.calculate_rsd(values, "test_metric") From e04a4181705b23863a3d1c3697d42458fb0b761b Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Mon, 27 Jul 2026 17:42:08 +0300 Subject: [PATCH 2/2] Document the workload-source options for aggregate The aggregate reference page listed only --test-runs, --test-runs-id and --results-file, while command-flags.md already documented --workload-repository for aggregate. The two pages disagreed, and neither mentioned --workload-revision, which aggregate has accepted all along via add_workload_source(). Adds --workload-path (new in this PR), --workload-repository and --workload-revision to both pages, using the wording already used for run. --- docs/reference/commands/aggregate.md | 3 +++ docs/reference/commands/command-flags.md | 2 ++ 2 files changed, 5 insertions(+) diff --git a/docs/reference/commands/aggregate.md b/docs/reference/commands/aggregate.md index 1a2b5518..790317ff 100644 --- a/docs/reference/commands/aggregate.md +++ b/docs/reference/commands/aggregate.md @@ -58,6 +58,9 @@ solr-orbit aggregate --test-runs ID1,ID2[,...] [OPTIONS] | `--test-runs` | Comma-separated list of test run IDs to aggregate | | `--test-runs-id` | Custom ID for the aggregated result (auto-generated if omitted) | | `--results-file` | Path to write the aggregated results JSON | +| `--workload-path` | Path to a local workload directory | +| `--workload-repository` | Git URL for the workloads repository (default: `"default"`, resolved from `benchmark.ini`) | +| `--workload-revision` | Git revision (branch, tag, or commit) of the workloads repository; optional, uses the repository's default branch if omitted | ## Output diff --git a/docs/reference/commands/command-flags.md b/docs/reference/commands/command-flags.md index 9631f9a2..532862de 100644 --- a/docs/reference/commands/command-flags.md +++ b/docs/reference/commands/command-flags.md @@ -171,7 +171,9 @@ These flags enable automated load-ramp and redline testing to find a cluster's p | `--test-runs` | Comma-separated test run IDs to aggregate | | `--test-runs-id` | Custom ID for the aggregated result | | `--results-file` | Path to write the aggregated results JSON | +| `--workload-path` | Local workload directory | | `--workload-repository` | Git URL for the workloads repository | +| `--workload-revision` | Git revision of the workloads repository | ## download flags