Skip to content

Commit

Permalink
Rewrite remaining panic! to percolate Result<_, CargoMakeError> t…
Browse files Browse the repository at this point in the history
…hroughout
  • Loading branch information
SamuelMarks committed Jul 13, 2024
1 parent 7354658 commit 8bd60c7
Show file tree
Hide file tree
Showing 32 changed files with 312 additions and 235 deletions.
13 changes: 3 additions & 10 deletions src/lib/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,8 @@ pub fn run(cli_args: &CliArgs, global_config: &GlobalConfig) -> Result<(), Cargo
let env = cli_args.env.clone();

let experimental = cli_args.experimental;
let descriptor_load_result = descriptor::load(&build_file, force_makefile, env, experimental);

let config = match descriptor_load_result {
Ok(config) => config,
Err(ref error) => {
error!("{}", error);
panic!("{}", error);
}
};
let config = descriptor::load(&build_file, force_makefile, env, experimental)?;

let mut time_summary_vec = vec![];
time_summary::add(
&mut time_summary_vec,
Expand Down Expand Up @@ -141,7 +134,7 @@ pub fn run(cli_args: &CliArgs, global_config: &GlobalConfig) -> Result<(), Cargo
cli_args.hide_uninteresting,
)
} else if cli_args.diff_execution_plan {
let default_config = descriptor::load_internal_descriptors(true, experimental, None);
let default_config = descriptor::load_internal_descriptors(true, experimental, None)?;
cli_commands::diff_steps::run(
&default_config,
&config,
Expand Down
4 changes: 2 additions & 2 deletions src/lib/cli_commands/diff_steps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ pub(crate) fn run(
let internal_file = create_file(
&move |file: &mut File| write_as_string(&internal_execution_plan, &file),
"toml",
);
)?;
let external_file = create_file(
&move |file: &mut File| write_as_string(&external_execution_plan, &file),
"toml",
);
)?;

info!("Printing diff...");
command::run_command(
Expand Down
2 changes: 1 addition & 1 deletion src/lib/cli_commands/list_steps_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn check(
let mut path = PathBuf::new();
path.push(&file);

let actual = io::read_text_file(&path);
let actual = io::read_text_file(&path).unwrap();
io::delete_file(&file);

expect.assert_eq(&actual);
Expand Down
9 changes: 5 additions & 4 deletions src/lib/descriptor/cargo_alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#[path = "cargo_alias_test.rs"]
mod cargo_alias_test;

use crate::error::CargoMakeError;
use crate::io;
use crate::types::{InstallCrate, Task};
use std::collections::HashMap;
Expand All @@ -24,13 +25,13 @@ struct CargoConfig {
alias: Option<HashMap<String, AliasValue>>,
}

fn load_from_file(file: &str) -> Vec<(String, Task)> {
fn load_from_file(file: &str) -> Result<Vec<(String, Task)>, CargoMakeError> {
let file_path = Path::new(file);

let mut tasks = vec![];
if file_path.exists() {
if file_path.is_file() {
let text = io::read_text_file(&file_path.to_path_buf());
let text = io::read_text_file(&file_path.to_path_buf())?;

if !text.is_empty() {
let cargo_config: CargoConfig = match toml::from_str(&text) {
Expand All @@ -57,9 +58,9 @@ fn load_from_file(file: &str) -> Vec<(String, Task)> {
}
}

tasks
Ok(tasks)
}

pub(crate) fn load() -> Vec<(String, Task)> {
pub(crate) fn load() -> Result<Vec<(String, Task)>, CargoMakeError> {
load_from_file("./.cargo/config.toml")
}
8 changes: 4 additions & 4 deletions src/lib/descriptor/cargo_alias_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,28 @@ use super::*;

#[test]
fn load_from_file_no_file() {
let tasks = load_from_file("./badfile.toml");
let tasks = load_from_file("./badfile.toml").unwrap();

assert!(tasks.is_empty());
}

#[test]
fn load_from_file_parse_error() {
let tasks = load_from_file("./src/lib/test/cargo/invalid_config.toml");
let tasks = load_from_file("./src/lib/test/cargo/invalid_config.toml").unwrap();

assert!(tasks.is_empty());
}

#[test]
fn load_from_file_no_alias_data() {
let tasks = load_from_file("./Cargo.toml");
let tasks = load_from_file("./Cargo.toml").unwrap();

assert!(tasks.is_empty());
}

#[test]
fn load_from_file_aliases_found() {
let tasks = load_from_file("./src/lib/test/cargo/config.toml");
let tasks = load_from_file("./src/lib/test/cargo/config.toml").unwrap();

assert_eq!(tasks.len(), 4);

Expand Down
30 changes: 19 additions & 11 deletions src/lib/descriptor/descriptor_deserializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@
#[path = "descriptor_deserializer_test.rs"]
mod descriptor_deserializer_test;

use crate::error::CargoMakeError;
use crate::types::{Config, ExternalConfig};

pub(crate) fn load_config(descriptor_string: &str, validate: bool) -> Config {
pub(crate) fn load_config(
descriptor_string: &str,
validate: bool,
) -> Result<Config, CargoMakeError> {
let config: Config = if validate {
let deserializer = toml::de::Deserializer::new(descriptor_string);

Expand All @@ -18,34 +22,38 @@ pub(crate) fn load_config(descriptor_string: &str, validate: bool) -> Config {
Ok(value) => value,
Err(error) => {
error!("Unable to parse internal descriptor: {}", error);
panic!("Unable to parse internal descriptor: {}", error);
return Err(CargoMakeError::DescriptorParseFailed(error.to_string()));
}
}
} else {
match toml::from_str(descriptor_string) {
Ok(value) => value,
Err(error) => {
error!("Unable to parse internal descriptor: {}", error);
panic!("Unable to parse internal descriptor: {}", error);
return Err(CargoMakeError::DescriptorParseFailed(error.to_string()));
}
}
};

config
Ok(config)
}

pub(crate) fn load_external_config(descriptor_string: &str, file: &str) -> ExternalConfig {
pub(crate) fn load_external_config(
descriptor_string: &str,
file: &str,
) -> Result<ExternalConfig, CargoMakeError> {
let deserializer = toml::de::Deserializer::new(descriptor_string);

let config: ExternalConfig = match serde_ignored::deserialize(deserializer, |path| {
match serde_ignored::deserialize(deserializer, |path| {
warn!("Found unknown key: {} in file: {}", path, file);
}) {
Ok(value) => value,
Ok(value) => Ok(value),
Err(error) => {
error!("Unable to parse external file: {:#?}, {}", &file, error);
panic!("Unable to parse external file: {:#?}, {}", &file, error);
return Err(CargoMakeError::ParseFileFailed(
String::from(file),
error.to_string(),
));
}
};

config
}
}
15 changes: 9 additions & 6 deletions src/lib/descriptor/descriptor_deserializer_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@ use crate::descriptor::makefiles;

#[test]
fn load_config_base() {
load_config(makefiles::BASE, true);
load_config(makefiles::BASE, true).unwrap();
}

#[test]
fn load_config_stable() {
load_config(makefiles::STABLE, true);
load_config(makefiles::STABLE, true).unwrap();
}

#[test]
fn load_config_beta() {
load_config(makefiles::BETA, true);
load_config(makefiles::BETA, true).unwrap();
}

#[test]
Expand All @@ -35,7 +35,8 @@ description = "Empty Task"
category2 = "Tools"
"#,
true,
);
)
.unwrap();
}

#[test]
Expand All @@ -56,7 +57,8 @@ description = "Empty Task"
category2 = "Tools"
"#,
false,
);
)
.unwrap();

assert!(config.tasks.contains_key("empty"));
}
Expand All @@ -70,7 +72,8 @@ description = "Empty Task"
category2 = "Tools"
"#,
"somefile",
);
)
.unwrap();

assert!(config.tasks.unwrap().contains_key("empty"));
}
18 changes: 10 additions & 8 deletions src/lib/descriptor/makefiles/mod_test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::condition;
use crate::descriptor;
use crate::error::CargoMakeError;
use crate::runner;
use crate::scriptengine;
use crate::scriptengine::EngineType;
Expand All @@ -10,7 +11,7 @@ use rust_info::types::RustInfo;
use std::cell::RefCell;
use std::rc::Rc;

fn load_descriptor() -> Config {
fn load_descriptor() -> Result<Config, CargoMakeError> {
descriptor::load_internal_descriptors(true, false, None)
}

Expand Down Expand Up @@ -42,7 +43,7 @@ fn create_flow_info(config: &Config) -> FlowInfo {

fn makefile_task_condition_test(name: &str, expect_enabled: bool, linux_only: bool, ci_only: bool) {
if !linux_only || test::is_linux() {
let config = load_descriptor();
let config = load_descriptor().unwrap();
let task = get_task(name, &config);
let flow_info = create_flow_info(&config);
let step = Step {
Expand Down Expand Up @@ -75,22 +76,23 @@ fn makefile_task_disabled_test(name: &str, linux_only: bool) {
}

fn makefile_task_script_engine_test(name: &str, engine: EngineType) {
let config = load_descriptor();
let config = load_descriptor().unwrap();
let task = get_task(name, &config);

let output = scriptengine::get_engine_type(
&task.script.unwrap(),
&task.script_runner,
&task.script_extension,
);
)
.unwrap();

assert_eq!(output, engine);
}

#[test]
fn makefile_coverage_test() {
if test::is_linux() {
let config = load_descriptor();
let config = load_descriptor().unwrap();
let task = get_task("coverage", &config);
let run_task_info = task.run_task.unwrap();

Expand Down Expand Up @@ -167,7 +169,7 @@ fn makefile_build_file_increment_no_file_test() {

let name = "build-file-increment";

let config = load_descriptor();
let config = load_descriptor().unwrap();
let task = get_task(name, &config);

let flow_info = create_flow_info(&config);
Expand Down Expand Up @@ -199,7 +201,7 @@ fn makefile_build_file_increment_file_exists_test() {

let name = "build-file-increment";

let config = load_descriptor();
let config = load_descriptor().unwrap();
let task = get_task(name, &config);

let flow_info = create_flow_info(&config);
Expand Down Expand Up @@ -236,7 +238,7 @@ fn makefile_build_file_increment_panic_invalid_data_test() {

let name = "build-file-increment";

let config = load_descriptor();
let config = load_descriptor().unwrap();
let task = get_task(name, &config);

let flow_info = create_flow_info(&config);
Expand Down
Loading

0 comments on commit 8bd60c7

Please sign in to comment.