Add JSON format to import blocks and set it as default (#5816)

* Add BlockStream Enum and utility fn

* WIP: Modify import closure to work with BlockStream

* Fix trait bounds

* Working prototype

* Revamp block importing

* Add export_import_flow tests

* Add comments and clean code

* Add more comments in the import fn

* Add link code to import function

* Add condition when returning Ready(Ok(()) to make sure we've imported every block

* Add check for imported blocks in JSON case

* Use rest pattern

* Fix compilation error for undeclared variable

* Add polling and waker before pending

* Print read_block_count instead of count

* Simplify binary cli option with structopt

* Update test to reflect changes in CLI api

* Change Stream to take SignedBlock<B> instead of B

* Add comments to BlockStream

* Move out logic to smaller functions for clearer code

* Remove result over import_blocks return type

* Check for error in command output rather than simply checking command exit status

* Revamp export/import/revert testing

* Fix minor typos and formatting errors

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Remove unnecessary if condition in terminating condition

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Explicit error instead of returning it as a string

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Pass BlockStream to log_importing_status_updates and simplify matching arms for block stream

* Use .contains() instead of regex match

* Line break in match block; return future::ready instead of poll_fn

* Update Cargo.lock

* Add check so that queue doesn't grow too big

* Use Iterator instead of Stream

* Remove allow dead_code

* Remove outdated comments

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Return Errors instead of logging them

* Simplify match arms

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Remove check before terminating block import

* Apply suggestions from code review

* Check that queue is not full BEFORE calling

* Revert "Remove check before terminating block import"

This reverts commit 377823c0a648a3eb2e61185a257a61023067893d.

* Improve unit tests to make sure we actually import blocks

* Remove Unpin implementation for BlockIter

* Add prototype of enum for ImportStates

* Add working prototype for StateMachine

* Add comments for clearer code

* Add sleep before calling Waker when waiting for import queue

* Add Speedometer

* add dbg!(&log) for test debugging

* Fix lines with more than 100 cols

* Fix regex capture for test

* Update regexes to take to capture the whole number

* Rename Cmd to Command

Co-authored-by: Gavin Wood <gavin@parity.io>

* Actually rename Cmd to Command

* Apply suggestions from code review

Co-authored-by: Gavin Wood <gavin@parity.io>

* Fix compilation errors for tests

* Fix compilation errors from code review suggestion

* Update bin/node/cli/tests/export_import_flow.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Gavin Wood <gavin@parity.io>
Co-authored-by: Benjamin Kampmann <ben@gnunicorn.org>
This commit is contained in:
pscott
2020-05-22 13:50:25 +02:00
committed by GitHub
parent 66fe846d48
commit 18d4fa10d2
10 changed files with 546 additions and 142 deletions
@@ -22,7 +22,7 @@ use assert_cmd::cargo::cargo_bin;
use std::process::Command;
use tempfile::tempdir;
mod common;
pub mod common;
#[test]
fn check_block_works() {
-1
View File
@@ -17,7 +17,6 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.
#![cfg(unix)]
#![allow(dead_code)]
use std::{process::{Child, ExitStatus}, thread, time::Duration, path::Path};
use assert_cmd::cargo::cargo_bin;
@@ -0,0 +1,212 @@
// This file is part of Substrate.
// Copyright (C) 2020 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
#![cfg(unix)]
use assert_cmd::cargo::cargo_bin;
use std::{process::Command, fs, path::PathBuf};
use tempfile::{tempdir, TempDir};
use regex::Regex;
pub mod common;
fn contains_error(logged_output: &str) -> bool {
logged_output.contains("Error")
}
/// Helper struct to execute the export/import/revert tests.
/// The fields are paths to a temporary directory
struct ExportImportRevertExecutor<'a> {
base_path: &'a TempDir,
exported_blocks_file: &'a PathBuf,
db_path: &'a PathBuf,
num_exported_blocks: Option<u64>,
}
/// Format options for export / import commands.
enum FormatOpt {
Json,
Binary,
}
/// Command corresponding to the different commands we would like to run.
enum SubCommand {
ExportBlocks,
ImportBlocks,
}
impl ToString for SubCommand {
fn to_string(&self) -> String {
match self {
SubCommand::ExportBlocks => String::from("export-blocks"),
SubCommand::ImportBlocks => String::from("import-blocks"),
}
}
}
impl<'a> ExportImportRevertExecutor<'a> {
fn new(
base_path: &'a TempDir,
exported_blocks_file: &'a PathBuf,
db_path: &'a PathBuf
) -> Self {
Self {
base_path,
exported_blocks_file,
db_path,
num_exported_blocks: None,
}
}
/// Helper method to run a command. Returns a string corresponding to what has been logged.
fn run_block_command(&self,
sub_command: SubCommand,
format_opt: FormatOpt,
expected_to_fail: bool
) -> String {
let sub_command_str = sub_command.to_string();
// Adding "--binary" if need be.
let arguments: Vec<&str> = match format_opt {
FormatOpt::Binary => vec![&sub_command_str, "--dev", "--pruning", "archive", "--binary", "-d"],
FormatOpt::Json => vec![&sub_command_str, "--dev", "--pruning", "archive", "-d"],
};
let tmp: TempDir;
// Setting base_path to be a temporary folder if we are importing blocks.
// This allows us to make sure we are importing from scratch.
let base_path = match sub_command {
SubCommand::ExportBlocks => &self.base_path.path(),
SubCommand::ImportBlocks => {
tmp = tempdir().unwrap();
tmp.path()
}
};
// Running the command and capturing the output.
let output = Command::new(cargo_bin("substrate"))
.args(&arguments)
.arg(&base_path)
.arg(&self.exported_blocks_file)
.output()
.unwrap();
let logged_output = String::from_utf8_lossy(&output.stderr).to_string();
if expected_to_fail {
// Checking that we did indeed find an error.
assert!(contains_error(&logged_output), "expected to error but did not error!");
assert!(!output.status.success());
} else {
// Making sure no error were logged.
assert!(!contains_error(&logged_output), "expected not to error but error'd!");
assert!(output.status.success());
}
logged_output
}
/// Runs the `export-blocks` command.
fn run_export(&mut self, fmt_opt: FormatOpt) {
let log = self.run_block_command(SubCommand::ExportBlocks, fmt_opt, false);
// Using regex to find out how many block we exported.
let re = Regex::new(r"Exporting blocks from #\d* to #(?P<exported_blocks>\d*)").unwrap();
let caps = re.captures(&log).unwrap();
// Saving the number of blocks we've exported for further use.
self.num_exported_blocks = Some(caps["exported_blocks"].parse::<u64>().unwrap());
let metadata = fs::metadata(&self.exported_blocks_file).unwrap();
assert!(metadata.len() > 0, "file exported_blocks should not be empty");
let _ = fs::remove_dir_all(&self.db_path);
}
/// Runs the `import-blocks` command, asserting that an error was found or
/// not depending on `expected_to_fail`.
fn run_import(&mut self, fmt_opt: FormatOpt, expected_to_fail: bool) {
let log = self.run_block_command(SubCommand::ImportBlocks, fmt_opt, expected_to_fail);
if !expected_to_fail {
// Using regex to find out how much block we imported,
// and what's the best current block.
let re = Regex::new(r"Imported (?P<imported>\d*) blocks. Best: #(?P<best>\d*)").unwrap();
let caps = re.captures(&log).expect("capture should have succeeded");
let imported = caps["imported"].parse::<u64>().unwrap();
let best = caps["best"].parse::<u64>().unwrap();
assert_eq!(
imported,
best,
"numbers of blocks imported and best number differs"
);
assert_eq!(
best,
self.num_exported_blocks.expect("number of exported blocks cannot be None; qed"),
"best block number and number of expected blocks should not differ"
);
}
self.num_exported_blocks = None;
}
/// Runs the `revert` command.
fn run_revert(&self) {
let output = Command::new(cargo_bin("substrate"))
.args(&["revert", "--dev", "--pruning", "archive", "-d"])
.arg(&self.base_path.path())
.output()
.unwrap();
let logged_output = String::from_utf8_lossy(&output.stderr).to_string();
// Reverting should not log any error.
assert!(!contains_error(&logged_output));
// Command should never fail.
assert!(output.status.success());
}
/// Helper function that runs the whole export / import / revert flow and checks for errors.
fn run(&mut self, export_fmt: FormatOpt, import_fmt: FormatOpt, expected_to_fail: bool) {
self.run_export(export_fmt);
self.run_import(import_fmt, expected_to_fail);
self.run_revert();
}
}
#[test]
fn export_import_revert() {
let base_path = tempdir().expect("could not create a temp dir");
let exported_blocks_file = base_path.path().join("exported_blocks");
let db_path = base_path.path().join("db");
common::run_dev_node_for_a_while(base_path.path());
let mut executor = ExportImportRevertExecutor::new(
&base_path,
&exported_blocks_file,
&db_path,
);
// Binary and binary should work.
executor.run(FormatOpt::Binary, FormatOpt::Binary, false);
// Binary and JSON should fail.
executor.run(FormatOpt::Binary, FormatOpt::Json, true);
// JSON and JSON should work.
executor.run(FormatOpt::Json, FormatOpt::Json, false);
// JSON and binary should fail.
executor.run(FormatOpt::Json, FormatOpt::Binary, true);
}
@@ -1,61 +0,0 @@
// This file is part of Substrate.
// Copyright (C) 2020 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
#![cfg(unix)]
use assert_cmd::cargo::cargo_bin;
use std::{process::Command, fs};
use tempfile::tempdir;
mod common;
#[test]
fn import_export_and_revert_work() {
let base_path = tempdir().expect("could not create a temp dir");
let exported_blocks = base_path.path().join("exported_blocks");
common::run_dev_node_for_a_while(base_path.path());
let status = Command::new(cargo_bin("substrate"))
.args(&["export-blocks", "--dev", "--pruning", "archive", "-d"])
.arg(base_path.path())
.arg(&exported_blocks)
.status()
.unwrap();
assert!(status.success());
let metadata = fs::metadata(&exported_blocks).unwrap();
assert!(metadata.len() > 0, "file exported_blocks should not be empty");
let _ = fs::remove_dir_all(base_path.path().join("db"));
let status = Command::new(cargo_bin("substrate"))
.args(&["import-blocks", "--dev", "--pruning", "archive", "-d"])
.arg(base_path.path())
.arg(&exported_blocks)
.status()
.unwrap();
assert!(status.success());
let status = Command::new(cargo_bin("substrate"))
.args(&["revert", "--dev", "--pruning", "archive", "-d"])
.arg(base_path.path())
.status()
.unwrap();
assert!(status.success());
}
@@ -22,7 +22,7 @@ use assert_cmd::cargo::cargo_bin;
use std::process::Command;
use tempfile::tempdir;
mod common;
pub mod common;
#[test]
fn inspect_works() {
@@ -20,7 +20,7 @@ use assert_cmd::cargo::cargo_bin;
use std::process::Command;
use tempfile::tempdir;
mod common;
pub mod common;
#[test]
#[cfg(unix)]
@@ -20,7 +20,7 @@ use assert_cmd::cargo::cargo_bin;
use std::{convert::TryInto, process::Command, thread, time::Duration};
use tempfile::tempdir;
mod common;
pub mod common;
#[test]
#[cfg(unix)]