cybercyst/jj
1
0
Fork 0
mirror of https://github.com/martinvonz/jj.git synced 2025-05-05 15:32:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

merge-tools builtin: add property-based testing

Browse Source 
This adds the proptest crate for property-based testing as well as the
proptest-state-machine crate as direct dev dependencies of jj-cli and as
dependencies of the internal testutils crate. 

Within testutils, a `proptest` module provides a reference state machine
which models a repository workspace as a tree-like data structure whose
leaves represent `File`s. The possible transitions of this state machine
are for now limited to the creation of new files (including replacements
of existing files or directories) and deletions of files (pruning the
tree of empty directory nodes). Additional transitions (moving files,
modifying file contents incrementally, ...) and states (symlinks,
submodules, conflicts, ...) may be added in the future.

The `ReferenceStateMachine` trait implementation provides proptest
with strategies for the generation and shrinking of both the initial
state and the transitions that are replayed on it; by shrinking the
transitions rather than another independent reference state, proptest
can search of a failing test input with a minimal diff. This makes this
approach quite suited to VCS problems.

This reference state machine is then applied to the builtin merge-tool's
test suite:
- The initial state is used to build a corresponding `MergedTree`. Its
  ID is used for the fixed "left" tree and serves as the starting point
  for the right tree.
- As transitions are applied, the right tree is updated accordingly.
- Each step of the way, the same test logic as in the manual
  `test_edit_diff_builtin*` tests is run to check that splitting off
  none or all of the changes results in the left or right tree,
  respectively.

Aside from the bug already captured by `*_replace_directory_with_file`,
the property-based test found an independent problem related to file
mode changes of empty files. Regression test seeds for both of these
issues are also checked in. This ensures that others / CI will reproduce
known edge cases deterministically before randomly exploring additional
onwards.
This commit is contained in:
Jonas Greitemann 2025-05-01 14:27:54 +02:00
parent 9b3433e507
commit e000275ba5
8 changed files with 526 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
Cargo.lock generated
View File

@ -2382,6 +2382,8 @@ dependencies = [
"pest",
"pest_derive",
"pollster",
"proptest",
"proptest-state-machine",
"rayon",
"regex",
"rpassword",
@ -3205,6 +3207,35 @@ dependencies = [
"parking_lot",
]
[[package]]
name = "proptest"
version = "1.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "14cae93065090804185d3b75f0bf93b8eeda30c7a9b4a33d3bdb3988d6229e50"
dependencies = [
"bit-set 0.8.0",
"bit-vec 0.8.0",
"bitflags 2.9.0",
"lazy_static",
"num-traits",
"rand",
"rand_chacha",
"rand_xorshift",
"regex-syntax 0.8.5",
"rusty-fork",
"tempfile",
"unarray",
]
[[package]]
name = "proptest-state-machine"
version = "0.3.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e943d140e09d07740fb496487c51fb8eb31c70389ac4a2e9dcd8a0d9fdf228d4"
dependencies = [
"proptest",
]
[[package]]
name = "prost"
version = "0.13.5"
@ -3257,6 +3288,12 @@ dependencies = [
"prost",
]
[[package]]
name = "quick-error"
version = "1.2.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0"
[[package]]
name = "quote"
version = "1.0.40"
@ -3302,6 +3339,15 @@ dependencies = [
"getrandom 0.2.15",
]
[[package]]
name = "rand_xorshift"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d25bf25ec5ae4a3f1b92f929810509a2f53d7dca2f50b794ff57e3face536c8f"
dependencies = [
"rand_core",
]
[[package]]
name = "ratatui"
version = "0.29.0"
@ -3501,6 +3547,18 @@ version = "1.0.20"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "eded382c5f5f786b989652c49544c4877d9f015cc22e145a5ea8ea66c2921cd2"
[[package]]
name = "rusty-fork"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cb3dcc6e454c328bb824492db107ab7c0ae8fcffe4ad210136ef014458c1bc4f"
dependencies = [
"fnv",
"quick-error",
"tempfile",
"wait-timeout",
]
[[package]]
name = "ryu"
version = "1.0.20"
@ -3987,6 +4045,8 @@ dependencies = [
"itertools 0.14.0",
"jj-lib",
"pollster",
"proptest",
"proptest-state-machine",
"rand",
"tempfile",
"toml_edit",
@ -4273,6 +4333,12 @@ dependencies = [
"arrayvec",
]
[[package]]
name = "unarray"
version = "0.1.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "eaea85b334db583fe3274d12b4cd1880032beab409c0d774be044d4480ab9a94"
[[package]]
name = "unicode-bom"
version = "2.0.3"

9
Cargo.toml
View File

@ -83,6 +83,8 @@ pest_derive = "2.8.0"
pollster = "0.4.0"
pretty_assertions = "1.4.1"
proc-macro2 = "1.0.95"
proptest = "1.6.0"
proptest-state-machine = "0.3.1"
prost = "0.13.5"
prost-build = "0.13.5"
quote = "1.0.40"
@ -145,11 +147,16 @@ uninlined_format_args = "warn"
unused_trait_names = "warn"
useless_conversion = "warn"
[profile.dev.package]
# Insta suggests compiling these packages in opt mode for faster testing.
# See https://docs.rs/insta/latest/insta/#optional-faster-runs.
[profile.dev.package]
insta.opt-level = 3
similar.opt-level = 3
# Proptest suggests compiling itself and its RNG in opt mode as well.
# See https://proptest-rs.github.io/proptest/proptest/tips-and-best-practices.html#setting-opt-level
proptest.opt-level = 3
proptest-state-machine.opt-level = 3
rand_chacha.opt-level = 3
[profile.release]
strip = "debuginfo"

2
cli/Cargo.toml
View File

@ -108,6 +108,8 @@ assert_matches = { workspace = true }
async-trait = { workspace = true }
datatest-stable = { workspace = true }
insta = { workspace = true }
proptest = { workspace = true }
proptest-state-machine = { workspace = true }
test-case = { workspace = true }
testutils = { workspace = true }
# https://github.com/rust-lang/cargo/issues/2911#issuecomment-1483256987

8
cli/proptest-regressions/merge_tools/builtin.txt Normal file
View File

@ -0,0 +1,8 @@
# Seeds for failure cases proptest has generated in the past. It is
# automatically read and these particular cases re-run before any
# novel cases are generated.
#
# It is recommended to check this file in to source control so that
# everyone who runs the test benefits from these saved cases.
cc 50055483105a0234d9fb9d2eaf623033f583b31856f280a75f4d3877ea68046f # shrinks to (initial_state, transitions, seen_counter) = (RepoRefState { root: Directory { gamma: RegularFile { contents: "", executable: false } } }, [CreateFile { path: "gamma", contents: "", executable: true }], None)
cc 7f8b4d5e36149675c133884be076c13317673a2fe0484986a2faf667924af673 # shrinks to (initial_state, transitions, seen_counter) = (RepoRefState { root: Directory { gamma: Directory { alpha: RegularFile { contents: "", executable: false } } } }, [CreateFile { path: "gamma", contents: "", executable: false }], None)

117
cli/src/merge_tools/builtin.rs
View File

@ -639,8 +639,13 @@ mod tests {
use jj_lib::matchers::EverythingMatcher;
use jj_lib::merge::MergedTreeValue;
use jj_lib::repo::Repo as _;
use proptest_state_machine::prop_state_machine;
use proptest_state_machine::StateMachineTest;
use testutils::proptest::RepoRefState;
use testutils::proptest::Transition;
use testutils::repo_path;
use testutils::repo_path_component;
use testutils::write_file;
use testutils::TestRepo;
use super::*;
@ -1416,4 +1421,116 @@ mod tests {
]
"#);
}
prop_state_machine! {
#[test]
fn test_edit_diff_builtin_proptest(sequential 1..20 => EditDiffBuiltinPropTest);
}
struct EditDiffBuiltinPropTest {
test_repo: TestRepo,
left_tree_id: MergedTreeId,
right_tree_id: MergedTreeId,
}
impl StateMachineTest for EditDiffBuiltinPropTest {
type SystemUnderTest = EditDiffBuiltinPropTest;
type Reference = RepoRefState;
fn init_test(ref_state: &RepoRefState) -> Self::SystemUnderTest {
use testutils::proptest::File::*;
let test_repo = TestRepo::init();
let store = test_repo.repo.store();
let mut tree_builder = MergedTreeBuilder::new(store.empty_merged_tree_id());
for (path, file) in ref_state.files() {
match file {
RegularFile {
contents,
executable,
} => {
let id = write_file(store, &path, contents);
tree_builder.set_or_remove(
path,
Merge::resolved(Some(TreeValue::File {
id: id.clone(),
executable: *executable,
})),
);
}
}
}
let left_tree_id = tree_builder.write_tree(store).unwrap();
let right_tree_id = left_tree_id.clone();
Self {
test_repo,
left_tree_id,
right_tree_id,
}
}
fn apply(
mut state: Self::SystemUnderTest,
_ref_state: &RepoRefState,
transition: Transition,
) -> Self::SystemUnderTest {
let store = state.test_repo.repo.store();
let mut tree_builder = MergedTreeBuilder::new(state.right_tree_id);
match transition {
Transition::CreateFile {
path,
contents,
executable,
} => {
let id = write_file(store, &path, &contents);
tree_builder.set_or_remove(
path,
Merge::resolved(Some(TreeValue::File {
id: id.clone(),
executable,
})),
);
}
Transition::DeleteFile { path } => {
tree_builder.set_or_remove(path, Merge::resolved(None));
}
}
state.right_tree_id = tree_builder.write_tree(store).unwrap();
state
}
fn check_invariants(state: &Self::SystemUnderTest, _ref_state: &RepoRefState) {
let store = state.test_repo.repo.store();
let left_tree = store.get_root_tree(&state.left_tree_id).unwrap();
let right_tree = store.get_root_tree(&state.right_tree_id).unwrap();
let store = state.test_repo.repo.store();
let (changed_files, files) = make_diff(store, &left_tree, &right_tree);
let no_changes_tree_id =
apply_diff(store, &left_tree, &right_tree, &changed_files, &files);
let no_changes_tree = store.get_root_tree(&no_changes_tree_id).unwrap();
assert_eq!(
no_changes_tree.id(),
state.left_tree_id,
"no-changes tree was different",
);
let mut files = files;
for file in &mut files {
file.toggle_all();
}
let all_changes_tree_id =
apply_diff(store, &left_tree, &right_tree, &changed_files, &files);
let all_changes_tree = store.get_root_tree(&all_changes_tree_id).unwrap();
assert_eq!(
all_changes_tree.id(),
state.right_tree_id,
"all-changes tree was different",
);
}
}
}

2
lib/testutils/Cargo.toml
View File

@ -27,6 +27,8 @@ hex = { workspace = true }
itertools = { workspace = true }
jj-lib = { workspace = true, features = ["testing"] }
pollster = { workspace = true }
proptest = { workspace = true }
proptest-state-machine = { workspace = true }
rand = { workspace = true }
tempfile = { workspace = true }
toml_edit = { workspace = true }

1
lib/testutils/src/lib.rs
View File

@ -74,6 +74,7 @@ use tempfile::TempDir;
use crate::test_backend::TestBackendFactory;
pub mod git;
pub mod proptest;
pub mod test_backend;
// TODO: Consider figuring out a way to make `GitBackend` and `git(1)` calls in

322
lib/testutils/src/proptest.rs Normal file
View File

@ -0,0 +1,322 @@
// Copyright 2025 The Jujutsu Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
use std::collections::BTreeMap;
use std::fmt::Debug;
use itertools::Either;
use itertools::Itertools as _;
use jj_lib::repo_path::RepoPath;
use jj_lib::repo_path::RepoPathBuf;
use jj_lib::repo_path::RepoPathComponentBuf;
use proptest::collection::btree_map;
use proptest::collection::vec;
use proptest::prelude::*;
use proptest::sample::select;
use proptest_state_machine::ReferenceStateMachine;
#[derive(Debug, Clone)]
pub struct RepoRefState {
root: Tree,
}
#[derive(Debug, Clone)]
pub enum File {
RegularFile { contents: String, executable: bool },
}
#[derive(Debug, Clone)]
pub enum Transition {
/// Create a file with the given contents at `path`.
///
/// Parent directories are created as necessary, existing files or
/// directories are replaced.
CreateFile {
path: RepoPathBuf,
contents: String,
executable: bool,
},
/// Delete the file at `path`.
///
/// Emptied parent directories are cleaned up (expect repo root).
DeleteFile { path: RepoPathBuf },
}
#[derive(Clone, Default)]
struct Tree {
nodes: BTreeMap<RepoPathComponentBuf, Node>,
}
#[derive(Clone)]
enum Node {
File(File),
Directory(Tree),
}
impl Debug for Tree {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.nodes
.iter()
.fold(&mut f.debug_struct("Directory"), |list, (name, node)| {
list.field(name.as_internal_str(), node)
})
.finish()
}
}
impl Debug for Node {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Node::File(file) => file.fmt(f),
Node::Directory(tree) => tree.fmt(f),
}
}
}
impl Default for Node {
fn default() -> Self {
Node::Directory(Tree::default())
}
}
impl RepoRefState {
pub fn files(&self) -> impl IntoIterator<Item = (RepoPathBuf, &File)> + '_ {
self.root.files(RepoPath::root())
}
}
impl Tree {
fn files(&self, prefix: &RepoPath) -> Vec<(RepoPathBuf, &File)> {
self.nodes
.iter()
.filter_map(move |(name, node)| match node {
Node::File(file) => Some((prefix.join(name), file)),
Node::Directory(_) => None,
})
.chain(
self.nodes
.iter()
.filter_map(move |(name, node)| match node {
Node::File(_) => None,
Node::Directory(tree) => Some((prefix.join(name), tree)),
})
.flat_map(|(path, tree)| tree.files(&path)),
)
.collect()
}
}
fn arb_tree() -> impl Strategy<Value = Tree> {
btree_map(arb_path_component(), arb_node(), 1..8).prop_map(|nodes| Tree { nodes })
}
fn arb_path_component() -> impl Strategy<Value = RepoPathComponentBuf> {
// biased towards naming collisions (alpha-delta) but with the option to
// generate arbitrary UTF-8
"(alpha|beta|gamma|delta|[\\PC&&[^/]]+)".prop_map(|s| RepoPathComponentBuf::new(s).unwrap())
}
fn arb_node() -> impl Strategy<Value = Node> {
let file_leaf = ("[a-z]{0,3}", proptest::bool::ANY).prop_map(|(contents, executable)| {
Node::File(File::RegularFile {
contents,
executable,
})
});
file_leaf.prop_recursive(4, 8, 8, |inner| {
btree_map(arb_path_component(), inner, 1..8)
.prop_map(|nodes| Node::Directory(Tree { nodes }))
})
}
fn arb_extant_dir(root: Tree) -> impl Strategy<Value = RepoPathBuf> {
fn arb_extant_dir_recursive(path: &RepoPath, tree: Tree) -> impl Strategy<Value = RepoPathBuf> {
let subdirs: Vec<_> = tree
.nodes
.into_iter()
.filter_map(|(name, node)| match node {
Node::File(_) => None,
Node::Directory(tree) => Some((path.join(&name), tree)),
})
.collect();
if subdirs.is_empty() {
Just(path.to_owned()).boxed()
} else {
prop_oneof![
Just(path.to_owned()),
select(subdirs)
.prop_flat_map(|(subdir, subtree)| arb_extant_dir_recursive(&subdir, subtree)),
]
.boxed()
}
}
arb_extant_dir_recursive(RepoPath::root(), root)
}
fn arb_extant_file(root: Tree) -> impl Strategy<Value = RepoPathBuf> {
fn arb_extant_file_recursive(
path: &RepoPath,
tree: Tree,
) -> impl Strategy<Value = RepoPathBuf> {
let (files, subdirs): (Vec<_>, Vec<_>) =
tree.nodes
.into_iter()
.partition_map(|(name, node)| match node {
Node::File(_) => Either::Left(path.join(&name)),
Node::Directory(tree) => Either::Right((path.join(&name), tree)),
});
match (&files[..], &subdirs[..]) {
([], []) => unreachable!("directory must not be empty"),
([], _) => select(subdirs)
.prop_flat_map(|(subdir, subtree)| arb_extant_file_recursive(&subdir, subtree))
.boxed(),
(_, []) => select(files).boxed(),
(_, _) => prop_oneof![
select(files),
select(subdirs)
.prop_flat_map(|(subdir, subtree)| arb_extant_file_recursive(&subdir, subtree)),
]
.boxed(),
}
}
arb_extant_file_recursive(RepoPath::root(), root)
}
fn arb_transition_create_file(state: &RepoRefState) -> impl Strategy<Value = Transition> {
arb_extant_dir(state.root.clone())
.prop_flat_map(|dir| {
vec(arb_path_component(), 1..4).prop_map(move |new_path_components| {
let mut file_path = dir.clone();
file_path.extend(new_path_components);
file_path
})
})
.prop_flat_map(|path| {
("[a-z]{0,3}", proptest::bool::ANY).prop_map(move |(contents, executable)| {
Transition::CreateFile {
path: path.clone(),
contents,
executable,
}
})
})
}
fn arb_transition_delete_file(state: &RepoRefState) -> impl Strategy<Value = Transition> {
arb_extant_file(state.root.clone()).prop_map(|path| Transition::DeleteFile { path })
}
impl ReferenceStateMachine for RepoRefState {
type State = Self;
type Transition = Transition;
fn init_state() -> BoxedStrategy<Self::State> {
prop_oneof![
1 => Just(Self {
root: Tree::default()
}),
10 => arb_tree().prop_map(|root| Self { root }),
]
.boxed()
}
fn transitions(state: &Self::State) -> BoxedStrategy<Self::Transition> {
if state.root.nodes.is_empty() {
arb_transition_create_file(state).boxed()
} else {
prop_oneof![
arb_transition_create_file(state),
arb_transition_delete_file(state),
]
.boxed()
}
}
fn apply(mut state: Self::State, transition: &Self::Transition) -> Self::State {
match transition {
Transition::CreateFile {
path,
contents,
executable,
} => {
let mut components = path.components();
let Some(filename) = components.next_back() else {
panic!("file path cannot be empty");
};
let directory = components.fold(&mut state.root, |tree, pc| {
let Node::Directory(tree) = tree
.nodes
.entry(pc.to_owned())
.and_modify(|node| {
if let Node::File { .. } = node {
// replace any files along the way with directories
*node = Node::default();
}
})
.or_default()
else {
panic!("encountered file, expected directory: {pc:?}");
};
tree
});
directory.nodes.insert(
filename.to_owned(),
Node::File(File::RegularFile {
contents: contents.clone(),
executable: *executable,
}),
);
}
Transition::DeleteFile { path } => {
fn delete_recursive(
directory: &mut Tree,
components: &mut jj_lib::repo_path::RepoPathComponentsIter<'_>,
) -> bool {
let component = components.next().expect("trying to delete a directory");
match directory.nodes.get_mut(component) {
Some(Node::File { .. }) => {
assert!(
components.next().is_none(),
"file does not exist: {component:?} is not a directory"
);
directory.nodes.remove(component);
true
}
Some(Node::Directory(tree)) => {
if delete_recursive(tree, components) && tree.nodes.is_empty() {
directory.nodes.remove(component);
true
} else {
false
}
}
None => false,
}
}
delete_recursive(&mut state.root, &mut path.components());
}
}
state
}
}