test: improve commit and submodule testing

Two of the tests in radicle-git-ext and radicle-surf relied on using
the Git repository itself. This can end up being an issue when
sandboxing is involved.

This improves the situation by being able to generate commit data to
add to a temporary repository.

It also adds a helper for creating a local, temporary submodule for
testing the submodule interactions in the radicle-surf API.

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett

radicle-git-ext/src/commit.rs

@@ -26,47 +26,9 @@ use trailers::{OwnedTrailer, Trailer, Trailers};
 
 use crate::author::Author;
 
-/// A git commit in its object description form, i.e. the output of
-/// `git cat-file` for a commit object.
-#[derive(Debug)]
-pub struct Commit {
-    tree: Oid,
-    parents: Vec<Oid>,
-    author: Author,
-    committer: Author,
-    headers: Headers,
-    message: String,
-    trailers: Vec<OwnedTrailer>,
-}
+pub type Commit = CommitData<Oid, Oid>;
 
 impl Commit {
-    pub fn new<P, I, T>(
-        tree: Oid,
-        parents: P,
-        author: Author,
-        committer: Author,
-        headers: Headers,
-        message: String,
-        trailers: I,
-    ) -> Self
-    where
-        P: IntoIterator<Item = Oid>,
-        I: IntoIterator<Item = T>,
-        OwnedTrailer: From<T>,
-    {
-        let trailers = trailers.into_iter().map(OwnedTrailer::from).collect();
-        let parents = parents.into_iter().collect();
-        Self {
-            tree,
-            parents,
-            author,
-            committer,
-            headers,
-            message,
-            trailers,
-        }
-    }
-
     /// Read the [`Commit`] from the `repo` that is expected to be found at
     /// `oid`.
     pub fn read(repo: &git2::Repository, oid: Oid) -> Result<Self, error::Read> {
@@ -83,14 +45,68 @@ impl Commit {
         Ok(odb.write(ObjectType::Commit, self.to_string().as_bytes())?)
     }
 
-    /// The tree [`Oid`] this commit points to.
-    pub fn tree(&self) -> Oid {
-        self.tree
+    fn verify_for_write(&self, odb: &git2::Odb) -> Result<(), error::Write> {
+        for parent in &self.parents {
+            verify_object(odb, parent, ObjectType::Commit)?;
+        }
+        verify_object(odb, &self.tree, ObjectType::Tree)?;
+
+        Ok(())
+    }
+}
+
+/// A git commit in its object description form, i.e. the output of
+/// `git cat-file` for a commit object.
+#[derive(Debug)]
+pub struct CommitData<Tree, Parent> {
+    tree: Tree,
+    parents: Vec<Parent>,
+    author: Author,
+    committer: Author,
+    headers: Headers,
+    message: String,
+    trailers: Vec<OwnedTrailer>,
+}
+
+impl<Tree, Parent> CommitData<Tree, Parent> {
+    pub fn new<P, I, T>(
+        tree: Tree,
+        parents: P,
+        author: Author,
+        committer: Author,
+        headers: Headers,
+        message: String,
+        trailers: I,
+    ) -> Self
+    where
+        P: IntoIterator<Item = Parent>,
+        I: IntoIterator<Item = T>,
+        OwnedTrailer: From<T>,
+    {
+        let trailers = trailers.into_iter().map(OwnedTrailer::from).collect();
+        let parents = parents.into_iter().collect();
+        Self {
+            tree,
+            parents,
+            author,
+            committer,
+            headers,
+            message,
+            trailers,
+        }
     }
 
-    /// The parent [`Oid`]s of this commit.
-    pub fn parents(&self) -> impl Iterator<Item = Oid> + '_ {
-        self.parents.iter().copied()
+    /// The tree this commit points to.
+    pub fn tree(&self) -> &Tree {
+        &self.tree
+    }
+
+    /// The parents of this commit.
+    pub fn parents(&self) -> impl Iterator<Item = Parent> + '_
+    where
+        Parent: Clone,
+    {
+        self.parents.iter().cloned()
     }
 
     /// The author of this commit, i.e. the header corresponding to `author`.
@@ -136,13 +152,49 @@ impl Commit {
         self.trailers.iter()
     }
 
-    fn verify_for_write(&self, odb: &git2::Odb) -> Result<(), error::Write> {
-        for parent in &self.parents {
-            verify_object(odb, parent, ObjectType::Commit)?;
-        }
-        verify_object(odb, &self.tree, ObjectType::Tree)?;
+    /// Convert the `CommitData::tree` into a value of type `U`. The
+    /// conversion function `f` can be fallible.
+    ///
+    /// For example, `map_tree` can be used to turn raw tree data into
+    /// an `Oid` by writing it to a repository.
+    pub fn map_tree<U, E, F>(self, f: F) -> Result<CommitData<U, Parent>, E>
+    where
+        F: FnOnce(Tree) -> Result<U, E>,
+    {
+        Ok(CommitData {
+            tree: f(self.tree)?,
+            parents: self.parents,
+            author: self.author,
+            committer: self.committer,
+            headers: self.headers,
+            message: self.message,
+            trailers: self.trailers,
+        })
+    }
 
-        Ok(())
+    /// Convert the `CommitData::parents` into a vector containing
+    /// values of type `U`. The conversion function `f` can be
+    /// fallible.
+    ///
+    /// For example, `map_parents` can be used to resolve the `Oid`s
+    /// to their respective `git2::Commit`s.
+    pub fn map_parents<U, E, F>(self, f: F) -> Result<CommitData<Tree, U>, E>
+    where
+        F: FnMut(Parent) -> Result<U, E>,
+    {
+        Ok(CommitData {
+            tree: self.tree,
+            parents: self
+                .parents
+                .into_iter()
+                .map(f)
+                .collect::<Result<Vec<_>, _>>()?,
+            author: self.author,
+            committer: self.committer,
+            headers: self.headers,
+            message: self.message,
+            trailers: self.trailers,
+        })
     }
 }
 

radicle-git-ext/src/commit/headers.rs

@@ -10,6 +10,7 @@ const BEGIN_PGP: &str = "-----BEGIN PGP SIGNATURE-----\n";
 pub struct Headers(pub(super) Vec<(String, String)>);
 
 /// A `gpgsig` signature stored in a [`crate::commit::Commit`].
+#[derive(Debug)]
 pub enum Signature<'a> {
     /// A PGP signature, i.e. starts with `-----BEGIN PGP SIGNATURE-----`.
     Pgp(Cow<'a, str>),

radicle-git-ext/t/Cargo.toml

@@ -32,5 +32,5 @@ features = ["vendored-libgit2"]
 path = ".."
 features = ["serde", "minicbor"]
 
-[dev-dependencies.test-helpers]
+[dependencies.test-helpers]
 path = "../../test/test-helpers"

radicle-git-ext/t/src/commit.rs

@@ -1,10 +1,13 @@
 use std::{io, str::FromStr as _, string::ToString as _};
 
+use proptest::proptest;
 use radicle_git_ext::{
     author::{self, Author},
     commit::{headers::Headers, trailers::OwnedTrailer, Commit},
 };
-use test_helpers::tempdir::WithTmpDir;
+use test_helpers::tempdir::{self, WithTmpDir};
+
+use crate::gen;
 
 const NO_TRAILER: &str = "\
 tree 50d6ef440728217febf9e35716d8b0296608d7f8
@@ -152,27 +155,23 @@ fn test_conversion() {
     assert_eq!(Commit::from_str(UNSIGNED).unwrap().to_string(), UNSIGNED);
 }
 
-#[test]
-fn valid_commits() {
-    let radicle_git = format!(
-        "file://{}",
-        git2::Repository::discover(".").unwrap().path().display()
-    );
-    let repo = WithTmpDir::new(|path| {
-        let repo = git2::Repository::clone(&radicle_git, path)
-            .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?;
-        Ok::<_, io::Error>(repo)
-    })
-    .unwrap();
+proptest! {
+    #[test]
+    fn valid_commits(commits in proptest::collection::vec(gen::commit::commit(), 5..20)) {
+        let repo = tempdir::WithTmpDir::new(|path| {
+            git2::Repository::init(path).map_err(|e| io::Error::new(io::ErrorKind::Other, e))
+        }).unwrap();
+        let commits = gen::commit::write_commits(&repo, commits).unwrap();
+        repo.reference("refs/heads/master", *commits.last().unwrap(), true, "").unwrap();
 
-    let mut walk = repo.revwalk().unwrap();
-    walk.push_head().unwrap();
+        let mut walk = repo.revwalk().unwrap();
+        walk.push_head().unwrap();
 
-    // take the first 20 commits and make sure we can parse them
-    for oid in walk.take(20) {
-        let oid = oid.unwrap();
-        let commit = Commit::read(&repo, oid);
-        assert!(commit.is_ok(), "Oid: {oid}, Error: {commit:?}")
+        for oid in walk.take(20) {
+            let oid = oid.unwrap();
+            let commit = Commit::read(&repo, oid);
+            assert!(commit.is_ok(), "Oid: {oid}, Error: {commit:?}");
+        }
     }
 }
 
@@ -182,6 +181,7 @@ fn write_valid_commit() {
         git2::Repository::init(path).map_err(|err| io::Error::new(io::ErrorKind::Other, err))
     })
     .unwrap();
+
     let author = Author {
         name: "Terry".to_owned(),
         email: "terry.pratchett@proton.mail".to_owned(),

radicle-git-ext/t/src/gen.rs

@@ -5,4 +5,15 @@
 
 //! Provides proptest generators
 
+use proptest::strategy::Strategy;
+
+pub mod commit;
 pub mod urn;
+
+pub fn alphanumeric() -> impl Strategy<Value = String> {
+    "[a-zA-Z0-9_]+"
+}
+
+pub fn alpha() -> impl Strategy<Value = String> {
+    "[a-zA-Z]+"
+}

radicle-git-ext/t/src/gen/commit.rs

@@ -0,0 +1,94 @@
+use std::convert::Infallible;
+
+use proptest::strategy::Strategy;
+use radicle_git_ext::commit::{self, CommitData};
+
+mod author;
+mod headers;
+mod trailers;
+
+pub use author::author;
+pub use headers::headers;
+pub use trailers::{trailer, trailers};
+
+use super::alphanumeric;
+
+pub fn commit() -> impl Strategy<Value = CommitData<TreeData, Infallible>> {
+    (
+        TreeData::gen(),
+        author(),
+        author(),
+        headers(),
+        alphanumeric(),
+        trailers(3),
+    )
+        .prop_map(|(tree, author, committer, headers, message, trailers)| {
+            CommitData::new(tree, vec![], author, committer, headers, message, trailers)
+        })
+}
+
+pub fn write_commits(
+    repo: &git2::Repository,
+    linear: Vec<CommitData<TreeData, Infallible>>,
+) -> Result<Vec<git2::Oid>, commit::error::Write> {
+    let mut parent = None;
+    let mut commits = Vec::new();
+    for commit in linear {
+        let commit = commit.map_tree(|tree| tree.write(repo))?;
+        let commit = match parent {
+            Some(parent) => commit
+                .map_parents::<git2::Oid, Infallible, _>(|_| Ok(parent))
+                .unwrap(),
+            None => commit
+                .map_parents::<git2::Oid, Infallible, _>(|_| unreachable!("no parents"))
+                .unwrap(),
+        };
+        let oid = commit.write(repo)?;
+        commits.push(oid);
+        parent = Some(oid);
+    }
+    Ok(commits)
+}
+
+#[derive(Clone, Debug)]
+pub enum TreeData {
+    Blob { name: String, data: String },
+    Tree { name: String, inner: Vec<TreeData> },
+}
+
+impl TreeData {
+    fn gen() -> impl Strategy<Value = Self> {
+        let leaf =
+            (alphanumeric(), alphanumeric()).prop_map(|(name, data)| Self::Blob { name, data });
+        leaf.prop_recursive(8, 16, 5, |inner| {
+            (proptest::collection::vec(inner, 1..5), alphanumeric())
+                .prop_map(|(inner, name)| Self::Tree { name, inner })
+        })
+    }
+
+    fn write(&self, repo: &git2::Repository) -> Result<git2::Oid, git2::Error> {
+        let mut builder = repo.treebuilder(None)?;
+        self.write_(repo, &mut builder)?;
+        builder.write()
+    }
+
+    fn write_(
+        &self,
+        repo: &git2::Repository,
+        builder: &mut git2::TreeBuilder,
+    ) -> Result<git2::Oid, git2::Error> {
+        match self {
+            Self::Blob { name, data } => {
+                let oid = repo.blob(data.as_bytes())?;
+                builder.insert(name, oid, git2::FileMode::Blob.into())?;
+            }
+            Self::Tree { name, inner } => {
+                for data in inner {
+                    let oid = data.write_(repo, builder)?;
+                    builder.insert(name, oid, git2::FileMode::Tree.into())?;
+                }
+            }
+        }
+        builder.write()
+    }
+}

radicle-git-ext/t/src/gen/commit/author.rs

@@ -0,0 +1,19 @@
+use proptest::strategy::{Just, Strategy};
+use radicle_git_ext::author::{Author, Time};
+
+use crate::gen;
+
+pub fn author() -> impl Strategy<Value = Author> {
+    gen::alphanumeric().prop_flat_map(move |name| {
+        (Just(name), gen::alphanumeric()).prop_flat_map(|(name, domain)| {
+            (Just(name), Just(domain), (0..1000i64)).prop_map(move |(name, domain, time)| {
+                let email = format!("{name}@{domain}");
+                Author {
+                    name,
+                    email,
+                    time: Time::new(time, 0),
+                }
+            })
+        })
+    })
+}

radicle-git-ext/t/src/gen/commit/headers.rs

@@ -0,0 +1,30 @@
+use proptest::{collection, prop_oneof, strategy::Strategy};
+use radicle_git_ext::commit::headers::Headers;
+
+use crate::gen;
+
+pub fn headers() -> impl Strategy<Value = Headers> {
+    collection::vec(prop_oneof![header(), signature()], 0..5).prop_map(|hs| {
+        let mut headers = Headers::new();
+        for (k, v) in hs {
+            headers.push(&k, &v);
+        }
+        headers
+    })
+}
+
+fn header() -> impl Strategy<Value = (String, String)> {
+    (prop_oneof!["test", "foo", "foobar"], gen::alphanumeric())
+}
+
+pub fn signature() -> impl Strategy<Value = (String, String)> {
+    ("gpgsig", prop_oneof![pgp(), ssh()])
+}
+
+pub fn pgp() -> impl Strategy<Value = String> {
+    "-----BEGIN PGP SIGNATURE-----\r?\n([A-Za-z0-9+/=\r\n]+)\r?\n-----END PGP SIGNATURE-----"
+}
+
+pub fn ssh() -> impl Strategy<Value = String> {
+    "-----BEGIN SSH SIGNATURE-----\r?\n([A-Za-z0-9+/=\r\n]+)\r?\n-----END SSH SIGNATURE-----"
+}

radicle-git-ext/t/src/gen/commit/trailers.rs

@@ -0,0 +1,18 @@
+use proptest::{collection, strategy::Strategy};
+use radicle_git_ext::commit::trailers::{OwnedTrailer, Token, Trailer};
+
+use crate::gen;
+
+pub fn trailers(n: usize) -> impl Strategy<Value = Vec<OwnedTrailer>> {
+    collection::vec(trailer(), 0..n)
+}
+
+pub fn trailer() -> impl Strategy<Value = OwnedTrailer> {
+    (gen::alpha(), gen::alphanumeric()).prop_map(|(token, value)| {
+        Trailer {
+            token: Token::try_from(format!("X-{}", token).as_str()).unwrap(),
+            value: value.into(),
+        }
+        .to_owned()
+    })
+}

radicle-git-ext/t/src/lib.rs

@@ -12,3 +12,6 @@ mod commit;
 
 #[cfg(any(test, feature = "test"))]
 pub mod git_ref_format;
+
+#[cfg(any(test, feature = "test"))]
+pub mod repository;

radicle-git-ext/t/src/repository.rs

@@ -0,0 +1,91 @@
+use std::{convert::Infallible, io, path::Path};
+
+use git2::Oid;
+use radicle_git_ext::{commit::CommitData, ref_format::RefString};
+use test_helpers::tempdir::{self, WithTmpDir};
+
+use crate::gen::commit::{self, TreeData};
+
+pub struct Fixture {
+    pub inner: WithTmpDir<git2::Repository>,
+    pub head: Option<git2::Oid>,
+}
+
+/// Initialise a [`git2::Repository`] in a temporary directory.
+///
+/// The provided `commits` will be added to the repository, and the
+/// head commit will be returned.
+pub fn fixture(
+    refname: &RefString,
+    commits: Vec<CommitData<TreeData, Infallible>>,
+) -> io::Result<Fixture> {
+    let repo = tempdir::WithTmpDir::new(|path| git2::Repository::init(path).map_err(io_other))?;
+    let commits = commit::write_commits(&repo, commits).map_err(io_other)?;
+    let head = commits.last().copied();
+
+    if let Some(head) = head {
+        repo.reference(refname.as_str(), head, false, "Initialise repository")
+            .map_err(io_other)?;
+    }
+
+    Ok(Fixture { inner: repo, head })
+}
+
+pub fn bare_fixture(
+    refname: &RefString,
+    commits: Vec<CommitData<TreeData, Infallible>>,
+) -> io::Result<Fixture> {
+    let repo =
+        tempdir::WithTmpDir::new(|path| git2::Repository::init_bare(path).map_err(io_other))?;
+    let commits = commit::write_commits(&repo, commits).map_err(io_other)?;
+    let head = commits.last().copied();
+
+    if let Some(head) = head {
+        repo.reference(refname.as_str(), head, false, "Initialise repository")
+            .map_err(io_other)?;
+    }
+
+    Ok(Fixture { inner: repo, head })
+}
+
+pub fn submodule<'a>(
+    parent: &'a git2::Repository,
+    child: &'a git2::Repository,
+    refname: &RefString,
+    head: Oid,
+    author: &git2::Signature,
+) -> io::Result<git2::Submodule<'a>> {
+    let url = format!("file://{}", child.path().canonicalize()?.display());
+    let mut sub = parent
+        .submodule(url.as_str(), Path::new("submodule"), true)
+        .map_err(io_other)?;
+    sub.open().map_err(io_other)?;
+    sub.clone(Some(&mut git2::SubmoduleUpdateOptions::default()))
+        .map_err(io_other)?;
+    sub.add_to_index(true).map_err(io_other)?;
+    sub.add_finalize().map_err(io_other)?;
+    {
+        let mut ix = parent.index().map_err(io_other)?;
+        let tree = ix.write_tree_to(parent).map_err(io_other)?;
+        let tree = parent.find_tree(tree).map_err(io_other)?;
+        let head = parent.find_commit(head).map_err(io_other)?;
+        parent
+            .commit(
+                Some(refname.as_str()),
+                author,
+                author,
+                "Commit submodule",
+                &tree,
+                &[&head],
+            )
+            .map_err(io_other)?;
+    }
+    Ok(sub)
+}
+
+fn io_other<E>(e: E) -> io::Error
+where
+    E: std::error::Error + Send + Sync + 'static,
+{
+    io::Error::new(io::ErrorKind::Other, e)
+}

radicle-surf/Cargo.toml

@@ -33,6 +33,7 @@ base64 = "0.13"
 log = "0.4"
 nonempty = "0.5"
 thiserror = "1.0"
+url = "2.5"
 
 [dependencies.git2]
 version = "0.18.1"
@@ -53,9 +54,6 @@ version = "1"
 features = ["serde_derive"]
 optional = true
 
-[dependencies.url]
-version = "2.5"
-
 [build-dependencies]
 anyhow = "1.0"
 flate2 = "1"

radicle-surf/t/Cargo.toml

@@ -17,6 +17,7 @@ nonempty = "0.5"
 pretty_assertions = "1.3.0"
 proptest = "1"
 serde_json = "1"
+url = "2.5"
 
 [dev-dependencies.git2]
 version = "0.18.1"

radicle-surf/t/src/submodule.rs

@@ -1,21 +1,86 @@
-use std::path::Path;
+use std::{convert::Infallible, path::Path};
 
+use proptest::{collection, proptest};
+use radicle_git_ext::commit::CommitData;
+use radicle_git_ext::ref_format::refname;
+use radicle_git_ext_test::gen;
 use radicle_surf::tree::EntryKind;
+use radicle_surf::{fs, Branch, Repository};
 
-#[test]
-fn test_submodule() {
-    use radicle_git_ext::ref_format::refname;
-    use radicle_surf::{fs, Branch, Repository};
+proptest! {
+    #[test]
+    fn test_submodule(
+        initial in gen::commit::commit(),
+        commits in collection::vec(gen::commit::commit(), 1..5)
+    ) {
+        prop::test_submodule(initial, commits)
+    }
 
-    let repo = Repository::discover(".").unwrap();
-    let branch = Branch::local(refname!("surf/submodule-support"));
-    let dir = repo.root_dir(&branch).unwrap();
-    let platinum = dir
-        .find_entry(&Path::new("radicle-surf/data/git-platinum"), &repo)
-        .unwrap();
-    assert!(matches!(&platinum, fs::Entry::Submodule(module) if module.url().is_some()));
+    #[ignore = "segfault"]
+    #[test]
+    fn test_submodule_bare(
+        initial in gen::commit::commit(),
+        commits in collection::vec(gen::commit::commit(), 1..5)
+    ) {
+        prop::test_submodule_bare(initial, commits)
+    }
 
-    let surf = repo.tree(&branch, &Path::new("radicle-surf/data")).unwrap();
-    let kind = EntryKind::from(platinum);
-    assert!(surf.entries().iter().any(|e| e.entry() == &kind));
+}
+
+mod prop {
+    use radicle_git_ext_test::{gen::commit, repository};
+
+    use super::*;
+
+    pub fn test_submodule(
+        initial: CommitData<commit::TreeData, Infallible>,
+        commits: Vec<CommitData<commit::TreeData, Infallible>>,
+    ) {
+        let refname = refname!("refs/heads/master");
+        let author = git2::Signature::try_from(initial.author()).unwrap();
+
+        let submodule = repository::fixture(&refname, commits).unwrap();
+        let repo = repository::fixture(&refname, vec![initial]).unwrap();
+
+        let head = repo.head.expect("missing initial commit");
+        let sub =
+            repository::submodule(&repo.inner, &submodule.inner, &refname, head, &author).unwrap();
+
+        let repo = Repository::open(repo.inner.path()).unwrap();
+        let branch = Branch::local(refname);
+        let dir = repo.root_dir(&branch).unwrap();
+
+        let platinum = dir.find_entry(&sub.path(), &repo).unwrap();
+        assert!(matches!(&platinum, fs::Entry::Submodule(module) if module.url().is_some()));
+
+        let root = repo.tree(&branch, &Path::new("")).unwrap();
+        let kind = EntryKind::from(platinum);
+        assert!(root.entries().iter().any(|e| e.entry() == &kind));
+    }
+
+    pub fn test_submodule_bare(
+        initial: CommitData<commit::TreeData, Infallible>,
+        commits: Vec<CommitData<commit::TreeData, Infallible>>,
+    ) {
+        let refname = refname!("refs/heads/master");
+        let author = git2::Signature::try_from(initial.author()).unwrap();
+
+        let submodule = repository::fixture(&refname, commits).unwrap();
+        let repo = repository::bare_fixture(&refname, vec![initial]).unwrap();
+
+        let head = repo.head.expect("missing initial commit");
+        let sub =
+            repository::submodule(&repo.inner, &submodule.inner, &refname, head, &author).unwrap();
+
+        let repo = Repository::open(repo.inner.path()).unwrap();
+        let branch = Branch::local(refname);
+        let dir = repo.root_dir(&branch).unwrap();
+
+        let platinum = dir.find_entry(&sub.path(), &repo).unwrap();
+        assert!(matches!(&platinum, fs::Entry::Submodule(module) if module.url().is_some()));
+
+        let root = repo.tree(&branch, &Path::new("")).unwrap();
+        let kind = EntryKind::from(platinum);
+        assert!(root.entries().iter().any(|e| e.entry() == &kind));
+    }
 }

test/test-helpers/src/tempdir.rs

@@ -11,6 +11,7 @@ use std::{
 
 use tempfile::{tempdir, TempDir};
 
+#[derive(Debug)]
 pub struct WithTmpDir<A> {
     _tmp: TempDir,
     inner: A,