Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn the user when Zebra doesn't recognize the format of zebrad.toml #4689

Merged
merged 6 commits into from
Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
16 changes: 11 additions & 5 deletions zebrad/src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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(),
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
};

let config = command.process_config(config)?;

Expand Down
94 changes: 84 additions & 10 deletions zebrad/tests/acceptance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -502,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 valid_generated_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.
fn config_test() -> Result<()> {
valid_generated_config("start", "Starting zebrad")?;

// Check that the stored configuration we have for Zebra works
// 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();

Expand Down Expand Up @@ -564,6 +569,77 @@ 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 that an older `zebrad.toml` can still be parsed by the latest `zebrad`.
fn stored_config_works() -> Result<()> {
let stored_config_path = stored_config_path();
let run_dir = testdir()?;
Expand Down Expand Up @@ -1586,8 +1662,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"
Expand Down