Merge pull request #259 from oli-obk/update_status

Remove `update_status`
oli-obk · Aug 15, 2024 · 2de49ab · 2de49ab
2 parents bbe74aa + 2f3a770
commit 2de49ab
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 48 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Removed
 
+* `TestStatus::update_status`, instead use a revision if you want to run subcommands
+
 ## [0.25.0] - 2024-07-24
 
 ### Added

diff --git a/src/build_manager.rs b/src/build_manager.rs
@@ -6,7 +6,10 @@ use std::{
     sync::{Arc, OnceLock, RwLock},
 };
 
-use crate::{status_emitter::StatusEmitter, Config, Errored};
+use crate::{
+    status_emitter::{RevisionStyle, StatusEmitter},
+    Config, Errored,
+};
 
 /// A build shared between all tests of the same `BuildManager`
 pub trait Build {
@@ -83,7 +86,7 @@ impl<'a> BuildManager<'a> {
             let build = self
                 .status_emitter
                 .register_test(what.description().into())
-                .for_revision("");
+                .for_revision("", RevisionStyle::Parent);
             let res = what.build(self).map_err(|e| err = Some(e));
             build.done(
                 &res.as_ref()

diff --git a/src/custom_flags/run.rs b/src/custom_flags/run.rs
@@ -5,8 +5,8 @@ use spanned::Spanned;
 use std::process::{Command, Output};
 
 use crate::{
-    build_manager::BuildManager, display, per_test_config::TestConfig, Error, Errored, TestOk,
-    TestRun,
+    build_manager::BuildManager, display, per_test_config::TestConfig,
+    status_emitter::RevisionStyle, Error, Errored, TestOk, TestRun,
 };
 
 use super::Flag;
@@ -37,7 +37,7 @@ impl Flag for Run {
             config: config.config.clone(),
             comments: config.comments,
             aux_dir: config.aux_dir,
-            status: config.status.for_revision(&revision),
+            status: config.status.for_revision(&revision, RevisionStyle::Show),
         };
         cmd.arg("--print").arg("file-names");
         let output = cmd.output().unwrap();

diff --git a/src/dependencies.rs b/src/dependencies.rs
@@ -15,8 +15,9 @@ use crate::{
     build_manager::{Build, BuildManager},
     custom_flags::Flag,
     per_test_config::TestConfig,
+    status_emitter::RevisionStyle,
     test_result::Errored,
-    CommandBuilder, Config, OutputConflictHandling,
+    CommandBuilder, Config, OutputConflictHandling, TestOk,
 };
 
 #[derive(Default, Debug)]
@@ -390,13 +391,23 @@ impl Flag for DependencyBuilder {
         config: &TestConfig<'_>,
         build_manager: &BuildManager<'_>,
     ) -> Result<(), Errored> {
-        config
-            .status
-            .update_status("waiting for dependencies to finish building".into());
-        let extra_args = build_manager.build(self.clone())?;
-        cmd.args(extra_args);
-        config.status.update_status(String::new());
-        Ok(())
+        let status = config.status.for_revision(
+            "waiting for dependencies to finish building",
+            RevisionStyle::Quiet,
+        );
+        match build_manager.build(self.clone()) {
+            Ok(extra_args) => {
+                cmd.args(extra_args);
+                status.done(&Ok(TestOk::Ok));
+                Ok(())
+            }
+            Err(err) => {
+                let err = Err(err);
+                status.done(&err);
+                #[allow(clippy::unnecessary_literal_unwrap)]
+                Err(err.unwrap_err())
+            }
+        }
     }
 }
 

diff --git a/src/lib.rs b/src/lib.rs
@@ -18,6 +18,7 @@ pub use core::CrateType;
 pub use filter::Match;
 use per_test_config::TestConfig;
 use spanned::Spanned;
+use status_emitter::RevisionStyle;
 use status_emitter::{StatusEmitter, TestStatus};
 use std::collections::VecDeque;
 use std::path::Path;
@@ -326,7 +327,7 @@ fn parse_and_test_file(
     let revisions = comments.revisions.as_deref().unwrap_or(EMPTY);
     let mut runs = vec![];
     for revision in revisions {
-        let status = status.for_revision(revision);
+        let status = status.for_revision(revision, RevisionStyle::Show);
         // Ignore file if only/ignore rules do (not) apply
         if !config.test_file_conditions(&comments, revision) {
             runs.push(TestRun {

diff --git a/src/status_emitter.rs b/src/status_emitter.rs
@@ -43,10 +43,22 @@ pub trait StatusEmitter: Sync + RefUnwindSafe {
     ) -> Box<dyn Summary>;
 }
 
+/// Some configuration options for revisions
+#[derive(Debug, Clone, Copy)]
+pub enum RevisionStyle {
+    /// Things like dependencies or aux files building are noise in non-interactif mode
+    /// and thus silenced.
+    Quiet,
+    /// Always show them, even if rendering to a file
+    Show,
+    /// The parent, only show in indicatif mode and on failure
+    Parent,
+}
+
 /// Information about a specific test run.
 pub trait TestStatus: Send + Sync + RefUnwindSafe {
     /// Create a copy of this test for a new revision.
-    fn for_revision(&self, revision: &str) -> Box<dyn TestStatus>;
+    fn for_revision(&self, revision: &str, style: RevisionStyle) -> Box<dyn TestStatus>;
 
     /// Create a copy of this test for a new path.
     fn for_path(&self, path: &Path) -> Box<dyn TestStatus>;
@@ -60,9 +72,6 @@ pub trait TestStatus: Send + Sync + RefUnwindSafe {
         stdout: &'a [u8],
     ) -> Box<dyn Debug + 'a>;
 
-    /// Change the status of the test while it is running to supply some kind of progress
-    fn update_status(&self, msg: String);
-
     /// A test has finished, handle the result immediately.
     fn done(&self, _result: &TestResult) {}
 
@@ -111,7 +120,7 @@ pub struct SilentStatus {
 }
 
 impl TestStatus for SilentStatus {
-    fn for_revision(&self, revision: &str) -> Box<dyn TestStatus> {
+    fn for_revision(&self, revision: &str, _style: RevisionStyle) -> Box<dyn TestStatus> {
         Box::new(SilentStatus {
             revision: revision.into(),
             path: self.path.clone(),
@@ -134,8 +143,6 @@ impl TestStatus for SilentStatus {
         Box::new(())
     }
 
-    fn update_status(&self, _msg: String) {}
-
     fn path(&self) -> &Path {
         &self.path
     }
@@ -179,7 +186,6 @@ enum Msg {
     Inc,
     IncLength,
     Finish,
-    Status(String, String),
 }
 
 impl Text {
@@ -212,9 +218,6 @@ impl Text {
                                     spinner.finish_and_clear();
                                 }
                             }
-                            Msg::Status(msg, status) => {
-                                threads.get_mut(&msg).unwrap().set_message(status);
-                            }
                             Msg::Push(msg) => {
                                 let spinner =
                                     bars.add(ProgressBar::new_spinner().with_prefix(msg.clone()));
@@ -295,6 +298,7 @@ struct TextTest {
     path: PathBuf,
     revision: String,
     first: AtomicBool,
+    style: RevisionStyle,
 }
 
 impl TextTest {
@@ -322,17 +326,23 @@ impl TestStatus for TextTest {
             let old_msg = self.msg();
             let msg = format!("... {result}");
             if ProgressDrawTarget::stdout().is_hidden() {
-                println!("{old_msg} {msg}");
-                std::io::stdout().flush().unwrap();
+                match self.style {
+                    RevisionStyle::Quiet => {}
+                    RevisionStyle::Show | RevisionStyle::Parent => {
+                        let revision = if self.revision.is_empty() {
+                            String::new()
+                        } else {
+                            format!(" (revision `{}`)", self.revision)
+                        };
+                        println!("{}{revision} {msg}", display(&self.path));
+                        std::io::stdout().flush().unwrap();
+                    }
+                }
             }
             self.text.sender.send(Msg::Pop(old_msg, Some(msg))).unwrap();
         }
     }
 
-    fn update_status(&self, msg: String) {
-        self.text.sender.send(Msg::Status(self.msg(), msg)).unwrap();
-    }
-
     fn failed_test<'a>(
         &self,
         cmd: &str,
@@ -373,7 +383,7 @@ impl TestStatus for TextTest {
         &self.path
     }
 
-    fn for_revision(&self, revision: &str) -> Box<dyn TestStatus> {
+    fn for_revision(&self, revision: &str, style: RevisionStyle) -> Box<dyn TestStatus> {
         if !self.first.swap(false, std::sync::atomic::Ordering::Relaxed)
             && self.text.is_quiet_output()
         {
@@ -385,6 +395,7 @@ impl TestStatus for TextTest {
             path: self.path.clone(),
             revision: revision.to_owned(),
             first: AtomicBool::new(false),
+            style,
         };
         self.text.sender.send(Msg::Push(text.msg())).unwrap();
         Box::new(text)
@@ -396,6 +407,7 @@ impl TestStatus for TextTest {
             path: path.to_path_buf(),
             revision: self.revision.clone(),
             first: AtomicBool::new(false),
+            style: RevisionStyle::Show,
         };
         self.text.sender.send(Msg::Push(text.msg())).unwrap();
         Box::new(text)
@@ -416,6 +428,7 @@ impl StatusEmitter for Text {
             path,
             revision: String::new(),
             first: AtomicBool::new(true),
+            style: RevisionStyle::Parent,
         })
     }
 
@@ -917,7 +930,7 @@ impl<const GROUP: bool> TestStatus for PathAndRev<GROUP> {
         &self.path
     }
 
-    fn for_revision(&self, revision: &str) -> Box<dyn TestStatus> {
+    fn for_revision(&self, revision: &str, _style: RevisionStyle) -> Box<dyn TestStatus> {
         Box::new(Self {
             path: self.path.clone(),
             revision: revision.to_owned(),
@@ -946,8 +959,6 @@ impl<const GROUP: bool> TestStatus for PathAndRev<GROUP> {
     fn revision(&self) -> &str {
         &self.revision
     }
-
-    fn update_status(&self, _msg: String) {}
 }
 
 impl<const GROUP: bool> StatusEmitter for Gha<GROUP> {
@@ -1050,18 +1061,16 @@ impl<T: TestStatus, U: TestStatus> TestStatus for (T, U) {
         rev
     }
 
-    fn for_revision(&self, revision: &str) -> Box<dyn TestStatus> {
-        Box::new((self.0.for_revision(revision), self.1.for_revision(revision)))
+    fn for_revision(&self, revision: &str, style: RevisionStyle) -> Box<dyn TestStatus> {
+        Box::new((
+            self.0.for_revision(revision, style),
+            self.1.for_revision(revision, style),
+        ))
     }
 
     fn for_path(&self, path: &Path) -> Box<dyn TestStatus> {
         Box::new((self.0.for_path(path), self.1.for_path(path)))
     }
-
-    fn update_status(&self, msg: String) {
-        self.0.update_status(msg.clone());
-        self.1.update_status(msg)
-    }
 }
 
 impl<T: StatusEmitter, U: StatusEmitter> StatusEmitter for (T, U) {
@@ -1099,8 +1108,8 @@ impl<T: TestStatus + ?Sized> TestStatus for Box<T> {
         (**self).revision()
     }
 
-    fn for_revision(&self, revision: &str) -> Box<dyn TestStatus> {
-        (**self).for_revision(revision)
+    fn for_revision(&self, revision: &str, style: RevisionStyle) -> Box<dyn TestStatus> {
+        (**self).for_revision(revision, style)
     }
 
     fn for_path(&self, path: &Path) -> Box<dyn TestStatus> {
@@ -1115,10 +1124,6 @@ impl<T: TestStatus + ?Sized> TestStatus for Box<T> {
     ) -> Box<dyn Debug + 'a> {
         (**self).failed_test(cmd, stderr, stdout)
     }
-
-    fn update_status(&self, msg: String) {
-        (**self).update_status(msg)
-    }
 }
 
 impl<T: StatusEmitter + ?Sized> StatusEmitter for Box<T> {

diff --git a/src/test_result.rs b/src/test_result.rs
@@ -4,6 +4,7 @@ use crate::{status_emitter::TestStatus, Error};
 use color_eyre::eyre::Result;
 
 /// The possible non-failure results a single test can have.
+#[derive(Debug)]
 pub enum TestOk {
     /// The test passed
     Ok,