Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bin/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
112 changes: 110 additions & 2 deletions src/shared/config_load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -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: (?<working_dir>.*)"
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"));
}
}
12 changes: 8 additions & 4 deletions src/shared/models/internal/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::models::HelpMetadata;
use crate::models::InternalScopeModel;
use crate::models::prelude::{
ModelRoot, V1AlphaDoctorGroup, V1AlphaKnownError, V1AlphaReportLocation,
Expand Down Expand Up @@ -63,18 +64,21 @@ impl TryFrom<ModelRoot<Value>> for ParsedConfig {
type Error = anyhow::Error;

fn try_from(value: ModelRoot<Value>) -> Result<Self, Self::Error> {
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()
))
}
}

Expand Down
Loading