Repository: dandavison/delta
Pull request #884 (2022-01-04 22:42:05)

Make Config cloneable for DeltaTest, store as Cow<Config> there
Only cloneable when testing, the types git2::Config and
git2::Repository in GitConfig contain C pointers and can't
really be cloned.

WayneD: Nice work!  My rust-novice eyes can follow most of that, and I think the GitConfig work-around seems quite reasonable for the tests.

Patch:
diff --git a/src/config.rs b/src/config.rs
index be944cf..1eb8778 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -57,6 +57,7 @@ fn adapt_wrap_max_lines_argument(arg: String) -> usize {
     }
 }
 
+#[cfg_attr(test, derive(Clone))]
 pub struct Config {
     pub available_terminal_width: usize,
     pub background_color_extends_to_terminal_width: bool,
diff --git a/src/features/side_by_side.rs b/src/features/side_by_side.rs
index fb0a819..ed55b3b 100644
--- a/src/features/side_by_side.rs
+++ b/src/features/side_by_side.rs
@@ -36,7 +36,7 @@ pub use MinusPlusIndex::Plus as Right;
 
 use super::line_numbers::LineNumbersData;
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct Panel {
     pub width: usize,
 }
diff --git a/src/git_config/mod.rs b/src/git_config/mod.rs
index 9b9f11f..2151725 100644
--- a/src/git_config/mod.rs
+++ b/src/git_config/mod.rs
@@ -8,8 +8,6 @@ use std::env;
 #[cfg(test)]
 use std::path::Path;
 
-use crate::fatal;
-
 use lazy_static::lazy_static;
 
 pub struct GitConfig {
@@ -17,10 +15,31 @@ pub struct GitConfig {
     config_from_env_var: HashMap<String, String>,
     pub enabled: bool,
     pub repo: Option<git2::Repository>,
+    // To make GitConfig cloneable when testing (in turn to make Config cloneable):
+    #[cfg(test)]
+    path: std::path::PathBuf,
+}
+
+#[cfg(test)]
+impl Clone for GitConfig {
+    fn clone(&self) -> Self {
+        assert!(self.repo.is_none());
+        GitConfig {
+            // Assumes no test modifies the file pointed to by `path`
+            config: git2::Config::open(&self.path).unwrap(),
+            config_from_env_var: self.config_from_env_var.clone(),
+            enabled: self.enabled,
+            repo: None,
+            path: self.path.clone(),
+        }
+    }
 }
 
 impl GitConfig {
+    #[cfg(not(test))]
     pub fn try_create() -> Option<Self> {
+        use crate::fatal;
+
         let repo = match std::env::current_dir() {
             Ok(dir) => git2::Repository::discover(dir).ok(),
             _ => None,
@@ -45,6 +64,11 @@ impl GitConfig {
         }
     }
 
+    #[cfg(test)]
+    pub fn try_create() -> Option<Self> {
+        unreachable!("GitConfig::try_create() is not available when testing");
+    }
+
     #[cfg(test)]
     pub fn from_path(path: &Path, honor_env_var: bool) -> Self {
         Self {
@@ -56,6 +80,7 @@ impl GitConfig {
             },
             repo: None,
             enabled: true,
+            path: path.into(),
         }
     }
 
diff --git a/src/minusplus.rs b/src/minusplus.rs
index e37c801..1e2e66c 100644
--- a/src/minusplus.rs
+++ b/src/minusplus.rs
@@ -2,7 +2,7 @@ use std::ops::{Index, IndexMut};
 
 /// Represent data related to removed/minus and added/plus lines which
 /// can be indexed with [`MinusPlusIndex::{Plus`](MinusPlusIndex::Plus)`,`[`Minus}`](MinusPlusIndex::Minus).
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct MinusPlus<T> {
     pub minus: T,
     pub plus: T,
diff --git a/src/style.rs b/src/style.rs
index fe0da88..925d7e7 100644
--- a/src/style.rs
+++ b/src/style.rs
@@ -215,6 +215,7 @@ pub fn ansi_term_style_equality(a: ansi_term::Style, b: ansi_term::Style) -> boo
 // TODO: The equality methods were implemented first, and the equality_key
 // methods later. The former should be re-implemented in terms of the latter.
 // But why did the former not address equality of ansi_term::Color::RGB values?
+#[derive(Clone)]
 pub struct AnsiTermStyleEqualityKey {
     attrs_key: (bool, bool, bool, bool, bool, bool, bool, bool),
     foreground_key: Option<(u8, u8, u8, u8)>,
diff --git a/src/wrapping.rs b/src/wrapping.rs
index f1078de..37f7a6f 100644
--- a/src/wrapping.rs
+++ b/src/wrapping.rs
@@ -988,11 +988,11 @@ index 223ca50..e69de29 100644
 
     #[test]
     fn test_alignment_2_lines_vs_3_lines() {
-        let config_1 =
-            || make_config_from_args(&default_wrap_cfg_plus(&["--side-by-side", "--width", "55"]));
+        let config =
+            make_config_from_args(&default_wrap_cfg_plus(&["--side-by-side", "--width", "55"]));
 
         {
-            DeltaTest::with_config(config_1())
+            DeltaTest::with_config(&config)
                 .with_input(&format!(
                     "{}-{}+{}",
                     HUNK_ALIGN_DIFF_HEADER, HUNK_ALIGN_DIFF_SHORT, HUNK_ALIGN_DIFF_LONG
@@ -1007,7 +1007,7 @@ index 223ca50..e69de29 100644
         }
 
         {
-            DeltaTest::with_config(config_1())
+            DeltaTest::with_config(&config)
                 .with_input(&format!(
                     "{}-{}+{}",
                     HUNK_ALIGN_DIFF_HEADER, HUNK_ALIGN_DIFF_LONG, HUNK_ALIGN_DIFF_SHORT
@@ -1023,18 +1023,16 @@ index 223ca50..e69de29 100644
 
     #[test]
     fn test_alignment_1_line_vs_3_lines() {
-        let config_2 = || {
-            make_config_from_args(&default_wrap_cfg_plus(&[
-                "--side-by-side",
-                "--width",
-                "61",
-                "--line-fill-method",
-                "spaces",
-            ]))
-        };
+        let config = make_config_from_args(&default_wrap_cfg_plus(&[
+            "--side-by-side",
+            "--width",
+            "61",
+            "--line-fill-method",
+            "spaces",
+        ]));
 
         {
-            DeltaTest::with_config(config_2())
+            DeltaTest::with_config(&config)
                 .with_input(&format!(
                     "{}-{}+{}",
                     HUNK_ALIGN_DIFF_HEADER, HUNK_ALIGN_DIFF_SHORT, HUNK_ALIGN_DIFF_LONG
@@ -1048,7 +1046,7 @@ index 223ca50..e69de29 100644
         }
 
         {
-            DeltaTest::with_config(config_2())
+            DeltaTest::with_config(&config)
                 .with_input(&format!(
                     "{}-{}+{}",
                     HUNK_ALIGN_DIFF_HEADER, HUNK_ALIGN_DIFF_LONG, HUNK_ALIGN_DIFF_SHORT
@@ -1065,22 +1063,19 @@ index 223ca50..e69de29 100644
     #[test]
     fn test_wrap_max_lines_2() {
         // TODO overriding is not possible, need to change config directly
-        let config_3 = || {
-            let mut config = make_config_from_args(&default_wrap_cfg_plus(&[
-                // "--wrap-max-lines",
-                // "2",
-                "--side-by-side",
-                "--width",
-                "72",
-                "--line-fill-method",
-                "spaces",
-            ]));
-            config.truncation_symbol = ">".into();
-            config
-        };
+        let mut config = make_config_from_args(&default_wrap_cfg_plus(&[
+            // "--wrap-max-lines",
+            // "2",
+            "--side-by-side",
+            "--width",
+            "72",
+            "--line-fill-method",
+            "spaces",
+        ]));
+        config.truncation_symbol = ">".into();
 
         {
-            DeltaTest::with_config(config_3())
+            DeltaTest::with_config(&config)
                 .with_input(&format!(
                     "{}-{}+{}",
                     HUNK_ALIGN_DIFF_HEADER, HUNK_ALIGN_DIFF_SHORT, HUNK_ALIGN_DIFF_LONG
@@ -1094,9 +1089,8 @@ index 223ca50..e69de29 100644
         }
 
         {
-            let mut config = config_3();
             config.wrap_config.max_lines = 2;
-            DeltaTest::with_config(config)
+            DeltaTest::with_config(&config)
                 .with_input(&format!(
                     "{}-{}+{}",
                     HUNK_ALIGN_DIFF_HEADER, HUNK_ALIGN_DIFF_SHORT, HUNK_ALIGN_DIFF_LONG


Test patch:
diff --git a/src/tests/integration_test_utils.rs b/src/tests/integration_test_utils.rs
index 2e7e24c..6c75efd 100644
--- a/src/tests/integration_test_utils.rs
+++ b/src/tests/integration_test_utils.rs
@@ -1,5 +1,6 @@
 #![cfg(test)]
 
+use std::borrow::Cow;
 use std::fs::File;
 use std::io::{BufReader, Write};
 use std::path::Path;
@@ -147,24 +148,24 @@ pub fn delineated_string(txt: &str) -> String {
     top + &nl + txt + &nl + &btm
 }
 
-pub struct DeltaTest {
-    config: config::Config,
+pub struct DeltaTest<'a> {
+    config: Cow<'a, config::Config>,
     calling_process: Option<String>,
     explain_ansi_: bool,
 }
 
-impl DeltaTest {
+impl<'a> DeltaTest<'a> {
     pub fn with_args(args: &[&str]) -> Self {
         Self {
-            config: make_config_from_args(args),
+            config: Cow::Owned(make_config_from_args(args)),
             calling_process: None,
             explain_ansi_: false,
         }
     }
 
-    pub fn with_config(config: config::Config) -> Self {
+    pub fn with_config(config: &'a config::Config) -> Self {
         Self {
-            config: config,
+            config: Cow::Borrowed(config),
             calling_process: None,
             explain_ansi_: false,
         }
@@ -174,7 +175,9 @@ impl DeltaTest {
     where
         F: Fn(&mut config::Config),
     {
-        f(&mut self.config);
+        let mut owned_config = self.config.into_owned();
+        f(&mut owned_config);
+        self.config = Cow::Owned(owned_config);
         self
     }