Skip to content

Commit

Permalink
Add a regression test for empty config files
Browse files Browse the repository at this point in the history
See the previous commit for a reason why!

Also, no longer create an empty config file by default. Initially I
thought this was a good idea so that if users wanted to customize the
file the name would be obvious (since the file existed). However, the
name isn't obvious because you need to know the directory anyways (e.g.
I _just_ had to look this up on my Windows machine). So it feels better
to not make empty files for no real reason. There are probably other
ways of self-describing config (e.g. render the location in --help).
  • Loading branch information
marcbowes committed Apr 17, 2021
1 parent e3cb025 commit 12570aa
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 11 deletions.
64 changes: 59 additions & 5 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 Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ pest = "2.1.3"
pest_derive = "2.1.0"
tracing = { version = "0.1.25", features = ["log"] }

[dev-dependencies]
tempdir = "0.3"

[[bin]]
name = "qldb"
path = "src/main.rs"
28 changes: 22 additions & 6 deletions src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ use pest::Parser;
use pest_derive::Parser;
use serde::{Deserialize, Serialize};
use std::path::Path;
use std::{
collections::HashMap,
fs::{self, File},
};
use std::{collections::HashMap, fs};
use thiserror::Error;
use toml;

Expand Down Expand Up @@ -158,9 +155,10 @@ impl Config {
fs::create_dir_all(&shell_dir)?;
let config_file = shell_dir.join("default_config.toml");
if !config_file.exists() {
File::create(&config_file)?;
Ok(Config::default())
} else {
Config::load(&config_file)
}
Config::load(&config_file)
}
}

Expand Down Expand Up @@ -284,3 +282,21 @@ impl FromStr for ExecuteStatementOpt {
})
}
}

#[cfg(test)]
mod settings_tests {
use super::*;
use fs::File;
use tempdir::TempDir;

/// Tests that an empty config is valid. This makes sure we don't forget an
/// `Optional` in any new fields we add.
#[test]
fn load_empty_config() -> Result<()> {
let tmp = TempDir::new("config")?;
let path = tmp.path().join("empty.toml");
File::create(&path)?;
let _ = Config::load(&path)?;
Ok(())
}
}

0 comments on commit 12570aa

Please sign in to comment.