Extract and rename Fix and Command (#188)

These internal models can be reused for #160 (fixes for known errors),
so they will no longer be specific to ScopeDoctorGroup

I also wrapped a test in `#[cfg(test)]` so it no longer gets compiled
into the production binary.

Author: rubberduck203

scope/src/doctor/check.rs

@@ -7,8 +7,8 @@ use std::collections::BTreeMap;
 use crate::models::HelpMetadata;
 use crate::prelude::{ActionReport, ActionReportBuilder, ActionTaskReport};
 use crate::shared::prelude::{
-    CaptureError, CaptureOpts, DoctorGroup, DoctorGroupAction, DoctorGroupActionCommand,
-    DoctorGroupCachePath, ExecutionProvider, OutputDestination,
+    CaptureError, CaptureOpts, DoctorCommand, DoctorGroup, DoctorGroupAction, DoctorGroupCachePath,
+    ExecutionProvider, OutputDestination,
 };
 use async_trait::async_trait;
 use derive_builder::Builder;
@@ -460,7 +460,7 @@ impl DefaultDoctorActionRun {
 
     async fn run_check_command(
         &self,
-        action_command: &DoctorGroupActionCommand,
+        action_command: &DoctorCommand,
     ) -> Result<CacheResults, RuntimeError> {
         info!("Evaluating {:?}", action_command);
         let mut action_reports = Vec::new();
@@ -663,13 +663,13 @@ pub(crate) mod tests {
             .check(
                 DoctorGroupActionCheckBuilder::default()
                     .files(None)
-                    .command(Some(DoctorGroupActionCommand::from(vec!["check"])))
+                    .command(Some(DoctorCommand::from(vec!["check"])))
                     .build()
                     .unwrap(),
             )
             .fix(
-                DoctorGroupActionFixBuilder::default()
-                    .command(Some(DoctorGroupActionCommand::from(vec!["fix"])))
+                DoctorFixBuilder::default()
+                    .command(Some(DoctorCommand::from(vec!["fix"])))
                     .build()
                     .unwrap(),
             )
@@ -685,14 +685,14 @@ pub(crate) mod tests {
             .check(
                 DoctorGroupActionCheckBuilder::default()
                     .files(None)
-                    .command(Some(DoctorGroupActionCommand::from(vec!["check"])))
+                    .command(Some(DoctorCommand::from(vec!["check"])))
                     .build()
                     .unwrap(),
             )
             .fix(
-                DoctorGroupActionFixBuilder::default()
-                    .command(Some(DoctorGroupActionCommand::from(vec!["fix"])))
-                    .prompt(Some(DoctorGroupActionFixPrompt {
+                DoctorFixBuilder::default()
+                    .command(Some(DoctorCommand::from(vec!["fix"])))
+                    .prompt(Some(DoctorFixPrompt {
                         text: "do you want to continue?".to_string(),
                         extra_context: Some("additional context here".to_string()),
                     }))
@@ -716,8 +716,8 @@ pub(crate) mod tests {
                     .unwrap(),
             )
             .fix(
-                DoctorGroupActionFixBuilder::default()
-                    .command(Some(DoctorGroupActionCommand::from(vec!["fix"])))
+                DoctorFixBuilder::default()
+                    .command(Some(DoctorCommand::from(vec!["fix"])))
                     .build()
                     .unwrap(),
             )
@@ -1201,7 +1201,7 @@ pub(crate) mod tests {
                 MockGlobWalker::new(),
             );
 
-            let action_commands = DoctorGroupActionCommandBuilder::default()
+            let action_commands = DoctorCommandBuilder::default()
                 .commands(vec!["test -f ~/.somefile".to_string()])
                 .build()
                 .unwrap();

scope/src/shared/models/internal/command.rs

@@ -0,0 +1,77 @@
+use std::path::Path;
+
+use derive_builder::Builder;
+
+use super::extract_command_path;
+
+#[derive(Debug, PartialEq, Clone, Builder)]
+#[builder(setter(into))]
+pub struct DoctorCommand {
+    pub commands: Vec<String>,
+}
+
+impl<T> From<(&Path, Vec<T>)> for DoctorCommand
+where
+    String: for<'a> From<&'a T>,
+{
+    fn from((base_path, command_strings): (&Path, Vec<T>)) -> Self {
+        let commands = command_strings
+            .iter()
+            .map(|s| {
+                let exec: String = s.into();
+                extract_command_path(base_path, &exec)
+            })
+            .collect();
+
+        DoctorCommand { commands }
+    }
+}
+
+#[cfg(test)]
+impl From<Vec<&str>> for DoctorCommand {
+    /// This is only used by some tests and should NOT be used in production code
+    /// because it does not properly pre-pend the command with a base path.
+    fn from(value: Vec<&str>) -> Self {
+        let commands = value.iter().map(|x| x.to_string()).collect();
+        Self { commands }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn from_vec_str() {
+        let input = vec!["echo 'foo'", "false"];
+
+        let actual = DoctorCommand::from(input.clone());
+
+        assert_eq!(
+            DoctorCommand {
+                commands: vec![input[0].to_string(), input[1].to_string(),]
+            },
+            actual
+        )
+    }
+
+    #[test]
+    fn from_path_and_vec_string() {
+        let base_path = Path::new("/foo/bar");
+        let input = vec!["echo 'foo'", "baz/qux", "./qux"];
+
+        let actual =
+            DoctorCommand::from((base_path, input.iter().map(|cmd| cmd.to_string()).collect()));
+
+        assert_eq!(
+            DoctorCommand {
+                commands: vec![
+                    "echo 'foo'".to_string(),
+                    "baz/qux".to_string(),
+                    "/foo/bar/qux".to_string(),
+                ]
+            },
+            actual
+        )
+    }
+}

scope/src/shared/models/internal/doctor_group.rs

@@ -8,70 +8,18 @@ use minijinja::{context, Environment};
 use crate::models::prelude::{ModelMetadata, V1AlphaDoctorGroup};
 use crate::models::HelpMetadata;
 use crate::prelude::{DoctorGroupActionSpec, DoctorInclude};
-use crate::shared::models::internal::extract_command_path;
+use crate::shared::models::internal::{DoctorCommand, DoctorFix, DoctorFixPrompt};
 
 #[derive(Debug, PartialEq, Clone, Builder)]
 #[builder(setter(into))]
 pub struct DoctorGroupAction {
     pub name: String,
     pub description: String,
-    pub fix: DoctorGroupActionFix,
+    pub fix: DoctorFix,
     pub check: DoctorGroupActionCheck,
     pub required: bool,
 }
 
-#[derive(Debug, PartialEq, Clone, Builder)]
-#[builder(setter(into))]
-pub struct DoctorGroupActionFix {
-    #[builder(default)]
-    pub command: Option<DoctorGroupActionCommand>,
-    #[builder(default)]
-    pub help_text: Option<String>,
-    #[builder(default)]
-    pub help_url: Option<String>,
-    #[builder(default)]
-    pub prompt: Option<DoctorGroupActionFixPrompt>,
-}
-
-#[derive(Debug, PartialEq, Clone, Builder)]
-#[builder(setter(into))]
-pub struct DoctorGroupActionFixPrompt {
-    #[builder(default)]
-    pub text: String,
-    #[builder(default)]
-    pub extra_context: Option<String>,
-}
-
-impl DoctorGroupAction {
-    pub fn make_from(
-        name: &str,
-        description: &str,
-        prompt: Option<DoctorGroupActionFixPrompt>,
-        fix_command: Option<Vec<&str>>,
-        check_path: Option<(&str, Vec<&str>)>,
-        check_command: Option<Vec<&str>>,
-    ) -> Self {
-        Self {
-            required: true,
-            name: name.to_string(),
-            description: description.to_string(),
-            fix: DoctorGroupActionFix {
-                command: fix_command.map(DoctorGroupActionCommand::from),
-                help_text: None,
-                help_url: None,
-                prompt,
-            },
-            check: DoctorGroupActionCheck {
-                command: check_command.map(DoctorGroupActionCommand::from),
-                files: check_path.map(|(base, paths)| DoctorGroupCachePath {
-                    base_path: PathBuf::from(base),
-                    paths: crate::shared::convert_to_string(paths),
-                }),
-            },
-        }
-    }
-}
-
 #[derive(Debug, PartialEq, Clone, Builder)]
 #[builder(setter(into))]
 pub struct DoctorGroupCachePath {
@@ -94,40 +42,10 @@ impl From<(&str, Vec<&str>)> for DoctorGroupCachePath {
 #[derive(Debug, PartialEq, Clone, Builder)]
 #[builder(setter(into))]
 pub struct DoctorGroupActionCheck {
-    pub command: Option<DoctorGroupActionCommand>,
+    pub command: Option<DoctorCommand>,
     pub files: Option<DoctorGroupCachePath>,
 }
 
-#[derive(Debug, PartialEq, Clone, Builder)]
-#[builder(setter(into))]
-pub struct DoctorGroupActionCommand {
-    pub commands: Vec<String>,
-}
-
-impl From<Vec<&str>> for DoctorGroupActionCommand {
-    fn from(value: Vec<&str>) -> Self {
-        let commands = value.iter().map(|x| x.to_string()).collect();
-        Self { commands }
-    }
-}
-
-impl<T> From<(&Path, Vec<T>)> for DoctorGroupActionCommand
-where
-    String: for<'a> From<&'a T>,
-{
-    fn from((base_path, command_strings): (&Path, Vec<T>)) -> Self {
-        let commands = command_strings
-            .iter()
-            .map(|s| {
-                let exec: String = s.into();
-                extract_command_path(base_path, &exec)
-            })
-            .collect();
-
-        DoctorGroupActionCommand { commands }
-    }
-}
-
 #[derive(Debug, PartialEq, Clone, Builder)]
 #[builder(setter(into))]
 pub struct DoctorGroup {
@@ -204,10 +122,7 @@ fn parse_action(
         for command in &fix.commands {
             templated_commands.push(substitute_templates(&working_dir, command)?);
         }
-        Some(DoctorGroupActionCommand::from((
-            containing_dir,
-            templated_commands,
-        )))
+        Some(DoctorCommand::from((containing_dir, templated_commands)))
     } else {
         None
     };
@@ -217,10 +132,7 @@ fn parse_action(
         for command in check {
             templated_commands.push(substitute_templates(&working_dir, command)?);
         }
-        Some(DoctorGroupActionCommand::from((
-            containing_dir,
-            templated_commands,
-        )))
+        Some(DoctorCommand::from((containing_dir, templated_commands)))
     } else {
         None
     };
@@ -231,14 +143,12 @@ fn parse_action(
         description: spec_action
             .description
             .unwrap_or_else(|| "default".to_string()),
-        fix: DoctorGroupActionFix {
+        fix: DoctorFix {
             command: fix_command,
-            prompt: spec_action.fix.and_then(|fix| fix.prompt).map(|p| {
-                DoctorGroupActionFixPrompt {
-                    text: p.text,
-                    extra_context: p.extra_context,
-                }
-            }),
+            prompt: spec_action
+                .fix
+                .and_then(|fix| fix.prompt)
+                .map(DoctorFixPrompt::from),
             help_text,
             help_url,
         },
@@ -261,7 +171,7 @@ mod tests {
 
     use crate::shared::models::parse_models_from_string;
     use crate::shared::models::prelude::{
-        DoctorGroupAction, DoctorGroupActionCheck, DoctorGroupActionCommand, DoctorGroupActionFix,
+        DoctorCommand, DoctorFix, DoctorGroupAction, DoctorGroupActionCheck,
     };
     use crate::shared::prelude::DoctorGroupCachePath;
 
@@ -290,18 +200,14 @@ mod tests {
                 name: "1".to_string(),
                 required: false,
                 description: "foo1".to_string(),
-                fix: DoctorGroupActionFix {
-                    command: Some(DoctorGroupActionCommand::from(vec![
-                        "/foo/bar/.scope/fix1.sh"
-                    ])),
+                fix: DoctorFix {
+                    command: Some(DoctorCommand::from(vec!["/foo/bar/.scope/fix1.sh"])),
                     prompt: None,
                     help_text: Some("There is a good way to fix this, maybe...".to_string()),
                     help_url: Some("https://go.example.com/fixit".to_string()),
                 },
                 check: DoctorGroupActionCheck {
-                    command: Some(DoctorGroupActionCommand::from(vec![
-                        "/foo/bar/.scope/foo1.sh"
-                    ])),
+                    command: Some(DoctorCommand::from(vec!["/foo/bar/.scope/foo1.sh"])),
                     files: Some(DoctorGroupCachePath::from((
                         "/foo/bar",
                         vec!["flig/bar/**/*"]
@@ -315,14 +221,14 @@ mod tests {
                 name: "2".to_string(),
                 required: true,
                 description: "foo2".to_string(),
-                fix: DoctorGroupActionFix {
+                fix: DoctorFix {
                     command: None,
                     help_text: None,
                     help_url: None,
                     prompt: None,
                 },
                 check: DoctorGroupActionCheck {
-                    command: Some(DoctorGroupActionCommand::from(vec!["sleep infinity"])),
+                    command: Some(DoctorCommand::from(vec!["sleep infinity"])),
                     files: Some(DoctorGroupCachePath::from(("/foo/bar", vec!["*/*.txt"])))
                 }
             }