From 34e487b99b2d5f74f2425325d9ae19933420479c Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 27 Jun 2022 00:28:18 +0200 Subject: [PATCH 1/4] Warn the user when Zebra cannot parse `zebrad.toml` --- zebrad/src/application.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index 494367216f0..5eb20ac4202 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -5,6 +5,7 @@ use std::{fmt::Write as _, io::Write as _, process}; use abscissa_core::{ application::{self, fatal_error, AppCell}, config::{self, Configurable}, + status_err, terminal::{component::Terminal, stderr, stdout, ColorChoice}, Application, Component, EntryPoint, FrameworkError, Shutdown, StandardPaths, Version, }; @@ -202,11 +203,16 @@ impl Application for ZebradApp { // Load config *after* framework components so that we can // report an error to the terminal if it occurs. - let config = command - .config_path() - .map(|path| self.load_config(&path)) - .transpose()? - .unwrap_or_default(); + let config = match command.config_path() { + Some(path) => match self.load_config(&path) { + Ok(config) => config, + Err(e) => { + status_err!("Zebra could not parse the provided config file. This might mean you are using a deprecated format of the file."); + return Err(e); + } + }, + None => ZebradConfig::default(), + }; let config = command.process_config(config)?; From c93bd89089d8980e37022a916b585a38b6666750 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 27 Jun 2022 00:31:50 +0200 Subject: [PATCH 2/4] Test that Zebra warns the user when it cannot parse `zebrad.toml` --- zebrad/tests/acceptance.rs | 80 +++++++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 6 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index f969febd87e..83b389868a6 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -103,10 +103,10 @@ //! //! Please refer to the documentation of each test for more information. -use std::{collections::HashSet, env, path::PathBuf}; +use std::{collections::HashSet, env, fs, path::PathBuf, time::Duration}; use color_eyre::{ - eyre::{Result, WrapErr}, + eyre::{eyre, Result, WrapErr}, Help, }; @@ -355,7 +355,6 @@ fn misconfigured_ephemeral_missing_directory() -> Result<()> { } fn ephemeral(cache_dir_config: EphemeralConfig, cache_dir_check: EphemeralCheck) -> Result<()> { - use std::fs; use std::io::ErrorKind; zebra_test::init(); @@ -503,12 +502,14 @@ fn version_args() -> Result<()> { } #[test] -fn valid_generated_config_test() -> Result<()> { +fn config_test() -> Result<()> { // Unlike the other tests, these tests can not be run in parallel, because // they use the generated config. So parallel execution can cause port and // cache conflicts. valid_generated_config("start", "Starting zebrad")?; + invalid_generated_config()?; + Ok(()) } @@ -561,6 +562,75 @@ fn valid_generated_config(command: &str, expect_stdout_line_contains: &str) -> R Ok(()) } +/// Checks that Zebra prints an informative message when it cannot parse the +/// config file. +fn invalid_generated_config() -> Result<()> { + zebra_test::init(); + + let testdir = &testdir()?; + + // Add a config file name to tempdir path. + let config_path = testdir.path().join("zebrad.toml"); + + // Generate a valid config file in the temp dir. + let child = testdir.spawn_child(args!["generate", "-o": config_path.to_str().unwrap()])?; + + let output = child.wait_with_output()?; + let output = output.assert_success()?; + + assert_with_context!( + config_path.exists(), + &output, + "generated config file not found" + ); + + // Load the valid config file that Zebra generated. + let mut config_file = fs::read_to_string(config_path.to_str().unwrap()).unwrap(); + + // Let's now alter the config file so that it contains a deprecated format + // of `mempool.eviction_memory_time`. + + config_file = config_file + .lines() + // Remove the valid `eviction_memory_time` key/value pair from the + // config. + .filter(|line| !line.contains("eviction_memory_time")) + .map(|line| line.to_owned() + "\n") + .collect(); + + // Append the `eviction_memory_time` key/value pair in a deprecated format. + config_file += r" + + [mempool.eviction_memory_time] + nanos = 0 + secs = 3600 + "; + + // Write the altered config file so that Zebra can pick it up. + fs::write(config_path.to_str().unwrap(), config_file.as_bytes()) + .expect("Could not write the altered config file."); + + // Run Zebra in a temp dir so that it loads the config. + let mut child = testdir.spawn_child(args!["start"])?; + + // Return an error if Zebra is running for more than two seconds. + // + // Since the config is invalid, Zebra should terminate instantly after its + // start. Two seconds should be sufficient for Zebra to read the config file + // and terminate. + std::thread::sleep(Duration::from_secs(2)); + if child.is_running() { + child.kill()?; + return Err(eyre!("Zebra should not be running anymore.")); + } + + let output = child.wait_with_output()?; + + // Check that Zebra produced an informative message. + output.stderr_contains("Zebra could not parse the provided config file. This might mean you are using a deprecated format of the file.")?; + + Ok(()) +} /// Test if `zebrad` can sync the first checkpoint on mainnet. /// /// The first checkpoint contains a single genesis block. @@ -1558,8 +1628,6 @@ where // See #1781. #[cfg(target_os = "linux")] if node2.is_running() { - use color_eyre::eyre::eyre; - return node2 .kill_on_error::<(), _>(Err(eyre!( "conflicted node2 was still running, but the test expected a panic" From 6591a8a177cdb5bf46d7ec31e673064e66ee6e87 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 27 Jun 2022 10:49:28 +1000 Subject: [PATCH 3/4] Fix up a mistaken merge change --- zebrad/tests/acceptance.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index f90aa281637..c955365d8c5 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -501,19 +501,25 @@ fn version_args() -> Result<()> { Ok(()) } +/// Run config tests that use the default ports and paths. +/// +/// Unlike the other tests, these tests can not be run in parallel, because +/// they use the generated config. So parallel execution can cause port and +/// cache conflicts. #[test] fn config_test() -> Result<()> { - // Unlike the other tests, these tests can not be run in parallel, because - // they use the generated config. So parallel execution can cause port and - // cache conflicts. valid_generated_config("start", "Starting zebrad")?; + // Check what happens when Zebra parses an invalid config + invalid_generated_config()?; + // Check that an older stored configuration we have for Zebra works stored_config_works()?; Ok(()) } +/// Test that `zebrad start` can parse the output from `zebrad generate`. fn valid_generated_config(command: &str, expect_stdout_line_contains: &str) -> Result<()> { zebra_test::init(); @@ -565,7 +571,6 @@ fn valid_generated_config(command: &str, expect_stdout_line_contains: &str) -> R /// Checks that Zebra prints an informative message when it cannot parse the /// config file. -#[test] fn invalid_generated_config() -> Result<()> { zebra_test::init(); From b30b2addd02b4b78dad35e931dc35da76ac5af8c Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 27 Jun 2022 18:38:16 +0200 Subject: [PATCH 4/4] Suggest how to fix `zebrad.toml` when Zebra cannot parse it --- zebrad/src/application.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index 5eb20ac4202..d2a1380693e 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -207,7 +207,7 @@ impl Application for ZebradApp { Some(path) => match self.load_config(&path) { Ok(config) => config, Err(e) => { - status_err!("Zebra could not parse the provided config file. This might mean you are using a deprecated format of the file."); + status_err!("Zebra could not parse the provided config file. This might mean you are using a deprecated format of the file. You can generate a valid config by running \"zebrad generate\", and diff it against yours to examine any format inconsistencies."); return Err(e); } },