From 07744c97d93672330cebe89a43a9eb9ee5d5cea7 Mon Sep 17 00:00:00 2001 From: Arni Hod Date: Mon, 18 Nov 2024 20:58:31 +0200 Subject: [PATCH] chore(infra): add error variant for file not found on get path --- Cargo.lock | 3 +++ crates/infra_utils/Cargo.toml | 3 +++ crates/infra_utils/src/path.rs | 20 +++++++++++++++++-- .../src/starknet_api_test_utils.rs | 3 ++- crates/papyrus_config/Cargo.toml | 1 + crates/papyrus_config/src/config_test.rs | 2 +- crates/papyrus_config/src/lib.rs | 3 +++ crates/papyrus_node/src/config/config_test.rs | 12 +++++------ .../src/config/config_test.rs | 4 ++-- .../src/config/node_config.rs | 2 +- .../src/compile_test.rs | 3 ++- 11 files changed, 42 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 297eda57fee..296d2c50326 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5163,6 +5163,9 @@ checksum = "b248f5224d1d606005e02c97f5aa4e88eeb230488bcc03bc9ca4d7991399f2b5" [[package]] name = "infra_utils" version = "0.0.0" +dependencies = [ + "thiserror", +] [[package]] name = "inout" diff --git a/crates/infra_utils/Cargo.toml b/crates/infra_utils/Cargo.toml index 69c33687b69..ed189315777 100644 --- a/crates/infra_utils/Cargo.toml +++ b/crates/infra_utils/Cargo.toml @@ -8,3 +8,6 @@ description = "Infrastructure utility." [lints] workspace = true + +[dependencies] +thiserror.workspace = true diff --git a/crates/infra_utils/src/path.rs b/crates/infra_utils/src/path.rs index a6aa6d74119..16dcd77f449 100644 --- a/crates/infra_utils/src/path.rs +++ b/crates/infra_utils/src/path.rs @@ -2,6 +2,17 @@ use std::env; use std::path::{Path, PathBuf}; use std::sync::LazyLock; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum GetPathError { + // TODO(Arni): Handle manifest dir not exist here? + #[error("No file exists at '{path}'")] + PathDoesNotExist { path: PathBuf }, + #[error(transparent)] + IoError(#[from] std::io::Error), +} + static PATH_TO_CARGO_MANIFEST_DIR: LazyLock> = LazyLock::new(|| env::var("CARGO_MANIFEST_DIR").ok().map(|dir| Path::new(&dir).into())); @@ -10,12 +21,17 @@ pub fn cargo_manifest_dir() -> Option { } /// Returns the absolute path from the project root. -pub fn get_absolute_path(relative_path: &str) -> PathBuf { +pub fn get_absolute_path(relative_path: &str) -> Result { let base_dir = cargo_manifest_dir() // Attempt to get the `CARGO_MANIFEST_DIR` environment variable and convert it to `PathBuf`. // Ascend two directories ("../..") to get to the project root. .map(|dir| dir.join("../..")) // If `CARGO_MANIFEST_DIR` isn't set, fall back to the current working directory .unwrap_or(env::current_dir().expect("Failed to get current directory")); - base_dir.join(relative_path) + let path = base_dir.join(relative_path); + if !path.try_exists()? { + return Err(GetPathError::PathDoesNotExist { path }); + } + + Ok(path) } diff --git a/crates/mempool_test_utils/src/starknet_api_test_utils.rs b/crates/mempool_test_utils/src/starknet_api_test_utils.rs index 0f3b8231f50..8c118f077c0 100644 --- a/crates/mempool_test_utils/src/starknet_api_test_utils.rs +++ b/crates/mempool_test_utils/src/starknet_api_test_utils.rs @@ -67,7 +67,8 @@ pub fn test_valid_resource_bounds() -> ValidResourceBounds { /// Get the contract class used for testing. pub fn contract_class() -> ContractClass { - env::set_current_dir(get_absolute_path(TEST_FILES_FOLDER)).expect("Couldn't set working dir."); + env::set_current_dir(get_absolute_path(TEST_FILES_FOLDER).unwrap()) + .expect("Couldn't set working dir."); let json_file_path = Path::new(CONTRACT_CLASS_FILE); serde_json::from_reader(File::open(json_file_path).unwrap()).unwrap() } diff --git a/crates/papyrus_config/Cargo.toml b/crates/papyrus_config/Cargo.toml index 82829b655bf..a8e385ec25c 100644 --- a/crates/papyrus_config/Cargo.toml +++ b/crates/papyrus_config/Cargo.toml @@ -11,6 +11,7 @@ development = ["tempfile"] # Dependency of a doc-test [dependencies] clap = { workspace = true, features = ["env", "string"] } +infra_utils.workspace = true itertools.workspace = true serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["arbitrary_precision"] } diff --git a/crates/papyrus_config/src/config_test.rs b/crates/papyrus_config/src/config_test.rs index 89fa48d2cc2..96b7c3be4a7 100644 --- a/crates/papyrus_config/src/config_test.rs +++ b/crates/papyrus_config/src/config_test.rs @@ -52,7 +52,7 @@ use crate::{ lazy_static! { static ref CUSTOM_CONFIG_PATH: PathBuf = - get_absolute_path("crates/papyrus_config/resources/custom_config_example.json"); + get_absolute_path("crates/papyrus_config/resources/custom_config_example.json").unwrap(); } #[derive(Clone, Copy, Default, Serialize, Deserialize, Debug, PartialEq, Validate)] diff --git a/crates/papyrus_config/src/lib.rs b/crates/papyrus_config/src/lib.rs index 7a383cef548..7dbb8272ff6 100644 --- a/crates/papyrus_config/src/lib.rs +++ b/crates/papyrus_config/src/lib.rs @@ -49,6 +49,7 @@ use clap::parser::MatchesError; use dumping::REQUIRED_PARAM_DESCRIPTION_PREFIX; +use infra_utils::path::GetPathError; use serde::{Deserialize, Serialize}; use serde_json::Value; use validator::ValidationError; @@ -179,6 +180,8 @@ pub enum ConfigError { #[error(transparent)] CommandMatches(#[from] MatchesError), #[error(transparent)] + GetPathError(#[from] GetPathError), + #[error(transparent)] IOError(#[from] std::io::Error), // TODO(Eitan): Improve error message #[error("Insert a new param is not allowed: {param_path}.")] diff --git a/crates/papyrus_node/src/config/config_test.rs b/crates/papyrus_node/src/config/config_test.rs index dfb828b4efd..441a9937426 100644 --- a/crates/papyrus_node/src/config/config_test.rs +++ b/crates/papyrus_node/src/config/config_test.rs @@ -73,14 +73,14 @@ fn get_args(additional_args: Vec<&str>) -> Vec { #[test] fn load_default_config() { - env::set_current_dir(get_absolute_path("")).expect("Couldn't set working dir."); + env::set_current_dir(get_absolute_path("").unwrap()).expect("Couldn't set working dir."); NodeConfig::load_and_process(get_args(vec![])).expect("Failed to load the config."); } #[test] fn load_http_headers() { let args = get_args(vec!["--central.http_headers", "NAME_1:VALUE_1 NAME_2:VALUE_2"]); - env::set_current_dir(get_absolute_path("")).expect("Couldn't set working dir."); + env::set_current_dir(get_absolute_path("").unwrap()).expect("Couldn't set working dir."); let config = NodeConfig::load_and_process(args).unwrap(); let target_http_headers = HashMap::from([ ("NAME_1".to_string(), "VALUE_1".to_string()), @@ -96,7 +96,7 @@ fn load_http_headers() { // Regression test which checks that the default config dumping hasn't changed. fn test_dump_default_config() { let mut default_config = NodeConfig::default(); - env::set_current_dir(get_absolute_path("")).expect("Couldn't set working dir."); + env::set_current_dir(get_absolute_path("").unwrap()).expect("Couldn't set working dir."); let dumped_default_config = default_config.dump(); insta::assert_json_snapshot!(dumped_default_config); @@ -108,7 +108,7 @@ fn test_dump_default_config() { #[test] fn test_default_config_process() { - env::set_current_dir(get_absolute_path("")).expect("Couldn't set working dir."); + env::set_current_dir(get_absolute_path("").unwrap()).expect("Couldn't set working dir."); assert_eq!(NodeConfig::load_and_process(get_args(vec![])).unwrap(), NodeConfig::default()); } @@ -120,7 +120,7 @@ fn test_update_dumped_config_by_command() { "--storage.db_config.path_prefix", "/abc", ]); - env::set_current_dir(get_absolute_path("")).expect("Couldn't set working dir."); + env::set_current_dir(get_absolute_path("").unwrap()).expect("Couldn't set working dir."); let config = NodeConfig::load_and_process(args).unwrap(); assert_eq!(config.central.retry_config.retry_max_delay_millis, 1234); @@ -130,7 +130,7 @@ fn test_update_dumped_config_by_command() { #[cfg(feature = "rpc")] #[test] fn default_config_file_is_up_to_date() { - env::set_current_dir(get_absolute_path("")).expect("Couldn't set working dir."); + env::set_current_dir(get_absolute_path("").unwrap()).expect("Couldn't set working dir."); let from_default_config_file: serde_json::Value = serde_json::from_reader(File::open(DEFAULT_CONFIG_PATH).unwrap()).unwrap(); diff --git a/crates/starknet_sequencer_node/src/config/config_test.rs b/crates/starknet_sequencer_node/src/config/config_test.rs index d8396812e0d..6b303ecc951 100644 --- a/crates/starknet_sequencer_node/src/config/config_test.rs +++ b/crates/starknet_sequencer_node/src/config/config_test.rs @@ -63,7 +63,7 @@ fn test_valid_component_execution_config( /// cargo run --bin sequencer_dump_config -q #[test] fn test_default_config_file_is_up_to_date() { - env::set_current_dir(get_absolute_path("")).expect("Couldn't set working dir."); + env::set_current_dir(get_absolute_path("").unwrap()).expect("Couldn't set working dir."); let from_default_config_file: serde_json::Value = serde_json::from_reader(File::open(DEFAULT_CONFIG_PATH).unwrap()).unwrap(); @@ -115,7 +115,7 @@ fn test_config_parsing() { #[test] fn test_required_params_setting() { // Load the default config file. - let file = std::fs::File::open(get_absolute_path(DEFAULT_CONFIG_PATH)).unwrap(); + let file = std::fs::File::open(get_absolute_path(DEFAULT_CONFIG_PATH).unwrap()).unwrap(); let mut deserialized = serde_json::from_reader::<_, serde_json::Value>(file).unwrap(); let expected_required_params = deserialized.as_object_mut().unwrap(); expected_required_params.retain(|_, value| { diff --git a/crates/starknet_sequencer_node/src/config/node_config.rs b/crates/starknet_sequencer_node/src/config/node_config.rs index 3b25b4c2d98..d475f1eebd2 100644 --- a/crates/starknet_sequencer_node/src/config/node_config.rs +++ b/crates/starknet_sequencer_node/src/config/node_config.rs @@ -157,7 +157,7 @@ impl SequencerNodeConfig { ) -> Result { let config_file_name = match config_file_name { Some(file_name) => Path::new(file_name), - None => &get_absolute_path(DEFAULT_CONFIG_PATH), + None => &get_absolute_path(DEFAULT_CONFIG_PATH)?, }; let default_config_file = File::open(config_file_name)?; diff --git a/crates/starknet_sierra_compile/src/compile_test.rs b/crates/starknet_sierra_compile/src/compile_test.rs index 4c2b31c6542..63351d6f1a6 100644 --- a/crates/starknet_sierra_compile/src/compile_test.rs +++ b/crates/starknet_sierra_compile/src/compile_test.rs @@ -26,7 +26,8 @@ fn command_line_compiler() -> CommandLineCompiler { CommandLineCompiler::new(SIERRA_TO_CASM_COMPILATION_CONFIG) } fn get_test_contract() -> ContractClass { - env::set_current_dir(get_absolute_path(TEST_FILES_FOLDER)).expect("Failed to set current dir."); + env::set_current_dir(get_absolute_path(TEST_FILES_FOLDER).unwrap()) + .expect("Failed to set current dir."); let sierra_path = Path::new(FAULTY_ACCOUNT_CLASS_FILE); contract_class_from_file(sierra_path) }