Add 'skipped' status to test summary

3 files changed, 69 insertions, 29 deletions

diff --git a/test/test-manager/src/main.rs b/test/test-manager/src/main.rs
index 78b951e90c..f5e7a70d9f 100644
--- a/test/test-manager/src/main.rs
+++ b/test/test-manager/src/main.rs
@@ -208,15 +208,6 @@ async fn main() -> Result<()> {
             verbose,
             test_report,
         } => {
-            let summary_logger = match test_report {
-                Some(path) => Some(
-                    summary::SummaryLogger::new(&name, &path)
-                        .await
-                        .context("Failed to create summary logger")?,
-                ),
-                None => None,
-            };
-
             let mut config = config.clone();
             config.runtime_opts.display = match (display, vnc.is_some()) {
                 (false, false) => config::Display::None,
@@ -233,6 +224,19 @@ async fn main() -> Result<()> {
 
             let vm_config = vm::get_vm_config(&config, &name).context("Cannot get VM config")?;
 
+            let summary_logger = match test_report {
+                Some(path) => Some(
+                    summary::SummaryLogger::new(
+                        &name,
+                        test_rpc::meta::Os::from(vm_config.os_type),
+                        &path,
+                    )
+                    .await
+                    .context("Failed to create summary logger")?,
+                ),
+                None => None,
+            };
+
             let manifest = package::get_app_manifest(vm_config, current_app, previous_app)
                 .await
                 .context("Could not find the specified app packages")?;
diff --git a/test/test-manager/src/summary.rs b/test/test-manager/src/summary.rs
index b8a6d5d809..27ff2db572 100644
--- a/test/test-manager/src/summary.rs
+++ b/test/test-manager/src/summary.rs
@@ -1,4 +1,5 @@
 use std::{collections::BTreeMap, io, path::Path};
+use test_rpc::meta::Os;
 use tokio::{
     fs,
     io::{AsyncBufReadExt, AsyncWriteExt},
@@ -15,18 +16,24 @@ pub enum Error {
     Read(#[error(source)] io::Error),
     #[error(display = "Failed to parse log file")]
     Parse,
+    #[error(display = "Failed to serialize value")]
+    Serialize(#[error(source)] serde_json::Error),
+    #[error(display = "Failed to deserialize value")]
+    Deserialize(#[error(source)] serde_json::Error),
 }
 
 #[derive(Clone, Copy)]
 pub enum TestResult {
     Pass,
     Fail,
+    Skip,
     Unknown,
 }
 
 impl TestResult {
     const PASS_STR: &'static str = "✅";
     const FAIL_STR: &'static str = "❌";
+    const SKIP_STR: &'static str = "↪️";
     const UNKNOWN_STR: &'static str = " ";
 }
 
@@ -37,6 +44,7 @@ impl std::str::FromStr for TestResult {
         match s {
             TestResult::PASS_STR => Ok(TestResult::Pass),
             TestResult::FAIL_STR => Ok(TestResult::Fail),
+            TestResult::SKIP_STR => Ok(TestResult::Skip),
             _ => Ok(TestResult::Unknown),
         }
     }
@@ -47,6 +55,7 @@ impl std::fmt::Display for TestResult {
         match self {
             TestResult::Pass => f.write_str(TestResult::PASS_STR),
             TestResult::Fail => f.write_str(TestResult::FAIL_STR),
+            TestResult::Skip => f.write_str(TestResult::SKIP_STR),
             TestResult::Unknown => f.write_str(TestResult::UNKNOWN_STR),
         }
     }
@@ -60,7 +69,7 @@ pub struct SummaryLogger {
 impl SummaryLogger {
     /// Create a new logger and log to `path`. If `path` does not exist, it will be created. If it
     /// already exists, it is truncated and overwritten.
-    pub async fn new(name: &str, path: &Path) -> Result<SummaryLogger, Error> {
+    pub async fn new(name: &str, os: Os, path: &Path) -> Result<SummaryLogger, Error> {
         let mut file = fs::OpenOptions::new()
             .create(true)
             .write(true)
@@ -69,11 +78,14 @@ impl SummaryLogger {
             .await
             .map_err(|err| Error::Open(err, path.to_path_buf()))?;
 
-        // The first row is the summary name
         file.write_all(name.as_bytes())
             .await
             .map_err(Error::Write)?;
         file.write_u8(b'\n').await.map_err(Error::Write)?;
+        file.write_all(&serde_json::to_vec(&os).map_err(Error::Serialize)?)
+            .await
+            .map_err(Error::Write)?;
+        file.write_u8(b'\n').await.map_err(Error::Write)?;
 
         Ok(SummaryLogger { file })
     }
@@ -113,15 +125,18 @@ pub async fn maybe_log_test_result(
 
 /// Parsed summary results
 pub struct Summary {
-    /// Summary name
-    name: String,
+    /// Name of the configuration
+    config_name: String,
     /// Pairs of test names mapped to test results
     results: BTreeMap<String, TestResult>,
 }
 
 impl Summary {
     /// Read test summary from `path`.
-    pub async fn parse_log<P: AsRef<Path>>(path: P) -> Result<Summary, Error> {
+    pub async fn parse_log<P: AsRef<Path>>(
+        all_tests: &[&crate::tests::TestMetadata],
+        path: P,
+    ) -> Result<Summary, Error> {
         let file = fs::OpenOptions::new()
             .read(true)
             .open(&path)
@@ -130,11 +145,17 @@ impl Summary {
 
         let mut lines = tokio::io::BufReader::new(file).lines();
 
-        let name = lines
+        let config_name = lines
             .next_line()
             .await
             .map_err(Error::Read)?
             .ok_or(Error::Parse)?;
+        let os = lines
+            .next_line()
+            .await
+            .map_err(Error::Read)?
+            .ok_or(Error::Parse)?;
+        let os: Os = serde_json::from_str(&os).map_err(Error::Deserialize)?;
 
         let mut results = BTreeMap::new();
 
@@ -147,7 +168,20 @@ impl Summary {
             results.insert(test_name.to_owned(), test_result);
         }
 
-        Ok(Summary { name, results })
+        for test in all_tests {
+            // Add missing test results
+            let entry = results.entry(test.name.to_owned());
+            if test.should_run_on_os(os) {
+                entry.or_insert(TestResult::Unknown);
+            } else {
+                entry.or_insert(TestResult::Skip);
+            }
+        }
+
+        Ok(Summary {
+            config_name,
+            results,
+        })
     }
 
     // Return all tests which passed.
@@ -165,18 +199,18 @@ impl Summary {
 /// exist. If some log file which is expected to exist, but for any reason fails to
 /// be parsed, we should not abort the entire summarization.
 pub async fn print_summary_table<P: AsRef<Path>>(summary_files: &[P]) {
-    let mut summaries = Vec::new();
-    let mut failed_to_parse = Vec::new();
+    // Collect test details
+    let tests: Vec<_> = inventory::iter::<crate::tests::TestMetadata>().collect();
+
+    let mut summaries = vec![];
+    let mut failed_to_parse = vec![];
     for sumfile in summary_files {
-        match Summary::parse_log(sumfile).await {
+        match Summary::parse_log(&tests, sumfile).await {
             Ok(summary) => summaries.push(summary),
             Err(_) => failed_to_parse.push(sumfile),
         }
     }
 
-    // Collect test details
-    let tests: Vec<_> = inventory::iter::<crate::tests::TestMetadata>().collect();
-
     // Print a table
     println!("<table>");
 
@@ -185,7 +219,7 @@ pub async fn print_summary_table<P: AsRef<Path>>(summary_files: &[P]) {
     println!("<td style='text-align: center;'>Test ⬇️ / Platform ➡️ </td>");
 
     for summary in &summaries {
-        let total_tests = tests.len();
+        let total_tests = summary.results.len();
         let total_passed = summary.passed().len();
         let counter_text = if total_passed == total_tests {
             String::from(TestResult::PASS_STR)
@@ -194,7 +228,7 @@ pub async fn print_summary_table<P: AsRef<Path>>(summary_files: &[P]) {
         };
         println!(
             "<td style='text-align: center;'>{} {}</td>",
-            summary.name, counter_text
+            summary.config_name, counter_text
         );
     }
 
@@ -203,7 +237,7 @@ pub async fn print_summary_table<P: AsRef<Path>>(summary_files: &[P]) {
     println!("{}", {
         let oses_passed: Vec<_> = summaries
             .iter()
-            .filter(|summary| summary.passed().len() == tests.len())
+            .filter(|summary| summary.passed().len() == summary.results.len())
             .collect();
         if oses_passed.len() == summaries.len() {
             "🎉 All Platforms passed 🎉".to_string()
@@ -211,7 +245,7 @@ pub async fn print_summary_table<P: AsRef<Path>>(summary_files: &[P]) {
             let failed: usize = summaries
                 .iter()
                 .map(|summary| {
-                    if summary.passed().len() == tests.len() {
+                    if summary.passed().len() == summary.results.len() {
                         0
                     } else {
                         1
@@ -246,9 +280,9 @@ pub async fn print_summary_table<P: AsRef<Path>>(summary_files: &[P]) {
                 .unwrap_or(&TestResult::Unknown);
             match result {
                 TestResult::Fail | TestResult::Unknown => {
-                    failed_platforms.push(summary.name.clone())
+                    failed_platforms.push(summary.config_name.clone())
                 }
-                TestResult::Pass => (),
+                TestResult::Pass | TestResult::Skip => (),
             }
             println!("<td style='text-align: center;'>{}</td>", result);
         }
@@ -267,7 +301,7 @@ pub async fn print_summary_table<P: AsRef<Path>>(summary_files: &[P]) {
         );
         println!("</td>");
 
-        // List the test name again (Useful for the summary accross the different platforms)
+        // List the test name again (Useful for the summary across the different platforms)
         println!("<td>{}</td>", test.name);
 
         // End row
@@ -279,4 +313,5 @@ pub async fn print_summary_table<P: AsRef<Path>>(summary_files: &[P]) {
     // Print explanation of test result
     println!("<p>{} = Test passed</p>", TestResult::PASS_STR);
     println!("<p>{} = Test failed</p>", TestResult::FAIL_STR);
+    println!("<p>{} = Test skipped</p>", TestResult::SKIP_STR);
 }
diff --git a/test/test-rpc/src/meta.rs b/test/test-rpc/src/meta.rs
index 958733b9be..09200f8690 100644
--- a/test/test-rpc/src/meta.rs
+++ b/test/test-rpc/src/meta.rs
@@ -2,6 +2,7 @@ use serde::{Deserialize, Serialize};
 use std::str::FromStr;
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Copy)]
+#[serde(rename_all = "snake_case")]
 pub enum Os {
     Linux,
     Macos,