Skip to content

Commit fccfc51

Browse files
refactor: extract config discovery into DiscoveredProjectConfig
Move config file discovery logic from `ProjectConfig` into a dedicated `DiscoveredProjectConfig` struct that preserves the config file path. This removes the side-effect of `set_current_dir()` during discovery and instead exposes `config_dir()` for callers to resolve paths relative to the config file location. Also stop merging `working_directory` from config in the merger since it needs path resolution relative to the config file directory, which the merger doesn't have access to.
1 parent 21437cb commit fccfc51

7 files changed

Lines changed: 167 additions & 212 deletions

File tree

src/cli/mod.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use crate::{
1515
config::CodSpeedConfig,
1616
local_logger::{CODSPEED_U8_COLOR_CODE, init_local_logger},
1717
prelude::*,
18-
project_config::ProjectConfig,
18+
project_config::DiscoveredProjectConfig,
1919
};
2020
use clap::{
2121
Parser, Subcommand,
@@ -97,9 +97,11 @@ pub async fn run() -> Result<()> {
9797
CodSpeedConfig::load_with_override(cli.config_name.as_deref(), cli.oauth_token.as_deref())?;
9898
let api_client = CodSpeedAPIClient::try_from((&cli, &codspeed_config))?;
9999

100-
// Discover project configuration file (this may change the working directory)
101-
let project_config =
102-
ProjectConfig::discover_and_load(cli.config.as_deref(), &std::env::current_dir()?)?;
100+
// Discover project configuration file
101+
let discovered_config = DiscoveredProjectConfig::discover_and_load(
102+
cli.config.as_deref(),
103+
&std::env::current_dir()?,
104+
)?;
103105

104106
// In the context of the CI, it is likely that a ~ made its way here without being expanded by the shell
105107
let setup_cache_dir = cli
@@ -121,7 +123,7 @@ pub async fn run() -> Result<()> {
121123
*args,
122124
&api_client,
123125
&codspeed_config,
124-
project_config.as_ref(),
126+
discovered_config.as_ref(),
125127
setup_cache_dir,
126128
)
127129
.await?
@@ -131,7 +133,7 @@ pub async fn run() -> Result<()> {
131133
*args,
132134
&api_client,
133135
&codspeed_config,
134-
project_config.as_ref(),
136+
discovered_config.as_ref().map(|d| &d.config),
135137
setup_cache_dir,
136138
)
137139
.await?

src/cli/run/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ use crate::executor;
55
use crate::executor::config::{self, OrchestratorConfig, RepositoryOverride};
66
use crate::instruments::Instruments;
77
use crate::prelude::*;
8-
use crate::project_config::ProjectConfig;
98
use crate::project_config::merger::ConfigMerger;
9+
use crate::project_config::{DiscoveredProjectConfig, ProjectConfig};
1010
use crate::run_environment::interfaces::RepositoryProvider;
1111
use crate::upload::poll_results::PollResultsOptions;
1212
use clap::{Args, ValueEnum};
@@ -152,13 +152,13 @@ pub async fn run(
152152
args: RunArgs,
153153
api_client: &CodSpeedAPIClient,
154154
codspeed_config: &CodSpeedConfig,
155-
project_config: Option<&ProjectConfig>,
155+
discovered_config: Option<&DiscoveredProjectConfig>,
156156
setup_cache_dir: Option<&Path>,
157157
) -> Result<()> {
158158
let output_json = args.message_format == Some(MessageFormat::Json);
159+
let project_config = discovered_config.map(|d| &d.config);
159160

160161
let args = args.merge_with_project_config(project_config);
161-
162162
let run_target = if args.command.is_empty() {
163163
// No command provided - check for targets in project config
164164
let targets = project_config

src/executor/tests.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,6 @@ fi
370370
// Unset GITHUB_ACTIONS to force LocalProvider which supports repository_override
371371
temp_env::async_with_vars(&[("GITHUB_ACTIONS", None::<&str>)], async {
372372
let config = walltime_config(&wrapped_command, true);
373-
dbg!(&config);
374373
let (execution_context, _temp_dir) = create_test_setup(config).await;
375374
executor.run(&execution_context, &None).await.unwrap();
376375
})

src/project_config/discover.rs

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
use crate::prelude::*;
2+
use std::path::{Path, PathBuf};
3+
4+
use super::ProjectConfig;
5+
6+
/// Config file names in priority order
7+
const CONFIG_FILENAMES: &[&str] = &[
8+
"codspeed.yaml",
9+
"codspeed.yml",
10+
".codspeed.yaml",
11+
".codspeed.yml",
12+
];
13+
14+
/// A project configuration paired with the path it was loaded from.
15+
#[derive(Debug)]
16+
#[allow(dead_code)]
17+
pub struct DiscoveredProjectConfig {
18+
pub config: ProjectConfig,
19+
pub path: PathBuf,
20+
}
21+
22+
impl DiscoveredProjectConfig {
23+
/// Discover and load project configuration file
24+
///
25+
/// # Search Strategy
26+
/// 1. If `config_path_override` is provided, load from that path only (error if not found)
27+
/// 2. Otherwise, search for config files in current directory and upward to git root
28+
/// 3. Try filenames in priority order: codspeed.yaml, codspeed.yml, .codspeed.yaml, .codspeed.yml
29+
pub fn discover_and_load(
30+
config_path_override: Option<&Path>,
31+
current_dir: &Path,
32+
) -> Result<Option<DiscoveredProjectConfig>> {
33+
// Case 1: Explicit --config path provided
34+
if let Some(config_path) = config_path_override {
35+
let config = ProjectConfig::load_from_path(config_path)
36+
.with_context(|| format!("Failed to load config from {}", config_path.display()))?;
37+
38+
return Ok(Some(DiscoveredProjectConfig {
39+
config,
40+
path: config_path.to_path_buf(),
41+
}));
42+
}
43+
44+
// Case 2: Search for config files
45+
let search_dirs = Self::get_search_directories(current_dir)?;
46+
47+
for dir in search_dirs {
48+
for filename in CONFIG_FILENAMES {
49+
let candidate_path = dir.join(filename);
50+
if candidate_path.exists() {
51+
debug!("Found config file at {}", candidate_path.display());
52+
let config = ProjectConfig::load_from_path(&candidate_path)?;
53+
54+
return Ok(Some(DiscoveredProjectConfig {
55+
config,
56+
path: candidate_path,
57+
}));
58+
}
59+
}
60+
}
61+
62+
// No config found - this is OK
63+
Ok(None)
64+
}
65+
66+
/// Returns the directory containing the config file.
67+
#[allow(dead_code)]
68+
pub fn config_dir(&self) -> Option<PathBuf> {
69+
let canonical_path = self
70+
.path
71+
.canonicalize()
72+
.unwrap_or_else(|_| self.path.clone());
73+
canonical_path.parent().map(|p| p.to_path_buf())
74+
}
75+
76+
/// Get list of directories to search for config files
77+
///
78+
/// Returns directories from current_dir upward to git root (if in a git repo)
79+
fn get_search_directories(current_dir: &Path) -> Result<Vec<PathBuf>> {
80+
let mut dirs = vec![current_dir.to_path_buf()];
81+
82+
// Try to find git repository root
83+
if let Some(git_root) = crate::cli::run::helpers::find_repository_root(current_dir) {
84+
// Add parent directories up to git root
85+
let mut dir = current_dir.to_path_buf();
86+
while let Some(parent) = dir.parent() {
87+
if parent == git_root {
88+
if !dirs.contains(&git_root) {
89+
dirs.push(git_root.clone());
90+
}
91+
break;
92+
}
93+
if !dirs.contains(&parent.to_path_buf()) {
94+
dirs.push(parent.to_path_buf());
95+
}
96+
dir = parent.to_path_buf();
97+
}
98+
}
99+
100+
Ok(dirs)
101+
}
102+
}

src/project_config/merger.rs

Lines changed: 35 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,12 @@ impl ConfigMerger {
4141
/// Note: Some fields like upload_url, token, repository are CLI-only and not in config.
4242
pub fn merge_shared_args(
4343
cli: &ExecAndRunSharedArgs,
44-
config_opts: Option<&ProjectOptions>,
44+
_config_opts: Option<&ProjectOptions>,
4545
) -> ExecAndRunSharedArgs {
46-
let mut merged = cli.clone();
47-
48-
// Merge working_directory if not set via CLI
49-
if merged.working_directory.is_none() {
50-
if let Some(opts) = config_opts {
51-
merged.working_directory = opts.working_directory.clone();
52-
}
53-
}
54-
55-
merged
46+
// Note: working_directory is NOT merged here because config paths need to be
47+
// resolved relative to the config file directory. This resolution is handled
48+
// by the caller (e.g., `codspeed run`) which has access to the config file path.
49+
cli.clone()
5650
}
5751

5852
/// Helper to merge Option values with precedence: CLI > config > None
@@ -67,6 +61,29 @@ mod tests {
6761
use crate::cli::PerfRunArgs;
6862
use crate::runner_mode::RunnerMode;
6963

64+
fn make_cli(working_directory: Option<&str>) -> ExecAndRunSharedArgs {
65+
ExecAndRunSharedArgs {
66+
upload_url: None,
67+
token: None,
68+
repository: None,
69+
provider: None,
70+
working_directory: working_directory.map(|s| s.to_string()),
71+
mode: vec![RunnerMode::Walltime],
72+
simulation_tool: None,
73+
profile_folder: None,
74+
skip_upload: false,
75+
skip_run: false,
76+
skip_setup: false,
77+
allow_empty: false,
78+
go_runner_version: None,
79+
show_full_output: false,
80+
perf_run_args: PerfRunArgs {
81+
enable_perf: true,
82+
perf_unwinding_mode: None,
83+
},
84+
}
85+
}
86+
7087
#[test]
7188
fn test_merge_walltime_all_from_cli() {
7289
let cli = WalltimeExecutionArgs {
@@ -146,27 +163,7 @@ mod tests {
146163

147164
#[test]
148165
fn test_merge_shared_args_working_directory_from_cli() {
149-
let cli = ExecAndRunSharedArgs {
150-
upload_url: None,
151-
token: None,
152-
repository: None,
153-
provider: None,
154-
working_directory: Some("./cli-dir".to_string()),
155-
mode: vec![RunnerMode::Walltime],
156-
simulation_tool: None,
157-
profile_folder: None,
158-
skip_upload: false,
159-
skip_run: false,
160-
skip_setup: false,
161-
allow_empty: false,
162-
go_runner_version: None,
163-
show_full_output: false,
164-
perf_run_args: PerfRunArgs {
165-
enable_perf: true,
166-
perf_unwinding_mode: None,
167-
},
168-
};
169-
166+
let cli = make_cli(Some("./cli-dir"));
170167
let config = ProjectOptions {
171168
walltime: None,
172169
working_directory: Some("./config-dir".to_string()),
@@ -179,69 +176,30 @@ mod tests {
179176
}
180177

181178
#[test]
182-
fn test_merge_shared_args_working_directory_from_config() {
183-
let cli = ExecAndRunSharedArgs {
184-
upload_url: None,
185-
token: None,
186-
repository: None,
187-
provider: None,
188-
working_directory: None,
189-
mode: vec![RunnerMode::Walltime],
190-
simulation_tool: None,
191-
profile_folder: None,
192-
skip_upload: false,
193-
skip_run: false,
194-
skip_setup: false,
195-
allow_empty: false,
196-
go_runner_version: None,
197-
show_full_output: false,
198-
perf_run_args: PerfRunArgs {
199-
enable_perf: true,
200-
perf_unwinding_mode: None,
201-
},
202-
};
203-
179+
fn test_merge_shared_args_working_directory_not_merged_from_config() {
180+
let cli = make_cli(None);
204181
let config = ProjectOptions {
205182
walltime: None,
206183
working_directory: Some("./config-dir".to_string()),
207184
};
208185

209186
let merged = ConfigMerger::merge_shared_args(&cli, Some(&config));
210187

211-
// Config working_directory should be used
212-
assert_eq!(merged.working_directory, Some("./config-dir".to_string()));
188+
// Config working_directory is NOT merged — resolution is handled by the caller
189+
// relative to the config file directory.
190+
assert_eq!(merged.working_directory, None);
213191
// Mode stays as CLI value
214192
assert_eq!(merged.mode, vec![RunnerMode::Walltime]);
215193
}
216194

217195
#[test]
218196
fn test_merge_shared_args_no_config() {
219-
let cli = ExecAndRunSharedArgs {
220-
upload_url: None,
221-
token: None,
222-
repository: None,
223-
provider: None,
224-
working_directory: Some("./dir".to_string()),
225-
mode: vec![RunnerMode::Simulation],
226-
simulation_tool: None,
227-
profile_folder: None,
228-
skip_upload: false,
229-
skip_run: false,
230-
skip_setup: false,
231-
allow_empty: false,
232-
go_runner_version: None,
233-
show_full_output: false,
234-
perf_run_args: PerfRunArgs {
235-
enable_perf: false,
236-
perf_unwinding_mode: None,
237-
},
238-
};
197+
let cli = make_cli(Some("./dir"));
239198

240199
let merged = ConfigMerger::merge_shared_args(&cli, None);
241200

242201
// Should be identical to CLI
243202
assert_eq!(merged.working_directory, Some("./dir".to_string()));
244-
assert_eq!(merged.mode, vec![RunnerMode::Simulation]);
245203
}
246204

247205
#[test]

0 commit comments

Comments
 (0)