Merge pull request #138 from dfinance/verbose-output-on-test-failure

show executor output on test failure, refactor output formatting
dfinance · Nov 5, 2020 · d966055 · d966055
2 parents 712f764 + 763a60c
commit d966055
Show file tree

Hide file tree

Showing 5 changed files with 125 additions and 74 deletions.
diff --git a/executor/src/bin/executor_cli.rs b/executor/src/bin/executor_cli.rs
@@ -5,22 +5,12 @@ use anyhow::{Context, Result};
 use clap::{App, Arg};
 
 use move_executor::execute_script;
-use move_executor::explain::{
-    PipelineExecutionResult, StepExecutionResult, ResourceChange, AddressResourceChanges,
-};
 use move_lang::name_pool::ConstPool;
 use lang::compiler::file::MoveFile;
 use lang::compiler::file;
 use lang::compiler::error::CompilerError;
 use move_lang::errors::report_errors;
-
-fn formatted_resource_change(change: &ResourceChange) -> String {
-    let ResourceChange(ty, val) = change;
-    match val {
-        Some(val) => format!("{} =\n    {}", ty, val),
-        None => ty.to_string(),
-    }
-}
+use move_executor::format::format_step_result;
 
 fn cli() -> App<'static, 'static> {
     App::new("Move Executor")
@@ -85,60 +75,12 @@ fn main() -> Result<()> {
     let res = execute_script(script, deps, dialect, sender, args);
     match res {
         Ok(exec_result) => {
-            let PipelineExecutionResult { step_results } = exec_result;
-            for (i, (name, gas, writeset_size, step_result)) in
-                step_results.into_iter().enumerate()
-            {
+            let step_results = exec_result.step_results;
+            for (i, step_result) in step_results.into_iter().enumerate() {
                 if i > 0 {
                     println!();
                 }
-                let status = if matches!(step_result, StepExecutionResult::Error(_)) {
-                    "ERROR"
-                } else {
-                    "OK"
-                };
-                println!("{} ...... {}", name, status,);
-                println!("[gas: {}, writeset bytes: {}]", gas, writeset_size);
-
-                let step_indent = "    ";
-                let content_indent = "        ";
-                match step_result {
-                    StepExecutionResult::Error(error)
-                    | StepExecutionResult::ExpectedError(error) => {
-                        print!("{}", textwrap::indent(&error, step_indent));
-                    }
-                    StepExecutionResult::Success(effects) => {
-                        for changes in effects.resources() {
-                            let AddressResourceChanges { address, changes } = changes;
-                            print!("{}", textwrap::indent(address, step_indent));
-                            for (operation, change) in changes {
-                                print!(
-                                    "{}",
-                                    textwrap::indent(
-                                        &format!(
-                                            "{} {}",
-                                            operation,
-                                            formatted_resource_change(change)
-                                        ),
-                                        content_indent
-                                    )
-                                );
-                            }
-                        }
-                        if !effects.events().is_empty() {
-                            print!("{}", textwrap::indent("Events:", content_indent));
-                            for event_change in effects.events() {
-                                print!(
-                                    "{}",
-                                    textwrap::indent(
-                                        &formatted_resource_change(event_change),
-                                        &(content_indent.to_owned() + "    ")
-                                    )
-                                );
-                            }
-                        }
-                    }
-                }
+                print!("{}", format_step_result(step_result, true, true));
             }
             Ok(())
         }

diff --git a/executor/src/bin/test.rs b/executor/src/bin/test.rs
@@ -6,10 +6,7 @@ use move_executor::explain::StepExecutionResult;
 use lang::compiler::{ConstPool, file};
 use lang::compiler::error::CompilerError;
 use move_lang::errors::report_errors;
-
-pub fn print_test_status(test_name: &str, status: &str) {
-    println!("{} ....... {}", test_name, status);
-}
+use move_executor::format::format_step_result;
 
 fn cli() -> App<'static, 'static> {
     App::new("Test runner")
@@ -96,18 +93,26 @@ pub fn main() -> Result<()> {
 
         match exec_result.last() {
             None => {
-                print_test_status(test_name, "NO_SCRIPT");
+                println!("{} ....... SCRIPT_NOT_FOUND", test_name);
             }
             Some(step_result) => match step_result {
-                StepExecutionResult::Error(error) => {
-                    print_test_status(test_name, "ERROR");
-
+                StepExecutionResult::Error(_) => {
                     has_failures = true;
-                    print!("{}", textwrap::indent(&error, "    "));
+                    println!("{} .......", test_name);
+
+                    for step_result in exec_result.step_results {
+                        print!(
+                            "{}",
+                            textwrap::indent(
+                                &format_step_result(step_result, true, false),
+                                "    "
+                            )
+                        );
+                    }
                     println!();
                 }
                 StepExecutionResult::ExpectedError(_) | StepExecutionResult::Success(_) => {
-                    print_test_status(test_name, "ok");
+                    println!("{} ....... ok", test_name);
                 }
             },
         }

diff --git a/executor/src/explain.rs b/executor/src/explain.rs
@@ -16,13 +16,15 @@ use num_format::ToFormattedString;
 use crate::session::ConstsMap;
 use move_lang::shared::Address;
 
+pub type StepResultInfo = (String, u64, usize, StepExecutionResult);
+
 #[derive(Debug)]
 pub struct PipelineExecutionResult {
-    pub step_results: Vec<(String, u64, usize, StepExecutionResult)>,
+    pub step_results: Vec<StepResultInfo>,
 }
 
 impl PipelineExecutionResult {
-    pub fn new(step_results: Vec<(String, u64, usize, StepExecutionResult)>) -> Self {
+    pub fn new(step_results: Vec<StepResultInfo>) -> Self {
         PipelineExecutionResult { step_results }
     }
 

diff --git a/executor/src/format.rs b/executor/src/format.rs
@@ -0,0 +1,101 @@
+use std::fmt::Write;
+
+use crate::explain::{
+    StepExecutionResult, AddressResourceChanges, ResourceChange, ExplainedTransactionEffects,
+    StepResultInfo,
+};
+
+const STEP_INDENT: &str = "    ";
+
+fn indent(num: usize) -> String {
+    let mut indent = String::new();
+    for _ in 0..num {
+        indent += STEP_INDENT;
+    }
+    indent
+}
+
+fn formatted_resource_change(change: &ResourceChange) -> String {
+    let ResourceChange(ty, val) = change;
+    match val {
+        Some(val) => format!("{} =\n    {}", ty, val),
+        None => ty.to_string(),
+    }
+}
+
+fn format_error(out: &mut String, error: String) {
+    write!(out, "{}", textwrap::indent(&error, &indent(1))).unwrap()
+}
+
+fn format_effects(out: &mut String, effects: ExplainedTransactionEffects) {
+    for changes in effects.resources() {
+        let AddressResourceChanges { address, changes } = changes;
+        write!(out, "{}", textwrap::indent(address, &indent(1))).unwrap();
+        for (operation, change) in changes {
+            write!(
+                out,
+                "{}",
+                textwrap::indent(
+                    &format!("{} {}", operation, formatted_resource_change(change)),
+                    &indent(2)
+                )
+            )
+            .unwrap();
+        }
+    }
+    if !effects.events().is_empty() {
+        write!(out, "{}", textwrap::indent("Events:", &indent(2))).unwrap();
+        for event_change in effects.events() {
+            write!(
+                out,
+                "{}",
+                textwrap::indent(&formatted_resource_change(event_change), &indent(3))
+            )
+            .unwrap();
+        }
+    }
+}
+
+fn format_exec_status(step_exec_result: &StepExecutionResult) -> String {
+    match step_exec_result {
+        StepExecutionResult::Error(_) => "FAILED",
+        _ => "ok",
+    }
+    .to_string()
+}
+
+pub fn format_step_result(
+    step_result: StepResultInfo,
+    verbose: bool,
+    show_stats: bool,
+) -> String {
+    let mut out = String::new();
+    let (name, gas, writeset_size, step_result) = step_result;
+
+    let status = format_exec_status(&step_result);
+    writeln!(&mut out, "{} ...... {}", name, status).unwrap();
+
+    if show_stats {
+        writeln!(
+            &mut out,
+            "[gas: {}, writeset bytes: {}]",
+            gas, writeset_size
+        )
+        .unwrap();
+    }
+
+    match step_result {
+        StepExecutionResult::Error(error) => format_error(&mut out, error),
+        StepExecutionResult::ExpectedError(error) => {
+            if verbose {
+                format_error(&mut out, error)
+            }
+        }
+        StepExecutionResult::Success(effects) => {
+            if verbose {
+                format_effects(&mut out, effects);
+            }
+        }
+    }
+    out
+}
diff --git a/executor/src/lib.rs b/executor/src/lib.rs
@@ -13,6 +13,7 @@ use lang::compiler::file::MoveFile;
 pub mod constants;
 pub mod execution;
 pub mod explain;
+pub mod format;
 pub mod meta;
 pub mod oracles;
 pub mod session;