diff --git a/src/bin/scope.rs b/src/bin/scope.rs index 979470a..92463ab 100644 --- a/src/bin/scope.rs +++ b/src/bin/scope.rs @@ -86,7 +86,7 @@ async fn main() { async fn run_subcommand(opts: Cli) -> i32 { let loaded_config = match opts.config.load_config().await { Err(e) => { - error!(target: "user", "Failed to load configuration: {}", e); + error!(target: "user", "Failed to load configuration: {:#}", e); return 2; } Ok(c) => c, diff --git a/src/shared/config_load.rs b/src/shared/config_load.rs index 4274dbd..9125e8e 100644 --- a/src/shared/config_load.rs +++ b/src/shared/config_load.rs @@ -165,8 +165,14 @@ impl FoundConfig { }; for raw_config in raw_config { - if let Ok(value) = raw_config.try_into() { - this.add_model(value); + let full_name = raw_config.full_name(); + let file_path = raw_config.file_path(); + + match raw_config.try_into() { + Ok(value) => this.add_model(value), + Err(e) => { + warn!(target: "user", "Unable to load {} from {} because {}", full_name.bold(), file_path, e); + } } } @@ -508,4 +514,106 @@ mod tests { let nonexistent_path = Path::new("/this/path/does/not/exist/hopefully"); build_config_path(nonexistent_path); } + + /// Writes each `(file_name, contents)` pair under a fresh temp `.scope` dir and loads it + /// through the real `FoundConfig::new` entrypoint. + async fn found_config_from_files(files: &[(&str, &str)]) -> FoundConfig { + let temp_dir = tempdir().unwrap(); + let scope_dir = temp_dir.path().join(".scope"); + fs::create_dir_all(&scope_dir).unwrap(); + for (name, contents) in files { + fs::write(scope_dir.join(name), contents).unwrap(); + } + + let config_options = ConfigOptions::parse_from(["scope"]); + FoundConfig::new( + &config_options, + temp_dir.path().to_path_buf(), + vec![scope_dir], + ) + .await + } + + /// Regression test for https://github.com/Gusto/scope/issues/344: a `ScopeKnownError` that + /// fails its own validation (here, a reserved capture group name) must be logged instead of + /// silently registering nothing, while a valid known error in the same directory still loads. + #[tokio::test] + async fn invalid_known_error_warns_and_continues() { + let found_config = found_config_from_files(&[ + ( + "bad-known-error.yaml", + r#" +apiVersion: scope.github.com/v1alpha +kind: ScopeKnownError +metadata: + name: bad-reserved +spec: + pattern: "boom: (?.*)" + help: "should never load" +"#, + ), + ( + "good-known-error.yaml", + r#" +apiVersion: scope.github.com/v1alpha +kind: ScopeKnownError +metadata: + name: good +spec: + pattern: "boom" + help: "fine" +"#, + ), + ]) + .await; + + assert!(!found_config.known_error.contains_key("bad-reserved")); + assert!(found_config.known_error.contains_key("good")); + } + + /// Companion to `invalid_known_error_warns_and_continues`: a `ScopeDoctorGroup` that fails to + /// build (here, a broken fix template) should warn and be dropped, not abort the whole load — + /// the pre-existing, intentional behavior from #68/PR #69. A valid group in the same directory + /// must still load, proving this is "drop the bad one", not "load nothing". + #[tokio::test] + async fn invalid_doctor_group_warns_and_continues() { + let found_config = found_config_from_files(&[ + ( + "broken-group.yaml", + r#" +apiVersion: scope.github.com/v1alpha +kind: ScopeDoctorGroup +metadata: + name: broken +spec: + actions: + - name: action1 + check: {} + fix: + commands: + - "echo {{ unterminated" +"#, + ), + ( + "good-group.yaml", + r#" +apiVersion: scope.github.com/v1alpha +kind: ScopeDoctorGroup +metadata: + name: good +spec: + actions: + - name: action1 + check: {} + fix: + commands: + - echo fine +"#, + ), + ]) + .await; + + assert!(!found_config.doctor_group.contains_key("broken")); + assert!(found_config.doctor_group.contains_key("good")); + } } diff --git a/src/shared/models/internal/mod.rs b/src/shared/models/internal/mod.rs index 0a53f3a..3d9b72a 100644 --- a/src/shared/models/internal/mod.rs +++ b/src/shared/models/internal/mod.rs @@ -1,3 +1,4 @@ +use crate::models::HelpMetadata; use crate::models::InternalScopeModel; use crate::models::prelude::{ ModelRoot, V1AlphaDoctorGroup, V1AlphaKnownError, V1AlphaReportLocation, @@ -63,18 +64,21 @@ impl TryFrom> for ParsedConfig { type Error = anyhow::Error; fn try_from(value: ModelRoot) -> Result { - if let Ok(Some(known)) = V1AlphaDoctorGroup::known_type(&value) { + if let Some(known) = V1AlphaDoctorGroup::known_type(&value)? { return Ok(ParsedConfig::DoctorGroup(DoctorGroup::try_from(known)?)); } - if let Ok(Some(known)) = V1AlphaKnownError::known_type(&value) { + if let Some(known) = V1AlphaKnownError::known_type(&value)? { return Ok(ParsedConfig::KnownError(KnownError::try_from(known)?)); } - if let Ok(Some(known)) = V1AlphaReportLocation::known_type(&value) { + if let Some(known) = V1AlphaReportLocation::known_type(&value)? { return Ok(ParsedConfig::ReportUpload(ReportUploadLocation::try_from( known, )?)); } - Err(anyhow!("Error was know a known type")) + Err(anyhow!( + "'{}' did not match the apiVersion/kind of any known resource", + value.full_name() + )) } }