feat: resolve review comments through the CLI
This commit is contained in:
@@ -128,3 +128,161 @@ fn cli_deletes_only_the_requested_comment_and_preserves_failed_targets() {
|
||||
assert!(!output.status.success());
|
||||
assert!(!storage.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cli_resolves_only_the_requested_comment_and_preserves_original_feedback() {
|
||||
let directory = Directory(
|
||||
std::env::temp_dir().join(format!("vamp-review-cli-resolve-{}", std::process::id())),
|
||||
);
|
||||
fs::create_dir(&directory.0).unwrap();
|
||||
let root = directory.0.canonicalize().unwrap();
|
||||
for name in ["repo", "other"] {
|
||||
let output = Command::new("jj")
|
||||
.args(["git", "init"])
|
||||
.arg(root.join(name))
|
||||
.env("JJ_CONFIG", "/dev/null")
|
||||
.output()
|
||||
.unwrap();
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
}
|
||||
let config = root.join("config.rb");
|
||||
fs::write(
|
||||
&config,
|
||||
"Vamp.configure { |config| config.review_directory = 'reviews' }",
|
||||
)
|
||||
.unwrap();
|
||||
let storage = root.join("reviews");
|
||||
let mut store = Store::open(&storage).unwrap();
|
||||
let mut review = Review::new(root.join("repo"));
|
||||
review.summary = "Keep summary".into();
|
||||
review.verdict = Some(Verdict::ChangesRequested);
|
||||
review.comments = [u64::MAX, 2, 3]
|
||||
.into_iter()
|
||||
.map(|id| Comment {
|
||||
id,
|
||||
body: format!("Original feedback {id}\nKeep this history."),
|
||||
resolved: id == 3,
|
||||
anchor: Anchor {
|
||||
revision: "original-snapshot".into(),
|
||||
path: "old/file.rs".into(),
|
||||
side: Side::Before,
|
||||
start: TextPosition { row: 0, column: 1 },
|
||||
end: TextPosition { row: 1, column: 1 },
|
||||
content: "xe\u{301}👩💻\n界z\n".into(),
|
||||
},
|
||||
})
|
||||
.collect();
|
||||
store.save(&mut review).unwrap();
|
||||
let mut other = Review::new(root.join("other"));
|
||||
other.comments = review.comments.clone();
|
||||
store.save(&mut other).unwrap();
|
||||
let id = review.storage_id.unwrap();
|
||||
let id_text = id.to_string();
|
||||
let cli = |repo: &str, args: &[&str]| {
|
||||
Command::new(env!("CARGO_BIN_EXE_vamp"))
|
||||
.arg("--config")
|
||||
.arg(&config)
|
||||
.arg("--review")
|
||||
.args(args)
|
||||
.arg("--repo")
|
||||
.arg(root.join(repo))
|
||||
.current_dir(&root)
|
||||
.output()
|
||||
.unwrap()
|
||||
};
|
||||
let path = storage.join("reviews.sqlite3");
|
||||
let before = fs::read(&path).unwrap();
|
||||
for (repo, args) in [
|
||||
("repo", vec!["resolve"]),
|
||||
("repo", vec!["resolve", &id_text]),
|
||||
("repo", vec!["resolve", "0", "1"]),
|
||||
("repo", vec!["resolve", &id_text, "0"]),
|
||||
("repo", vec!["resolve", "999", "1"]),
|
||||
("repo", vec!["resolve", &id_text, "1"]),
|
||||
("other", vec!["resolve", &id_text, "2"]),
|
||||
("repo", vec!["resolve", &id_text, "2", "--all"]),
|
||||
] {
|
||||
let output = cli(repo, &args);
|
||||
assert!(!output.status.success(), "{:?}", output);
|
||||
assert_eq!(fs::read(&path).unwrap(), before);
|
||||
}
|
||||
let output = cli(
|
||||
"repo",
|
||||
&["resolve", &id_text, "18446744073709551615", "--json"],
|
||||
);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
|
||||
assert_eq!(json["id"], id);
|
||||
assert_eq!(json["revision"], review.storage_revision + 1);
|
||||
assert_eq!(
|
||||
json["counts"],
|
||||
serde_json::json!({"total": 3, "resolved": 2, "unresolved": 1})
|
||||
);
|
||||
assert_eq!(json["comments"].as_array().unwrap().len(), 1);
|
||||
assert_eq!(json["comments"][0]["id"], 2);
|
||||
let loaded = store.load(&review.root, id).unwrap();
|
||||
let mut expected = review.comments.clone();
|
||||
expected[0].resolved = true;
|
||||
assert_eq!(loaded.comments, expected);
|
||||
assert_eq!(loaded.summary, review.summary);
|
||||
assert_eq!(loaded.verdict, review.verdict);
|
||||
assert_eq!(loaded.reviewed_files, review.reviewed_files);
|
||||
assert!(store.save(&mut review).is_err()); // Stale editors cannot undo resolution.
|
||||
assert_eq!(store.load(&review.root, id).unwrap().comments, expected);
|
||||
|
||||
let output = cli("repo", &["show", &id_text, "--all", "--json"]);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
let all: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
|
||||
assert_eq!(all["comments"].as_array().unwrap().len(), 3);
|
||||
let original = &all["comments"][0];
|
||||
assert_eq!(original["id"], u64::MAX);
|
||||
assert_eq!(original["body"], review.comments[0].body);
|
||||
assert_eq!(original["resolved"], true);
|
||||
assert_eq!(original["anchor"]["revision"], "original-snapshot");
|
||||
assert_eq!(original["anchor"]["path"], "old/file.rs");
|
||||
assert_eq!(
|
||||
original["anchor"]["start"],
|
||||
serde_json::json!({"row": 0, "column": 1})
|
||||
);
|
||||
assert_eq!(original["excerpt"], "e\u{301}👩💻\n界");
|
||||
let output = cli("repo", &["show", &id_text, "--json"]);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
assert_eq!(
|
||||
serde_json::from_slice::<serde_json::Value>(&output.stdout).unwrap(),
|
||||
json
|
||||
);
|
||||
let before = fs::read(&path).unwrap();
|
||||
for comment in ["18446744073709551615", "3"] {
|
||||
let output = cli("repo", &["resolve", &id_text, comment, "--json"]);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
assert_eq!(
|
||||
serde_json::from_slice::<serde_json::Value>(&output.stdout).unwrap(),
|
||||
json
|
||||
);
|
||||
assert_eq!(fs::read(&path).unwrap(), before);
|
||||
}
|
||||
let output = cli("repo", &["list", "--json"]);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
let list: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
|
||||
assert_eq!(list["reviews"].as_array().unwrap().len(), 1);
|
||||
assert_eq!(list["reviews"][0]["counts"], json["counts"]);
|
||||
let output = cli("repo", &["resolve", &id_text, "2"]);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
let text = String::from_utf8(output.stdout).unwrap();
|
||||
assert!(!text.contains("Original feedback"));
|
||||
let loaded = store.load(&review.root, id).unwrap();
|
||||
assert_eq!(loaded.storage_revision, review.storage_revision + 2);
|
||||
assert!(loaded.comments.iter().all(|comment| comment.resolved));
|
||||
assert_eq!(
|
||||
store
|
||||
.load(&other.root, other.storage_id.unwrap())
|
||||
.unwrap()
|
||||
.comments,
|
||||
other.comments
|
||||
);
|
||||
drop(store);
|
||||
fs::remove_dir_all(&storage).unwrap();
|
||||
let output = cli("repo", &["resolve", &id_text, "2"]);
|
||||
assert!(!output.status.success(), "{:?}", output);
|
||||
assert!(!storage.exists());
|
||||
}
|
||||
|
||||
@@ -105,3 +105,41 @@ fn existing_storage_is_read_only_and_unchanged() {
|
||||
}
|
||||
std::fs::remove_dir_all(directory).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn comment_mutations_require_positive_explicit_ids_and_reject_all() {
|
||||
for command in ["delete", "resolve"] {
|
||||
for ids in [
|
||||
vec![],
|
||||
vec!["1"],
|
||||
vec!["0", "1"],
|
||||
vec!["-1", "1"],
|
||||
vec!["9223372036854775808", "1"],
|
||||
vec!["all", "1"],
|
||||
vec!["1", "0"],
|
||||
vec!["1", "-1"],
|
||||
vec!["1", "18446744073709551616"],
|
||||
vec!["1", "--json"],
|
||||
] {
|
||||
let mut args = std::iter::once(command).chain(ids).map(OsString::from);
|
||||
assert!(Request::parse(&mut args).is_err());
|
||||
}
|
||||
let mut args = [command, "9223372036854775807", "18446744073709551615"]
|
||||
.into_iter()
|
||||
.map(OsString::from);
|
||||
let mut request = Request::parse(&mut args).unwrap();
|
||||
assert_eq!(request.id, Some(i64::MAX));
|
||||
let (action, id) = request.comment.as_ref().unwrap();
|
||||
assert_eq!(*id, u64::MAX);
|
||||
assert!(matches!(
|
||||
(command, action),
|
||||
("delete", CommentAction::Delete) | ("resolve", CommentAction::Resolve)
|
||||
));
|
||||
assert!(request.option(OsStr::new("--all"), &mut args).is_err());
|
||||
request.option(OsStr::new("--json"), &mut args).unwrap();
|
||||
let mut repo = [OsString::from("checkout")].into_iter();
|
||||
request.option(OsStr::new("--repo"), &mut repo).unwrap();
|
||||
assert!(request.json);
|
||||
assert_eq!(request.repo, Some(PathBuf::from("checkout")));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user