diff --git a/git-checks/CHANGELOG.md b/git-checks/CHANGELOG.md index 607acaab2136634b678c135df2dfdaac62883150..4be865fb035c621e3da30aa1c957cde6d47b5c16 100644 --- a/git-checks/CHANGELOG.md +++ b/git-checks/CHANGELOG.md @@ -27,6 +27,13 @@ the mode on a changelog file is no longer sufficient. * The `InvalidPaths` check now supports enforcing Windows filename rules. +# v3.9.1 + +## Updated checks + + * The `BadCommits` check now has a `TopicCheck` implementation which is + equivalent, but more efficient, than its `Check` implementation. + # v3.9.0 ## Updated checks diff --git a/git-checks/src/bad_commits.rs b/git-checks/src/bad_commits.rs index f71a59d3daf63cfbfc2bc08b70e729847dfee40a..343faf138d1101ffe55c1c4a8fe1a41c7426f689 100644 --- a/git-checks/src/bad_commits.rs +++ b/git-checks/src/bad_commits.rs @@ -49,6 +49,43 @@ impl Check for BadCommits { } } +impl TopicCheck for BadCommits { + fn name(&self) -> &str { + "bad-commits-topic" + } + + fn check(&self, ctx: &CheckGitContext, topic: &Topic) -> Result { + let rev_list = ctx.git() + .arg("rev-list") + .arg("--reverse") + .arg("--topo-order") + .arg(&topic.sha1.as_str()) + .arg(&format!("^{}", topic.base)) + .output() + .chain_err(|| "failed to construct rev-list command")?; + if !rev_list.status.success() { + bail!(ErrorKind::Git(format!("failed to list all topic refs: {}", + String::from_utf8_lossy(&rev_list.stderr)))); + } + + let refs = String::from_utf8_lossy(&rev_list.stdout); + + Ok(refs.lines() + .map(CommitId::new) + .fold(CheckResult::new(), |mut result, commit| { + if self.bad_commits.contains(&commit) { + result.add_error(format!("commit {} is a known-bad commit that was removed from the \ + server.", + commit)) + .add_alert(format!("commit {} was pushed to the server.", commit), + true); + } + + result + })) + } +} + #[cfg(test)] mod tests { use BadCommits; @@ -104,4 +141,51 @@ mod tests { assert_eq!(result.allowed(), false); assert_eq!(result.pass(), false); } + + #[test] + fn test_bad_commits_topic_good_commit() { + let check = BadCommits::new(&[ + BAD_COMMIT, + ]); + run_topic_check_ok("test_bad_commits_topic_good_commit", GOOD_COMMIT, check); + } + + #[test] + fn test_bad_commits_topic_no_bad_commit() { + let check = BadCommits::new(&[ + // This commit should never exist. + "0000000000000000000000000000000000000000", + ]); + run_topic_check_ok("test_bad_commits_topic_no_bad_commit", BAD_TOPIC, check); + } + + #[test] + fn test_bad_commits_topic_already_in_history() { + let check = BadCommits::new(&[ + // This commit is in the shared history. + FILLER_COMMIT, + ]); + run_topic_check_ok("test_bad_commits_topic_already_in_history", BAD_TOPIC, check); + } + + #[test] + fn test_bad_commits_topic_not_already_in_history() { + let check = BadCommits::new(&[ + // This commit is on the topic being brought in. + BAD_COMMIT, + ]); + let result = run_topic_check("test_bad_commits_topic_not_already_in_history", BAD_TOPIC, check); + + assert_eq!(result.warnings().len(), 0); + assert_eq!(result.alerts().len(), 1); + assert_eq!(result.alerts()[0], + "commit 029a00428913ee915ce5ee7250c023abfbc2aca3 was pushed to the server."); + assert_eq!(result.errors().len(), 1); + assert_eq!(result.errors()[0], + "commit 029a00428913ee915ce5ee7250c023abfbc2aca3 is a known-bad commit that \ + was removed from the server."); + assert_eq!(result.temporary(), false); + assert_eq!(result.allowed(), false); + assert_eq!(result.pass(), false); + } }