Update testool (privacy-scaling-explorations#1306)

### Description introduce few features to make testool/reporting more dev friendly. ### Issue Link partially support features listed in privacy-scaling-explorations#1303 ### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Contents - fix empty path and add hyperlink in reporting page - support test in cache levels whitelist via args. E.g. `--levels Fail,Panic` ### How Has This Been Tested? test locally --------- Co-authored-by: Wu Sung-Ming <[email protected]>
ElijahVlasov · Apr 5, 2023 · 2cd311e · 2cd311e
1 parent d4b4b81
commit 2cd311e
Show file tree

Hide file tree

Showing 7 changed files with 150 additions and 70 deletions.
diff --git a/.github/ISSUE_TEMPLATE/bug_form_template.yml b/.github/ISSUE_TEMPLATE/bug_form_template.yml
@@ -19,5 +19,5 @@ body:
           required: true
     - type: textarea
       attributes:
-        label: Concrete steps to reproduce the bug
+        label: Concrete steps to reproduce the bug. If it's able reproduce via testool, please share `test_id` from jenkins report 
         description: Leave empty if not relevant
diff --git a/testool/README.md b/testool/README.md
@@ -107,6 +107,8 @@ NOTE: if you do not execute with `--report` the tool will exit the process with
 
 Usually we have to debug and run the tests manually to check if everything works ok. We provide a set of command line parameters to help with this.
 
-- `testool [--suite xxx] --cache <cache_file>` to execute all tests, and keeping the results (cache) CSV file. If you delete entries from the cache file, and re-run the tool again, only the deleted tests will be executed again
+- `testool [--suite xxx] --cache <cache_file>` to execute all tests, and by default skipping Ignored tests in cache, and keeping the results (cache) CSV file. If you delete entries from the cache file, and re-run the tool again, only the deleted tests will be executed again
+
+- `testool [--suite xxx] --cache <cache_file> --levels fail,panic` to execute all tests but skipping the tests in cache which status (i.g. result level) is NOT Fail or Panic. Notice levels is case insensitive.
 
 - `testool [--suite xxx] --inspect <test_id>` only executed the selected test (even if cached, or ignored). Use `RUST_BACKTRACE=1` here to check if anything fails. Also gives a dump of the test as also to the geth steps executed.
diff --git a/testool/src/main.rs b/testool/src/main.rs
@@ -15,7 +15,7 @@ use statetest::{
     geth_trace, load_statetests_suite, run_statetests_suite, run_test, CircuitsConfig, Results,
     StateTest,
 };
-use std::{path::PathBuf, time::SystemTime};
+use std::{collections::HashSet, path::PathBuf, time::SystemTime};
 use strum::EnumString;
 
 const REPORT_FOLDER: &str = "report";
@@ -51,6 +51,10 @@ struct Args {
     #[clap(long)]
     cache: Option<String>,
 
+    /// whitelist level from cache result
+    #[clap(short, long, value_parser, value_delimiter = ',')]
+    levels: Vec<ResultLevel>,
+
     /// Generates log and and html file with info.
     #[clap(long)]
     report: bool,
@@ -135,6 +139,7 @@ fn go() -> Result<()> {
 
     if args.report {
         let git_hash = utils::current_git_commit()?;
+        let git_submodule_tests_hash = utils::current_submodule_git_commit()?;
         let timestamp = SystemTime::now()
             .duration_since(SystemTime::UNIX_EPOCH)
             .unwrap()
@@ -151,18 +156,30 @@ fn go() -> Result<()> {
         );
 
         // when running a report, the tests result of the containing cache file
-        // are used, but removing all Ignored tests
-        let mut results = if let Some(cache_filename) = args.cache {
-            let mut results = Results::from_file(PathBuf::from(cache_filename))?;
-            results
-                .tests
-                .retain(|_, test| test.level != ResultLevel::Ignored);
-            results
+        // are used, but by default removing all Ignored tests
+        // Another way is to skip the test which level not in whitelist_levels
+        let mut previous_results = if let Some(cache_filename) = args.cache {
+            let whitelist_levels = HashSet::<ResultLevel>::from_iter(args.levels);
+
+            let mut previous_results = Results::from_file(PathBuf::from(cache_filename))?;
+            if !whitelist_levels.is_empty() {
+                // if whitelist is provided, test not in whitelist will be skip
+                previous_results
+                    .tests
+                    .retain(|_, test| !whitelist_levels.contains(&test.level));
+            } else {
+                // by default only skip ignore
+                previous_results
+                    .tests
+                    .retain(|_, test| test.level != ResultLevel::Ignored);
+            }
+
+            previous_results
         } else {
             Results::default()
         };
-        results.set_cache(PathBuf::from(csv_filename));
-        run_statetests_suite(state_tests, &circuits_config, &suite, &mut results)?;
+        previous_results.set_cache(PathBuf::from(csv_filename));
+        run_statetests_suite(state_tests, &circuits_config, &suite, &mut previous_results)?;
 
         // filter non-csv files and files from the same commit
         let mut files: Vec<_> = std::fs::read_dir(REPORT_FOLDER)
@@ -185,8 +202,8 @@ fn go() -> Result<()> {
         } else {
             None
         };
-        let report = results.report(previous);
-        std::fs::write(&html_filename, report.gen_html()?)?;
+        let report = previous_results.report(previous);
+        std::fs::write(&html_filename, report.gen_html(git_submodule_tests_hash)?)?;
 
         report.print_tty()?;
         info!("{}", html_filename);

diff --git a/testool/src/statetest/report.handlebars b/testool/src/statetest/report.handlebars
@@ -1,6 +1,10 @@
 <HTML>
 <HEAD>
 <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/skeleton-framework/1.1.1/skeleton.css">
+<style>
+table#table tr th:first-child { display: none; }
+table#table tr td:first-child { display: none; }
+</style>
 </HEAD>
 <BODY>
 
@@ -23,9 +27,10 @@ This file contains:<br>
 {{{ by_result }}}
 
 <H2 id="all">All results</H2>
-<table>
+<table id="table">
 <tr>
     <th>id</th>
+    <th>test_id</th>
     <th>level</th>
     <th>details</th>
     <th>path</th>
@@ -35,14 +40,19 @@ This file contains:<br>
             <td>
                 {{@key}}
             </td>
+            <td id="{{@key}}">
+                {{this.test_id}}
+            </td>
             <td>
                 {{this.level}}
             </td>
             <td>
                 {{this.details}}
             </td>
             <td>
-                {{this.path}}
+                <a href='https://github.com/ethereum/tests/blob/{{@root.githash}}/{{this.path}}' target="_blank">
+                    https://github.com/ethereum/tests/blob/{{@root.githash}}/{{this.path}}
+                </a>
             </td>
         </tr>
 {{/each}}

diff --git a/testool/src/statetest/results.rs b/testool/src/statetest/results.rs
@@ -16,16 +16,22 @@ const MAX_DETAILS_LEN: usize = 128;
 
 #[derive(Clone, Copy, Debug, Hash, Eq, PartialEq, EnumIter, EnumString, Serialize, Deserialize)]
 pub enum ResultLevel {
+    #[strum(ascii_case_insensitive)]
     Success,
+    #[strum(ascii_case_insensitive)]
     Ignored,
+    #[strum(ascii_case_insensitive)]
     Fail,
+    #[strum(ascii_case_insensitive)]
     Panic,
 }
 
 #[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize)]
 pub struct ResultInfo {
+    pub test_id: String,
     pub level: ResultLevel,
     pub details: String,
+    pub path: String,
 }
 
 impl ResultLevel {
@@ -40,6 +46,7 @@ impl ResultLevel {
         .to_string()
     }
 }
+
 pub struct DiffEntry {
     id: String,
     prev: Option<ResultInfo>,
@@ -136,7 +143,7 @@ impl Report {
         }
         Ok(())
     }
-    pub fn gen_html(&self) -> Result<String> {
+    pub fn gen_html(&self, githash: String) -> Result<String> {
         let template = include_str!("report.handlebars");
         let reg = Handlebars::new();
         let mut by_folder = Vec::new();
@@ -147,11 +154,24 @@ impl Report {
         self.by_result.print_html(&mut by_result)?;
         self.diffs.gen_info().1.print_html(&mut diffs)?;
 
+        // strip_prefix `tests/` for rendering purpose. It helps to generate hyperlink
+        let leading_tests_path = "tests/";
+        let mut tests_for_render = self.tests.clone();
+        for (_, result) in tests_for_render.iter_mut() {
+            assert!(result.path.starts_with(leading_tests_path));
+            result.path = result
+                .path
+                .strip_prefix(leading_tests_path)
+                .unwrap()
+                .to_string();
+        }
+
         let data = &json!({
                 "by_folder": String::from_utf8(by_folder)?,
                 "by_result" : String::from_utf8(by_result)? ,
                 "diffs" : String::from_utf8(diffs)?,
-                "all_results" : self.tests
+                "all_results" : tests_for_render,
+                "githash": githash,
         });
 
         let html = reg.render_template(template, data)?;
@@ -172,14 +192,29 @@ impl Results {
         file.read_to_string(&mut buf)?;
         let mut tests = HashMap::new();
         for line in buf.lines().filter(|l| l.len() > 1) {
-            let mut split = line.splitn(3, ';');
+            let split: Vec<&str> = line.splitn(4, ';').collect();
+            if split.len() != 4 {
+                log::warn!("un-supported line {:?}", line);
+                return Ok(Self { cache: None, tests });
+            }
+            let mut split = split.iter();
             let level = split.next().unwrap();
             let level = ResultLevel::from_str(level).unwrap();
-            let id = split.next().unwrap().to_string();
+            let test_id = split.next().unwrap().to_string();
             let details = urlencoding::decode(split.next().unwrap())
                 .expect("should be urldecodeable")
                 .to_string();
-            tests.insert(id, ResultInfo { level, details });
+            let path = split.next().unwrap().to_string();
+            let id = format!("{}#{}", test_id, path);
+            tests.insert(
+                id,
+                ResultInfo {
+                    test_id,
+                    level,
+                    details,
+                    path,
+                },
+            );
         }
         Ok(Self { cache: None, tests })
     }
@@ -329,29 +364,32 @@ impl Results {
     }
 
     #[allow(clippy::map_entry)]
-    pub fn insert(&mut self, test_id: String, result: ResultInfo) -> Result<()> {
-        if !self.tests.contains_key(&test_id) {
+    pub fn insert(&mut self, result: ResultInfo) -> Result<()> {
+        if !self.tests.contains_key(&result.test_id) {
             if result.level == ResultLevel::Ignored {
                 log::debug!(
                     target : "testool",
-                    "{} {} {}",
+                    "{} {} {} {}",
                     result.level.display_string(),
-                    test_id,
-                    result.details
+                    result.test_id,
+                    result.details,
+                    result.path,
                 );
             } else {
                 log::info!(
-                    "{} {} {}",
+                    "{} {} {} {}",
                     result.level.display_string(),
-                    test_id,
-                    result.details
+                    result.test_id,
+                    result.details,
+                    result.path,
                 );
             }
             let entry = format!(
-                "{:?};{};{}\n",
+                "{:?};{};{};{}\n",
                 result.level,
-                test_id,
-                urlencoding::encode(&result.details)
+                result.test_id,
+                urlencoding::encode(&result.details),
+                result.path,
             );
             if let Some(path) = &self.cache {
                 std::fs::OpenOptions::new()
@@ -362,7 +400,8 @@ impl Results {
                     .open(path)?
                     .write_all(entry.as_bytes())?;
             }
-            self.tests.insert(test_id, result);
+            let id = format!("{}#{}", result.test_id, result.path);
+            self.tests.insert(id, result);
         }
 
         Ok(())