Skip to content

Commit

Permalink
chore(infra): add error variant for file not found on get path
Browse files Browse the repository at this point in the history
  • Loading branch information
ArniStarkware committed Nov 19, 2024
1 parent cb4bbcb commit 48c61ac
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 14 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/infra_utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ description = "Infrastructure utility functions for the sequencer node."

[lints]
workspace = true

[dependencies]
thiserror.workspace = true
20 changes: 18 additions & 2 deletions crates/infra_utils/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,32 @@ 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),
}

pub static PATH_TO_CARGO_MANIFEST_DIR: LazyLock<Option<PathBuf>> =
LazyLock::new(|| env::var("CARGO_MANIFEST_DIR").ok().map(|dir| Path::new(&dir).into()));

/// 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<PathBuf, GetPathError> {
let base_dir = PATH_TO_CARGO_MANIFEST_DIR.clone()
// 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_buf = base_dir.join(relative_path);
if !path_buf.try_exists()? {
return Err(GetPathError::PathDoesNotExist { path: path_buf });
}

Ok(path_buf)
}
3 changes: 2 additions & 1 deletion crates/mempool_test_utils/src/starknet_api_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
1 change: 1 addition & 0 deletions crates/papyrus_config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
2 changes: 1 addition & 1 deletion crates/papyrus_config/src/config_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
3 changes: 3 additions & 0 deletions crates/papyrus_config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}.")]
Expand Down
12 changes: 6 additions & 6 deletions crates/papyrus_node/src/config/config_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,14 @@ fn get_args(additional_args: Vec<&str>) -> Vec<String> {

#[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()),
Expand All @@ -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);

Expand All @@ -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());
}

Expand All @@ -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);
Expand All @@ -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();

Expand Down
4 changes: 2 additions & 2 deletions crates/starknet_sequencer_node/src/config/config_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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| {
Expand Down
2 changes: 1 addition & 1 deletion crates/starknet_sequencer_node/src/config/node_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl SequencerNodeConfig {
) -> Result<Self, ConfigError> {
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)?;
Expand Down
3 changes: 2 additions & 1 deletion crates/starknet_sierra_compile/src/compile_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 48c61ac

Please sign in to comment.