Skip to content

Commit

Permalink
feat(levm): disable spinner tests (#1396)
Browse files Browse the repository at this point in the history
**Motivation**

<!-- Why does this pull request exist? What are its goals? -->
- When running EFTests in CI the spinner is very annoying and it makes
it hard to observe the output. The objective is to make the EFTests more
suitable for the CI so that we can use that as a tool for checking when
something stopped working (or simply tracking progress)

**Description**

<!-- A clear and concise general description of the changes this PR
introduces -->
- Implements flag `disable-spinner`, if set, the main spinners will stop
and it will replace the "spinner prints" for `println!`
- Fixes index error in REVM

<!-- Link to issues: Resolves #111, Resolves #222 -->

Closes #issue_number
  • Loading branch information
JereSalo authored Dec 3, 2024
1 parent 2212fa9 commit bcdbf6d
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 62 deletions.
56 changes: 38 additions & 18 deletions cmd/ef_tests/levm/parser.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
report::format_duration_as_mm_ss,
runner::EFTestRunnerOptions,
runner::{spinner_success_or_print, spinner_update_text_or_print, EFTestRunnerOptions},
types::{EFTest, EFTests},
};
use colored::Colorize;
Expand All @@ -24,6 +24,9 @@ pub fn parse_ef_tests(opts: &EFTestRunnerOptions) -> Result<Vec<EFTest>, EFTestP
let cargo_manifest_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
let ef_general_state_tests_path = cargo_manifest_dir.join("vectors/GeneralStateTests");
let mut spinner = Spinner::new(Dots, "Parsing EF Tests".bold().to_string(), Color::Cyan);
if opts.disable_spinner {
spinner.stop();
}
let mut tests = Vec::new();
for test_dir in std::fs::read_dir(ef_general_state_tests_path.clone())
.map_err(|err| {
Expand All @@ -37,12 +40,13 @@ pub fn parse_ef_tests(opts: &EFTestRunnerOptions) -> Result<Vec<EFTest>, EFTestP
let directory_tests = parse_ef_test_dir(test_dir, opts, &mut spinner)?;
tests.extend(directory_tests);
}
spinner.success(
&format!(
spinner_success_or_print(
&mut spinner,
format!(
"Parsed EF Tests in {}",
format_duration_as_mm_ss(parsing_time.elapsed())
)
.bold(),
),
opts.disable_spinner,
);
Ok(tests)
}
Expand All @@ -52,7 +56,11 @@ pub fn parse_ef_test_dir(
opts: &EFTestRunnerOptions,
directory_parsing_spinner: &mut Spinner,
) -> Result<Vec<EFTest>, EFTestParseError> {
directory_parsing_spinner.update_text(format!("Parsing directory {:?}", test_dir.file_name()));
spinner_update_text_or_print(
directory_parsing_spinner,
format!("Parsing directory {:?}", test_dir.file_name()),
opts.disable_spinner,
);

let mut directory_tests = Vec::new();
for test in std::fs::read_dir(test_dir.path())
Expand Down Expand Up @@ -93,10 +101,14 @@ pub fn parse_ef_test_dir(
.tests
.contains(&test_dir.file_name().to_str().unwrap().to_owned())
{
directory_parsing_spinner.update_text(format!(
"Skipping test {:?} as it is not in the list of tests to run",
test.path().file_name()
));
spinner_update_text_or_print(
directory_parsing_spinner,
format!(
"Skipping test {:?} as it is not in the list of tests to run",
test.path().file_name()
),
opts.disable_spinner,
);
return Ok(Vec::new());
}

Expand All @@ -105,10 +117,14 @@ pub fn parse_ef_test_dir(
.skip
.contains(&test_dir.file_name().to_str().unwrap().to_owned())
{
directory_parsing_spinner.update_text(format!(
"Skipping test {:?} as it is in the folder of tests to skip",
test.path().file_name()
));
spinner_update_text_or_print(
directory_parsing_spinner,
format!(
"Skipping test {:?} as it is in the folder of tests to skip",
test.path().file_name()
),
opts.disable_spinner,
);
continue;
}

Expand All @@ -122,10 +138,14 @@ pub fn parse_ef_test_dir(
.unwrap()
.to_owned(),
) {
directory_parsing_spinner.update_text(format!(
"Skipping test {:?} as it is in the list of tests to skip",
test.path().file_name()
));
spinner_update_text_or_print(
directory_parsing_spinner,
format!(
"Skipping test {:?} as it is in the list of tests to skip",
test.path().file_name()
),
opts.disable_spinner,
);
continue;
}

Expand Down
150 changes: 106 additions & 44 deletions cmd/ef_tests/levm/runner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,24 @@ pub struct EFTestRunnerOptions {
pub summary: bool,
#[arg(long, value_name = "SKIP", use_value_delimiter = true)]
pub skip: Vec<String>,
#[arg(long, value_name = "LEVM_ONLY", default_value = "false")]
pub levm_only: bool,
#[arg(long, value_name = "DISABLE_SPINNER", default_value = "false")]
pub disable_spinner: bool, // Replaces spinner for normal prints.
}

pub fn spinner_update_text_or_print(spinner: &mut Spinner, text: String, no_spinner: bool) {
if no_spinner {
println!("{}", text);
} else {
spinner.update_text(text);
}
}

pub fn spinner_success_or_print(spinner: &mut Spinner, text: String, no_spinner: bool) {
if no_spinner {
println!("{}", text);
} else {
spinner.success(&text);
}
}

pub fn run_ef_tests(
Expand All @@ -60,11 +76,8 @@ pub fn run_ef_tests(
if reports.is_empty() {
run_with_levm(&mut reports, &ef_tests, opts)?;
}
if opts.summary {
return Ok(());
}
if !opts.levm_only {
re_run_with_revm(&mut reports, &ef_tests)?;
if !opts.summary {
re_run_with_revm(&mut reports, &ef_tests, opts)?;
}
write_report(&reports)
}
Expand All @@ -80,12 +93,13 @@ fn run_with_levm(
report::progress(reports, levm_run_time.elapsed()),
Color::Cyan,
);
if opts.disable_spinner {
levm_run_spinner.stop();
}
for test in ef_tests.iter() {
// println!(
// "Time elapsed: {:?}",
// format_duration_as_mm_ss(levm_run_time.elapsed())
// );
// println!("Running test: {:?}", test.name);
if opts.disable_spinner {
println!("Running test: {:?}", test.name);
}
let ef_test_report = match levm_runner::run_ef_test(test) {
Ok(ef_test_report) => ef_test_report,
Err(EFTestRunnerError::Internal(err)) => return Err(EFTestRunnerError::Internal(err)),
Expand All @@ -96,17 +110,33 @@ fn run_with_levm(
}
};
reports.push(ef_test_report);
levm_run_spinner.update_text(report::progress(reports, levm_run_time.elapsed()));
spinner_update_text_or_print(
&mut levm_run_spinner,
report::progress(reports, levm_run_time.elapsed()),
opts.disable_spinner,
);
}
levm_run_spinner.success(&report::progress(reports, levm_run_time.elapsed()));
spinner_success_or_print(
&mut levm_run_spinner,
report::progress(reports, levm_run_time.elapsed()),
opts.disable_spinner,
);

if opts.summary {
report::write_summary_for_slack(reports)?;
report::write_summary_for_github(reports)?;
}

let mut summary_spinner = Spinner::new(Dots, "Loading summary...".to_owned(), Color::Cyan);
summary_spinner.success(&report::summary_for_shell(reports));
if opts.disable_spinner {
summary_spinner.stop();
}
spinner_success_or_print(
&mut summary_spinner,
report::summary_for_shell(reports),
opts.disable_spinner,
);

Ok(())
}

Expand All @@ -115,21 +145,32 @@ fn run_with_levm(
fn _run_with_revm(
reports: &mut Vec<EFTestReport>,
ef_tests: &[EFTest],
opts: &EFTestRunnerOptions,
) -> Result<(), EFTestRunnerError> {
let revm_run_time = std::time::Instant::now();
let mut revm_run_spinner = Spinner::new(
Dots,
"Running all tests with REVM...".to_owned(),
Color::Cyan,
);
if opts.disable_spinner {
revm_run_spinner.stop();
}
for (idx, test) in ef_tests.iter().enumerate() {
if opts.disable_spinner {
println!("Running test: {:?}", test.name);
}
let total_tests = ef_tests.len();
revm_run_spinner.update_text(format!(
"{} {}/{total_tests} - {}",
"Running all tests with REVM".bold(),
idx + 1,
format_duration_as_mm_ss(revm_run_time.elapsed())
));
spinner_update_text_or_print(
&mut revm_run_spinner,
format!(
"{} {}/{total_tests} - {}",
"Running all tests with REVM".bold(),
idx + 1,
format_duration_as_mm_ss(revm_run_time.elapsed())
),
opts.disable_spinner,
);
let ef_test_report = match revm_runner::_run_ef_test_revm(test) {
Ok(ef_test_report) => ef_test_report,
Err(EFTestRunnerError::Internal(err)) => return Err(EFTestRunnerError::Internal(err)),
Expand All @@ -140,41 +181,58 @@ fn _run_with_revm(
}
};
reports.push(ef_test_report);
revm_run_spinner.update_text(report::progress(reports, revm_run_time.elapsed()));
spinner_update_text_or_print(
&mut revm_run_spinner,
report::progress(reports, revm_run_time.elapsed()),
opts.disable_spinner,
);
}
revm_run_spinner.success(&format!(
"Ran all tests with REVM in {}",
format_duration_as_mm_ss(revm_run_time.elapsed())
));
spinner_success_or_print(
&mut revm_run_spinner,
format!(
"Ran all tests with REVM in {}",
format_duration_as_mm_ss(revm_run_time.elapsed())
),
opts.disable_spinner,
);
Ok(())
}

fn re_run_with_revm(
reports: &mut [EFTestReport],
ef_tests: &[EFTest],
opts: &EFTestRunnerOptions,
) -> Result<(), EFTestRunnerError> {
let revm_run_time = std::time::Instant::now();
let mut revm_run_spinner = Spinner::new(
Dots,
"Running failed tests with REVM...".to_owned(),
Color::Cyan,
);
if opts.disable_spinner {
revm_run_spinner.stop();
}
let failed_tests = reports.iter().filter(|report| !report.passed()).count();
for (idx, failed_test_report) in reports.iter_mut().enumerate() {
if failed_test_report.passed() {
continue;

// Iterate only over failed tests
for (idx, failed_test_report) in reports
.iter_mut()
.filter(|report| !report.passed())
.enumerate()
{
if opts.disable_spinner {
println!("Running test: {:?}", failed_test_report.name);
}
// println!(
// "Time elapsed: {:?}",
// format_duration_as_mm_ss(revm_run_time.elapsed())
// );
// println!("Running test: {:?}", failed_test_report.name);
revm_run_spinner.update_text(format!(
"{} {}/{failed_tests} - {}",
"Re-running failed tests with REVM".bold(),
idx + 1,
format_duration_as_mm_ss(revm_run_time.elapsed())
));
spinner_update_text_or_print(
&mut revm_run_spinner,
format!(
"{} {}/{failed_tests} - {}",
"Re-running failed tests with REVM".bold(),
idx + 1,
format_duration_as_mm_ss(revm_run_time.elapsed())
),
opts.disable_spinner,
);

match revm_runner::re_run_failed_ef_test(
ef_tests
Expand All @@ -201,10 +259,14 @@ fn re_run_with_revm(
}
}
}
revm_run_spinner.success(&format!(
"Re-ran failed tests with REVM in {}",
format_duration_as_mm_ss(revm_run_time.elapsed())
));
spinner_success_or_print(
&mut revm_run_spinner,
format!(
"Re-ran failed tests with REVM in {}",
format_duration_as_mm_ss(revm_run_time.elapsed())
),
opts.disable_spinner,
);
Ok(())
}

Expand Down

0 comments on commit bcdbf6d

Please sign in to comment.