review-classifier: check that a commit message's claimed changes are in the diff
message-claims-match-diff is the reverse of one-purpose: a README, doc page, test or rename the message names must be in the diff. The rules file then holds the ten-rule cap, so the new-rule merge test appends to the file minus its last entry and no longer depends on how many rules the file holds.
This commit is contained in:
parent
ff1ad2fe31
commit
7aa8441aa1
3 changed files with 19 additions and 4 deletions
|
|
@ -82,3 +82,12 @@ classifications:
|
||||||
does, and nothing unrelated is bundled in.
|
does, and nothing unrelated is bundled in.
|
||||||
polarity: positive
|
polarity: positive
|
||||||
threshold: 65
|
threshold: 65
|
||||||
|
- name: message-claims-match-diff
|
||||||
|
description: >-
|
||||||
|
Everything a commit message says it changes in a file is in the diff:
|
||||||
|
a doc page, a README, a test, a rename or a removal that it names.
|
||||||
|
This is the reverse of one-purpose, which checks that the diff has
|
||||||
|
nothing the messages do not mention. A commit message that names no
|
||||||
|
such change makes the question not applicable.
|
||||||
|
polarity: positive
|
||||||
|
threshold: 65
|
||||||
|
|
|
||||||
|
|
@ -1336,6 +1336,7 @@ classifications:
|
||||||
"tests-with-behaviour",
|
"tests-with-behaviour",
|
||||||
"docs-updated",
|
"docs-updated",
|
||||||
"one-purpose",
|
"one-purpose",
|
||||||
|
"message-claims-match-diff",
|
||||||
]
|
]
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -537,22 +537,27 @@ mod tests {
|
||||||
#[test]
|
#[test]
|
||||||
fn a_new_rule_leaves_every_existing_entry_as_it_was_written() {
|
fn a_new_rule_leaves_every_existing_entry_as_it_was_written() {
|
||||||
let [yml, _] = RULES_PATHS;
|
let [yml, _] = RULES_PATHS;
|
||||||
let before = parse_rules(yml, OWN.as_bytes()).expect("this repo's rules parse");
|
// This repo's file without its last entry: the file may already hold
|
||||||
|
// the cap, and the property under test is how a new entry is appended,
|
||||||
|
// not how many entries the file has today.
|
||||||
|
let last = OWN.rfind("\n - name:").expect("a last entry") + 1;
|
||||||
|
let own = &OWN[..last];
|
||||||
|
let before = parse_rules(yml, own.as_bytes()).expect("this repo's rules parse");
|
||||||
let new = rule(
|
let new = rule(
|
||||||
"no-todo",
|
"no-todo",
|
||||||
"The diff adds a TODO or FIXME comment without a tracker reference next to it, \
|
"The diff adds a TODO or FIXME comment without a tracker reference next to it, \
|
||||||
which leaves the follow-up with nobody to own it.",
|
which leaves the follow-up with nobody to own it.",
|
||||||
);
|
);
|
||||||
let merged = merge(Some(&file(yml, OWN)), vec![new.clone()]).expect("merges");
|
let merged = merge(Some(&file(yml, own)), vec![new.clone()]).expect("merges");
|
||||||
let text = String::from_utf8(merged.content).expect("utf-8");
|
let text = String::from_utf8(merged.content).expect("utf-8");
|
||||||
assert!(text.starts_with(OWN), "{text}");
|
assert!(text.starts_with(own), "{text}");
|
||||||
assert_eq!(merged.sha.as_deref(), Some("b1"));
|
assert_eq!(merged.sha.as_deref(), Some("b1"));
|
||||||
assert_eq!(merged.message, "review classifier: add no-todo");
|
assert_eq!(merged.message, "review classifier: add no-todo");
|
||||||
let after = parse_rules(yml, text.as_bytes()).expect("parses");
|
let after = parse_rules(yml, text.as_bytes()).expect("parses");
|
||||||
assert_eq!(after, [before, vec![new.clone()]].concat());
|
assert_eq!(after, [before, vec![new.clone()]].concat());
|
||||||
assert!(text.lines().all(|l| l.len() <= YAML_WIDTH), "{text}");
|
assert!(text.lines().all(|l| l.len() <= YAML_WIDTH), "{text}");
|
||||||
|
|
||||||
let unterminated = OWN.trim_end();
|
let unterminated = own.trim_end();
|
||||||
let merged = merge(Some(&file(yml, unterminated)), vec![new]).expect("merges");
|
let merged = merge(Some(&file(yml, unterminated)), vec![new]).expect("merges");
|
||||||
let text = String::from_utf8(merged.content).expect("utf-8");
|
let text = String::from_utf8(merged.content).expect("utf-8");
|
||||||
assert!(
|
assert!(
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue