claude-code
94f059c1e1
werkator build succeeded after 00:17
werkrevisor 0.10.0: change kind "feature with tests", harmless files, built-in DB schema paths, config version skew. - Main code with purely additive tests (new test methods or asserts) is AI-approvable; any changed or removed test line stays YELLOW. - `[categories] harmless` lists files that do not count as "other files". - DB schema paths are built in and always give at least YELLOW. - Settings unknown to the installed werkrevisor in a changed `.werkrevisor.toml` give YELLOW instead of RED. Details: the PR document under `docs/prs/`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed-on: #7 Co-authored-by: claude-code <claude-code@javagil.de>
208 lines
12 KiB
Python
208 lines
12 KiB
Python
import os
|
|
import tomllib
|
|
import unittest
|
|
|
|
from tests.fakes import ROOT, FakeGitea, FakeModel, config, deps, diff_for, gitea_file, pr_ref
|
|
from werkrevisor.cli import execute
|
|
from werkrevisor.config import build_config, parse_repository_config
|
|
from werkrevisor.diff import parse_unified_diff
|
|
from werkrevisor.redact import Redactor
|
|
from werkrevisor.rules import evaluate_rules, files_from_gitea
|
|
from werkrevisor.verdict import Verdict
|
|
|
|
MAIN = "src/main/java/A.java"
|
|
TEST = "src/test/java/ATest.java"
|
|
DOC = "doc/PR/1.md"
|
|
API = "src/main/resources/api-definition/api.yaml"
|
|
SCHEMA = "src/main/resources/db/changelog/1-create.sql"
|
|
|
|
|
|
def evaluate(files, diff="", **overrides):
|
|
return evaluate_rules(files_from_gitea(files), parse_unified_diff(diff), config(**overrides))
|
|
|
|
|
|
def main_and_test(test_added=(), test_removed=(), **overrides):
|
|
files = [gitea_file(MAIN), gitea_file(TEST, deletions=len(test_removed))]
|
|
diff = diff_for(MAIN, added=["int x;"]) + diff_for(TEST, added=test_added, removed=test_removed)
|
|
return evaluate(files, diff, **overrides)
|
|
|
|
|
|
def weakening(outcome):
|
|
return next((r.text for r in outcome.reasons if r.code == "tests_weakened"), None)
|
|
|
|
|
|
class ChangeKindTest(unittest.TestCase):
|
|
def test_change_kind_matrix(self):
|
|
added = ["@Test void y() { assertThat(1).isOne(); }"]
|
|
changed = {"added": ["var a = new A(2);"], "removed": ["var a = new A(1);"]}
|
|
cases = (
|
|
("docs", [DOC], diff_for(DOC, added=["x"]), "docs_only", True),
|
|
("added tests", [TEST], diff_for(TEST, added=added), "tests_only_added", True),
|
|
("docs and added tests", [DOC, TEST], diff_for(TEST, added=added), "docs_and_tests", True),
|
|
("main code", [MAIN], diff_for(MAIN, added=["x"]), "refactoring", True),
|
|
("main code and docs", [MAIN, DOC], diff_for(MAIN, added=["x"]), "refactoring", True),
|
|
("main code and added tests", [MAIN, TEST], diff_for(TEST, added=added), "feature_with_tests", True),
|
|
("main code, added tests and docs", [MAIN, TEST, DOC], diff_for(TEST, added=added),
|
|
"feature_with_tests", True),
|
|
("tests with a changed line", [TEST], diff_for(TEST, **changed), "tests_modified", False),
|
|
("main code and tests with a changed line", [MAIN, TEST], diff_for(TEST, **changed),
|
|
"feature_with_modified_tests", False),
|
|
("main code and an API spec", [MAIN, API], diff_for(MAIN, added=["x"]), "mixed", False),
|
|
("main code, tests and an other file", [MAIN, TEST, "Makefile"], diff_for(TEST, added=added),
|
|
"mixed", False),
|
|
)
|
|
for name, paths, diff, kind, approvable in cases:
|
|
with self.subTest(name):
|
|
outcome = evaluate([gitea_file(p) for p in paths], diff)
|
|
self.assertEqual(outcome.change_kind, kind)
|
|
self.assertIs(outcome.auto_approvable, approvable)
|
|
self.assertIs(outcome.verdict, Verdict.GREEN if approvable else Verdict.YELLOW)
|
|
|
|
def test_not_approvable_reason_names_the_feature_kind(self):
|
|
outcome = main_and_test(test_added=["@Test void y() {}"], categories={"auto_approvable": ["docs_only"]})
|
|
reason = next(r for r in outcome.reasons if r.code == "category")
|
|
self.assertEqual(reason.text, "change kind 'feature_with_tests' (main code, tests); "
|
|
"AI-approvable are only: docs only")
|
|
|
|
|
|
class FeatureWithTestsFlowTest(unittest.TestCase):
|
|
def test_feature_with_additive_tests_is_green(self):
|
|
gitea = FakeGitea(files=[gitea_file(MAIN), gitea_file(TEST)],
|
|
diff=diff_for(MAIN, added=["int x;"])
|
|
+ diff_for(TEST, added=[" @Test void y() {", " assertThat(a.y()).isEqualTo(2);",
|
|
" }"]))
|
|
model = FakeModel(prompt_files=[MAIN, TEST])
|
|
outcome = execute(config(gate={"yellow_gate": "block"}), pr_ref(), deps(gitea, model), Redactor())
|
|
self.assertIs(outcome.result.verdict, Verdict.GREEN)
|
|
self.assertEqual(outcome.result.change_kind, "feature_with_tests")
|
|
self.assertIn("Check that the added tests cover the changed behavior", model.calls[0])
|
|
|
|
|
|
class TestWeakeningTest(unittest.TestCase):
|
|
def test_new_assert_line_in_an_existing_method_is_additive(self):
|
|
outcome = main_and_test(test_added=[" assertThat(a.y()).isEqualTo(2);"])
|
|
self.assertEqual(outcome.change_kind, "feature_with_tests")
|
|
self.assertIsNone(weakening(outcome))
|
|
|
|
def test_changed_or_removed_test_line_is_yellow(self):
|
|
cases = (
|
|
("modified assert", [" assertThat(x).isNotNull();"], [" assertThat(x).isEqualTo(1);"],
|
|
f"{TEST}:1 removes a test or assertion"),
|
|
("changed setup line", [" var a = new A(2);"], [" var a = new A(1);"],
|
|
f"{TEST}:1 changes an existing test line"),
|
|
("removed test method", [], [" @Test", " void x() {}"], f"{TEST}:1 removes a test or assertion"),
|
|
)
|
|
for name, added, removed, finding in cases:
|
|
with self.subTest(name):
|
|
outcome = main_and_test(test_added=added, test_removed=removed)
|
|
self.assertIs(outcome.verdict, Verdict.YELLOW)
|
|
self.assertEqual(outcome.change_kind, "feature_with_modified_tests")
|
|
self.assertTrue(weakening(outcome).startswith(f"test changes are not purely additive: {finding}"))
|
|
outcome = evaluate([gitea_file(MAIN), gitea_file(TEST, status="deleted", deletions=1)],
|
|
diff_for(MAIN, added=["x"]) + diff_for(TEST, removed=["class ATest {}"], deleted=True))
|
|
self.assertEqual(weakening(outcome), f"test changes are not purely additive: {TEST} deleted")
|
|
|
|
def test_re_added_unchanged_line_is_additive_except_an_assertion(self):
|
|
outcome = main_and_test(test_added=[" @Test void y() {}", "}"], test_removed=["}"])
|
|
self.assertIsNone(weakening(outcome))
|
|
self.assertEqual(outcome.change_kind, "feature_with_tests")
|
|
moved = " assertThat(x).isEqualTo(1);"
|
|
outcome = main_and_test(test_added=[" void y() {", moved, " }"], test_removed=[moved])
|
|
self.assertEqual(weakening(outcome), f"test changes are not purely additive: {TEST}:1 removes a test or assertion")
|
|
|
|
def test_a_moved_line_with_content_counts_as_changed(self):
|
|
line = " var a = new A(1);"
|
|
outcome = main_and_test(test_added=[" @Test void y() {", line, " }"], test_removed=[line])
|
|
self.assertEqual(weakening(outcome), f"test changes are not purely additive: {TEST}:1 changes an existing test line")
|
|
|
|
def test_skip_markers_inside_string_literals_are_test_data(self):
|
|
for line in (' ("@Disabled", "@Ignore"),', """ ('xit("a")', '.skip(')"""):
|
|
with self.subTest(line=line):
|
|
self.assertIsNone(weakening(main_and_test(test_added=[line])))
|
|
|
|
def test_blank_line_removal_is_additive(self):
|
|
self.assertIsNone(weakening(main_and_test(test_added=[" @Test void y() {}"], test_removed=[""])))
|
|
|
|
def test_disabled_test_is_yellow(self):
|
|
for line in ("@Disabled", "@Ignore", "@pytest.mark.skip", '@unittest.skip("flaky")', 'xit("works", () => {})',
|
|
'describe.skip("all", () => {})', 'it.only("this one", () => {})', 'self.skipTest("later")',
|
|
"@pytest.mark.xfail"):
|
|
with self.subTest(line=line):
|
|
outcome = main_and_test(test_added=[line])
|
|
self.assertIs(outcome.verdict, Verdict.YELLOW)
|
|
self.assertEqual(weakening(outcome), f"test changes are not purely additive: {TEST}:1 disables a test")
|
|
|
|
|
|
class HarmlessTest(unittest.TestCase):
|
|
def test_harmless_file_does_not_spoil_the_change_kind(self):
|
|
harmless = {"categories": {"harmless": [".editorconfig"]}}
|
|
for other, kind in ((".editorconfig", "refactoring"), ("Makefile", "mixed")):
|
|
with self.subTest(other=other):
|
|
outcome = evaluate([gitea_file(MAIN), gitea_file(other)], diff_for(MAIN, added=["x"]), **harmless)
|
|
self.assertEqual(outcome.change_kind, kind)
|
|
outcome = evaluate([gitea_file(DOC), gitea_file(".editorconfig")], **harmless)
|
|
self.assertEqual(outcome.change_kind, "docs_only")
|
|
outcome = evaluate([gitea_file(MAIN), gitea_file(TEST), gitea_file(".editorconfig")],
|
|
diff_for(TEST, added=["@Test void y() {}"]), **harmless)
|
|
self.assertEqual(outcome.change_kind, "feature_with_tests")
|
|
|
|
def test_harmless_only_is_approvable(self):
|
|
outcome = evaluate([gitea_file(".editorconfig")], categories={"harmless": [".editorconfig"]})
|
|
self.assertEqual(outcome.change_kind, "harmless_only")
|
|
self.assertTrue(outcome.auto_approvable)
|
|
self.assertIs(outcome.verdict, Verdict.GREEN)
|
|
|
|
def test_harmless_never_hides_risky_paths_or_main_code(self):
|
|
outcome = evaluate([gitea_file(".gitea/workflows/ci.yaml")], categories={"harmless": ["**"]})
|
|
self.assertIs(outcome.verdict, Verdict.YELLOW)
|
|
self.assertIn("risky paths changed: .gitea/workflows/ci.yaml (.gitea/**)", [r.text for r in outcome.reasons])
|
|
outcome = evaluate([gitea_file(MAIN), gitea_file(API)], categories={"harmless": ["**"]})
|
|
self.assertEqual(outcome.categories, {MAIN: "main", API: "api_spec"})
|
|
self.assertEqual(outcome.change_kind, "mixed")
|
|
|
|
def test_harmless_comes_from_host_or_repository_and_defaults_to_empty(self):
|
|
with open(os.path.join(ROOT, "config", "werkprobe.toml"), "rb") as f:
|
|
host = tomllib.load(f)
|
|
self.assertEqual(build_config(host).categories.harmless, ())
|
|
repo = parse_repository_config(b'[categories]\nharmless = [".editorconfig"]\n')
|
|
self.assertEqual(build_config(host, repo).categories.harmless, (".editorconfig",))
|
|
host["categories"]["harmless"] = [".gitignore"]
|
|
self.assertEqual(build_config(host).categories.harmless, (".gitignore",))
|
|
|
|
|
|
class DbSchemaTest(unittest.TestCase):
|
|
PATHS = (SCHEMA, "app/migrations/0002_add_field.py", "src/main/resources/db/migration/V2__x.java",
|
|
"src/main/resources/liquibase/master.xml", "src/main/resources/flyway/V3.sql", "db/schema.changelog.yaml",
|
|
"src/main/resources/db.changelog-master.yaml")
|
|
|
|
def test_db_schema_change_is_yellow(self):
|
|
for path in self.PATHS:
|
|
with self.subTest(path=path):
|
|
outcome = evaluate([gitea_file(path)], hard_yellow={"paths": []})
|
|
self.assertIs(outcome.verdict, Verdict.YELLOW)
|
|
self.assertIn(f"DB schema changed: {path} (built in)", [r.text for r in outcome.reasons])
|
|
|
|
def test_no_layer_removes_the_db_schema_paths(self):
|
|
weakest = {"hard_yellow": {"paths": []}, "categories": {"docs": ["**"], "harmless": ["**"]}}
|
|
outcome = evaluate([gitea_file(SCHEMA)], **weakest)
|
|
self.assertIn(f"DB schema changed: {SCHEMA} (built in)", [r.text for r in outcome.reasons])
|
|
self.assertIs(outcome.verdict, Verdict.YELLOW)
|
|
with open(os.path.join(ROOT, "config", "werkprobe.toml"), "rb") as f:
|
|
host = tomllib.load(f)
|
|
host["hard_yellow"]["paths"] = []
|
|
repo = parse_repository_config(b'[hard_yellow]\npaths = []\n[categories]\ndocs = ["**"]\nharmless = ["**"]\n')
|
|
outcome = evaluate_rules(files_from_gitea([gitea_file(SCHEMA)]), {}, build_config(host, repo))
|
|
self.assertIn(f"DB schema changed: {SCHEMA} (built in)", [r.text for r in outcome.reasons])
|
|
outcome = evaluate([gitea_file("app/migrations/README.md")], **weakest)
|
|
self.assertNotIn("db_schema", [r.code for r in outcome.reasons])
|
|
|
|
def test_renamed_schema_file_counts_with_its_old_path(self):
|
|
f = gitea_file("archive/1-create.txt", status="renamed")
|
|
f["previous_filename"] = SCHEMA
|
|
outcome = evaluate([f], hard_yellow={"paths": []})
|
|
self.assertIn(f"DB schema changed: {SCHEMA} (built in)", [r.text for r in outcome.reasons])
|
|
|
|
|
|
if __name__ == "__main__":
|
|
unittest.main()
|