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

local vs global chain_type (for better test isolation) #412

Merged
merged 9 commits into from
May 23, 2020
Merged
Show file tree
Hide file tree
Changes from all 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: 8 additions & 8 deletions Cargo.lock

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

12 changes: 11 additions & 1 deletion api/src/foreign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ where
/// ```
/// use grin_wallet_util::grin_keychain as keychain;
/// use grin_wallet_util::grin_util as util;
/// use grin_wallet_util::grin_core;
/// use grin_wallet_api as api;
/// use grin_wallet_config as config;
/// use grin_wallet_impls as impls;
Expand All @@ -113,11 +114,15 @@ where
/// use std::sync::Arc;
/// use util::{Mutex, ZeroingString};
///
/// use grin_core::global;
///
/// use api::Foreign;
/// use config::WalletConfig;
/// use impls::{DefaultWalletImpl, DefaultLCProvider, HTTPNodeClient};
/// use libwallet::WalletInst;
///
/// global::set_local_chain_type(global::ChainTypes::AutomatedTesting);
///
/// let mut wallet_config = WalletConfig::default();
/// # let dir = tempdir().map_err(|e| format!("{:#?}", e)).unwrap();
/// # let dir = dir
Expand Down Expand Up @@ -406,9 +411,11 @@ macro_rules! doctest_helper_setup_doc_env_foreign {
use grin_wallet_config as config;
use grin_wallet_impls as impls;
use grin_wallet_libwallet as libwallet;
use grin_wallet_util::grin_core;
use grin_wallet_util::grin_keychain as keychain;
use grin_wallet_util::grin_util as util;

use grin_core::global;
use keychain::ExtKeychain;
use tempfile::tempdir;

Expand All @@ -423,7 +430,10 @@ macro_rules! doctest_helper_setup_doc_env_foreign {
// don't run on windows CI, which gives very inconsistent results
if cfg!(windows) {
return;
}
}

// Set our local chain_type for testing.
global::set_local_chain_type(global::ChainTypes::AutomatedTesting);

let dir = tempdir().map_err(|e| format!("{:#?}", e)).unwrap();
let dir = dir
Expand Down
2 changes: 1 addition & 1 deletion api/src/foreign_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ pub fn run_doctest_foreign(

util::init_test_logger();
let _ = fs::remove_dir_all(test_dir);
global::set_mining_mode(ChainTypes::AutomatedTesting);
global::set_local_chain_type(ChainTypes::AutomatedTesting);

let mut wallet_proxy: WalletProxy<
DefaultLCProvider<LocalWalletClient, ExtKeychain>,
Expand Down
11 changes: 10 additions & 1 deletion api/src/owner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,13 @@ where
/// ```
/// use grin_wallet_util::grin_keychain as keychain;
/// use grin_wallet_util::grin_util as util;
/// use grin_wallet_util::grin_core;
/// use grin_wallet_api as api;
/// use grin_wallet_config as config;
/// use grin_wallet_impls as impls;
/// use grin_wallet_libwallet as libwallet;
///
/// use grin_core::global;
/// use keychain::ExtKeychain;
/// use tempfile::tempdir;
///
Expand All @@ -125,6 +127,8 @@ where
/// use impls::{DefaultWalletImpl, DefaultLCProvider, HTTPNodeClient};
/// use libwallet::WalletInst;
///
/// global::set_local_chain_type(global::ChainTypes::AutomatedTesting);
///
/// let mut wallet_config = WalletConfig::default();
/// # let dir = tempdir().map_err(|e| format!("{:#?}", e)).unwrap();
/// # let dir = dir
Expand Down Expand Up @@ -2113,6 +2117,8 @@ macro_rules! doctest_helper_setup_doc_env {
use grin_wallet_util::grin_keychain as keychain;
use grin_wallet_util::grin_util as util;

use grin_core::global;

use keychain::ExtKeychain;
use tempfile::tempdir;

Expand All @@ -2129,7 +2135,10 @@ macro_rules! doctest_helper_setup_doc_env {
// don't run on windows CI, which gives very inconsistent results
if cfg!(windows) {
return;
}
}

// Set our local chain_type for testing.
global::set_local_chain_type(global::ChainTypes::AutomatedTesting);

let dir = tempdir().map_err(|e| format!("{:#?}", e)).unwrap();
let dir = dir
Expand Down
2 changes: 1 addition & 1 deletion api/src/owner_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1937,7 +1937,7 @@ pub fn run_doctest_owner(

util::init_test_logger();
let _ = fs::remove_dir_all(test_dir);
global::set_mining_mode(ChainTypes::AutomatedTesting);
global::set_local_chain_type(ChainTypes::AutomatedTesting);

let mut wallet_proxy: WalletProxy<
DefaultLCProvider<LocalWalletClient, ExtKeychain>,
Expand Down
14 changes: 5 additions & 9 deletions controller/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ pub struct GlobalArgs {
pub api_secret: Option<String>,
pub node_api_secret: Option<String>,
pub show_spent: bool,
pub chain_type: global::ChainTypes,
pub password: Option<ZeroingString>,
pub tls_conf: Option<TLSConfig>,
}
Expand All @@ -71,23 +70,20 @@ pub struct InitArgs {

pub fn init<L, C, K>(
owner_api: &mut Owner<L, C, K>,
g_args: &GlobalArgs,
_g_args: &GlobalArgs,
args: InitArgs,
) -> Result<(), Error>
where
L: WalletLCProvider<'static, C, K> + 'static,
C: NodeClient + 'static,
K: keychain::Keychain + 'static,
{
// Assume global chain type has already been initialized.
let chain_type = global::get_chain_type();

let mut w_lock = owner_api.wallet_inst.lock();
let p = w_lock.lc_provider()?;
p.create_config(
&g_args.chain_type,
WALLET_CONFIG_FILE_NAME,
None,
None,
None,
)?;
p.create_config(&chain_type, WALLET_CONFIG_FILE_NAME, None, None, None)?;
p.create_wallet(
None,
args.recovery_phrase,
Expand Down
10 changes: 9 additions & 1 deletion controller/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,15 @@ pub fn clean_output_dir(test_dir: &str) {
pub fn setup(test_dir: &str) {
util::init_test_logger();
clean_output_dir(test_dir);
global::set_mining_mode(ChainTypes::AutomatedTesting);
global::set_local_chain_type(ChainTypes::AutomatedTesting);
}

/// Some tests require the global chain_type to be configured due to threads being spawned internally.
/// It is recommended to avoid relying on this if at all possible as global chain_type
/// leaks across multiple tests and will likely have unintended consequences.
#[allow(dead_code)]
pub fn setup_global_chain_type() {
global::init_global_chain_type(global::ChainTypes::AutomatedTesting);
}

pub fn create_wallet_proxy(
Expand Down
6 changes: 5 additions & 1 deletion controller/tests/updater_thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use std::time::Duration;

#[macro_use]
mod common;
use common::{clean_output_dir, create_wallet_proxy, setup};
use common::{clean_output_dir, create_wallet_proxy, setup, setup_global_chain_type};

/// updater thread test impl
fn updater_thread_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
Expand Down Expand Up @@ -110,6 +110,10 @@ fn updater_thread_test_impl(test_dir: &'static str) -> Result<(), libwallet::Err

#[test]
fn updater_thread() {
// The "updater" kicks off a new thread so we need to ensure the global chain_type
// is set for this to work correctly.
setup_global_chain_type();

let test_dir = "test_output/updater_thread";
setup(test_dir);
if let Err(e) = updater_thread_test_impl(test_dir) {
Expand Down
2 changes: 1 addition & 1 deletion impls/src/lifecycle/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ where
logging_config: Option<LoggingConfig>,
tor_config: Option<TorConfig>,
) -> Result<(), Error> {
let mut default_config = GlobalWalletConfig::for_chain(chain_type);
let mut default_config = GlobalWalletConfig::for_chain(&chain_type);
let logging = match logging_config {
Some(l) => Some(l),
None => match default_config.members.as_ref() {
Expand Down
8 changes: 6 additions & 2 deletions impls/src/test_framework/testclient.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::chain::types::NoopAdapter;
use crate::chain::Chain;
use crate::core::core::verifier_cache::LruVerifierCache;
use crate::core::core::{Transaction, TxKernel};
use crate::core::global::{set_mining_mode, ChainTypes};
use crate::core::global::{set_local_chain_type, ChainTypes};
use crate::core::pow;
use crate::keychain::Keychain;
use crate::libwallet;
Expand Down Expand Up @@ -93,7 +93,7 @@ where
{
/// Create a new client that will communicate with the given grin node
pub fn new(chain_dir: &str) -> Self {
set_mining_mode(ChainTypes::AutomatedTesting);
set_local_chain_type(ChainTypes::AutomatedTesting);
let genesis_block = pow::mine_genesis_block().unwrap();
let verifier_cache = Arc::new(RwLock::new(LruVerifierCache::new()));
let dir_name = format!("{}/.grin", chain_dir);
Expand Down Expand Up @@ -136,6 +136,10 @@ where
/// Run the incoming message queue and respond more or less
/// synchronously
pub fn run(&mut self) -> Result<(), libwallet::Error> {
// We run the wallet_proxy within a spawned thread in tests.
// We set the local chain_type here within the thread.
set_local_chain_type(ChainTypes::AutomatedTesting);

self.running.store(true, Ordering::Relaxed);
loop {
thread::sleep(Duration::from_millis(10));
Expand Down
4 changes: 2 additions & 2 deletions integration/tests/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ fn simple_server_wallet() {
init_test_logger();
info!("starting simple_server_wallet");
let _test_name_dir = "test_servers";
core::global::set_mining_mode(core::global::ChainTypes::AutomatedTesting);
core::global::set_local_chain_type(core::global::ChainTypes::AutomatedTesting);

// Run a separate coinbase wallet for coinbase transactions
let coinbase_dir = "coinbase_wallet_api";
Expand Down Expand Up @@ -142,7 +142,7 @@ fn simple_server_wallet() {
fn test_p2p() {
init_test_logger();
info!("starting test_p2p");
global::set_mining_mode(ChainTypes::AutomatedTesting);
global::set_local_chain_type(ChainTypes::AutomatedTesting);

let _test_name_dir = "test_servers";

Expand Down
2 changes: 1 addition & 1 deletion integration/tests/dandelion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use std::{thread, time};
#[ignore]
fn test_dandelion_timeout() {
let test_name_dir = "test_dandelion_timeout";
core::global::set_mining_mode(core::global::ChainTypes::AutomatedTesting);
core::global::set_local_chain_type(core::global::ChainTypes::AutomatedTesting);
framework::clean_all_output(test_name_dir);
let mut log_config = util::LoggingConfig::default();
//log_config.stdout_log_level = util::LogLevel::Trace;
Expand Down
16 changes: 8 additions & 8 deletions integration/tests/simulnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ use crate::framework::{
#[test]
fn basic_genesis_mine() {
util::init_test_logger();
global::set_mining_mode(ChainTypes::AutomatedTesting);
global::set_local_chain_type(ChainTypes::AutomatedTesting);

let test_name_dir = "genesis_mine";
framework::clean_all_output(test_name_dir);
Expand Down Expand Up @@ -84,7 +84,7 @@ fn basic_genesis_mine() {
#[test]
fn simulate_seeding() {
util::init_test_logger();
global::set_mining_mode(ChainTypes::AutomatedTesting);
global::set_local_chain_type(ChainTypes::AutomatedTesting);

let test_name_dir = "simulate_seeding";
framework::clean_all_output(test_name_dir);
Expand Down Expand Up @@ -151,7 +151,7 @@ fn simulate_seeding() {
#[ignore]
#[test]
fn simulate_parallel_mining() {
global::set_mining_mode(ChainTypes::AutomatedTesting);
global::set_local_chain_type(ChainTypes::AutomatedTesting);

let test_name_dir = "simulate_parallel_mining";
// framework::clean_all_output(test_name_dir);
Expand Down Expand Up @@ -208,7 +208,7 @@ fn simulate_block_propagation() {

// we actually set the chain_type in the ServerConfig below
// TODO - avoid needing to set it in two places?
global::set_mining_mode(ChainTypes::AutomatedTesting);
global::set_local_chain_type(ChainTypes::AutomatedTesting);

let test_name_dir = "grin-prop";
framework::clean_all_output(test_name_dir);
Expand Down Expand Up @@ -268,7 +268,7 @@ fn simulate_full_sync() {
util::init_test_logger();

// we actually set the chain_type in the ServerConfig below
global::set_mining_mode(ChainTypes::AutomatedTesting);
global::set_local_chain_type(ChainTypes::AutomatedTesting);

let test_name_dir = "grin-sync";
framework::clean_all_output(test_name_dir);
Expand Down Expand Up @@ -324,7 +324,7 @@ fn simulate_fast_sync() {
util::init_test_logger();

// we actually set the chain_type in the ServerConfig below
global::set_mining_mode(ChainTypes::AutomatedTesting);
global::set_local_chain_type(ChainTypes::AutomatedTesting);

let test_name_dir = "grin-fast";
framework::clean_all_output(test_name_dir);
Expand Down Expand Up @@ -417,7 +417,7 @@ fn simulate_long_fork() {
println!("starting simulate_long_fork");

// we actually set the chain_type in the ServerConfig below
global::set_mining_mode(ChainTypes::AutomatedTesting);
global::set_local_chain_type(ChainTypes::AutomatedTesting);

let test_name_dir = "grin-long-fork";
framework::clean_all_output(test_name_dir);
Expand Down Expand Up @@ -903,7 +903,7 @@ pub fn create_wallet(
#[test]
fn replicate_tx_fluff_failure() {
util::init_test_logger();
global::set_mining_mode(ChainTypes::UserTesting);
global::set_local_chain_type(ChainTypes::UserTesting);
framework::clean_all_output("tx_fluff");

// Create Wallet 1 (Mining Input) and start it listening
Expand Down
Loading