f8a529e196
All checks were successful
Tests / Declarative: Post Actions passed: 1348
Triage has only ever entered on a failure. Static analysis is the opposite shape - a standing backlog that never fails a build and so never asks anyone for attention. On this instance that backlog is 139 open findings on Ariadne alone, each already naming its file, its line, its rule and what is wrong. That is better-located evidence than the console text the code-repair flow normally mines, and it was being thrown away. This is a second way into the same flow, not a second flow. A scheduled sweep picks one finding and hands it to the existing proposal path, which is unchanged: Hermes returns a patch as data, Ariadne validates it against the file it names, pushes a branch, opens a pull request nobody merges. A finding arriving from outside the build is not a reason to relax the gates that make a proposal worth reading, so it does not. Three deliberate limits. Security hotspots are never fetched: SonarQube models them as needing human review, the quality gate here fails on exactly that condition, and an automation that resolved them would be marking them reviewed without review - defeating the control rather than satisfying it. Findings already marked won't-fix carry a judgement someone made, and reopening it produces pull requests that argue with a person. And the sweep proposes one fix per run by default, because 139 pull requests nobody reads would make the review gate theatre. Selection is by SonarQube's own effort estimate rather than severity: effort is the closest available proxy for the one-anchor change the patch validator can actually check, so a trivial CRITICAL beats an involved MINOR. An unparseable estimate is treated as ineligible, not as free. Off by default. Triage reacts to a failure someone already cares about; this opens pull requests nobody asked for, and that is a decision an operator makes deliberately rather than inherits on upgrade. Branch naming now sanitizes its token, since it arrives from a finding key as well as a build number and a ref is one of the few places where an unexpected character stops being cosmetic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
394 lines
15 KiB
Python
394 lines
15 KiB
Python
"""Tests for recognising mechanically-fixable defects in console evidence."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
|
|
from ariadne.services import hermes_code_defects as module
|
|
|
|
|
|
CFG = {"allowed_path_prefixes": ["ariadne/", "src/"], "allowed_suffixes": [".py", ".go", ".ts"]}
|
|
|
|
|
|
def _bundle(*lines: str) -> dict:
|
|
return {"jenkins": {"console_failures": [{"text": "\n".join(lines)}], "console_tail": ""}}
|
|
|
|
|
|
def test_ruff_diagnostic_is_extracted_whole() -> None:
|
|
found = module.extract_lint_defects(
|
|
_bundle("ariadne/app.py:12:5: E501 Line too long (95 > 88)"), CFG
|
|
)
|
|
assert found == [
|
|
{
|
|
"category": module.LINT_VIOLATION,
|
|
"tool": "ruff",
|
|
"path": "ariadne/app.py",
|
|
"line": 12,
|
|
"rule": "E501",
|
|
"message": "Line too long (95 > 88)",
|
|
}
|
|
]
|
|
|
|
|
|
def test_golangci_diagnostic_is_extracted() -> None:
|
|
found = module.extract_lint_defects(_bundle("src/main.go:9:2: unused variable x (govet)"), CFG)
|
|
assert found[0]["tool"] == "golangci"
|
|
assert (found[0]["path"], found[0]["line"], found[0]["rule"]) == ("src/main.go", 9, "govet")
|
|
|
|
|
|
def test_eslint_diagnostics_inherit_the_preceding_file_header() -> None:
|
|
"""eslint prints the path once, then indented diagnostics beneath it."""
|
|
|
|
found = module.extract_lint_defects(
|
|
_bundle("src/panel.ts", " 12:5 error Unexpected console statement no-console"), CFG
|
|
)
|
|
assert (found[0]["path"], found[0]["line"], found[0]["rule"]) == ("src/panel.ts", 12, "no-console")
|
|
|
|
|
|
def test_a_file_outside_the_write_allowlist_is_dropped() -> None:
|
|
"""Reporting a defect the patcher could never write is worse than silence."""
|
|
|
|
assert module.extract_lint_defects(_bundle("vendor/x.py:1:1: E501 Line too long"), CFG) == []
|
|
assert module.extract_lint_defects(_bundle("ariadne/x.rb:1:1: E501 Line too long"), CFG) == []
|
|
|
|
|
|
def test_duplicate_diagnostics_are_collapsed() -> None:
|
|
line = "ariadne/app.py:12:5: E501 Line too long (95 > 88)"
|
|
assert len(module.extract_lint_defects(_bundle(line, line, line), CFG)) == 1
|
|
|
|
|
|
def test_extraction_is_bounded() -> None:
|
|
lines = [f"ariadne/f{i}.py:{i}:1: E501 Line too long" for i in range(50)]
|
|
assert len(module.extract_lint_defects(_bundle(*lines), CFG)) == module._MAX_DEFECTS
|
|
|
|
|
|
def test_ordinary_console_noise_matches_nothing() -> None:
|
|
noise = _bundle("Running tests...", "OK", "+ ruff check ariadne", "All checks passed!")
|
|
assert module.extract_lint_defects(noise, CFG) == []
|
|
|
|
|
|
def test_never_raises_on_hostile_input() -> None:
|
|
for bad in (None, {}, {"jenkins": None}, {"jenkins": {"console_failures": "no"}}, 7):
|
|
assert module.extract_lint_defects(bad, CFG) == []
|
|
assert module.extract_lint_defects(_bundle("ariadne/a.py:1:1: E1 x"), None) == []
|
|
|
|
|
|
def test_instruction_names_rule_and_line() -> None:
|
|
defects = module.extract_lint_defects(_bundle("ariadne/app.py:12:5: E501 Line too long"), CFG)
|
|
text = module.defect_instruction(defects)
|
|
assert "ariadne/app.py line 12: E501" in text
|
|
assert "do not suppress the rule" in text
|
|
|
|
|
|
def test_no_defects_yields_no_instruction() -> None:
|
|
"""The caller must fall back to the open-ended request, not an empty header."""
|
|
|
|
assert module.defect_instruction([]) == ""
|
|
|
|
|
|
def test_the_patch_prompt_carries_the_linter_diagnosis(monkeypatch) -> None:
|
|
"""The instruction must reach Hermes, not just be computed."""
|
|
|
|
from ariadne.services import hermes_code_prompt
|
|
|
|
bundle = {
|
|
"jenkins": {
|
|
"console_failures": [{"text": "ariadne/app.py:12:5: E501 Line too long (95 > 88)"}],
|
|
"console_tail": "",
|
|
}
|
|
}
|
|
cfg = {
|
|
"owner": "bstein",
|
|
"repo": "ariadne",
|
|
"base_branch": "master",
|
|
"allowed_path_prefixes": ["ariadne/"],
|
|
"allowed_suffixes": [".py"],
|
|
}
|
|
prompt = hermes_code_prompt.build_patch_prompt("ariadne/1", bundle, cfg, {"ariadne/app.py": "x"})
|
|
|
|
assert "ariadne/app.py line 12: E501" in prompt
|
|
assert "do not suppress the rule" in prompt
|
|
|
|
|
|
def test_the_patch_prompt_is_unchanged_without_diagnostics() -> None:
|
|
"""A build with no linter output must fall back to the open request."""
|
|
|
|
from ariadne.services import hermes_code_prompt
|
|
|
|
bundle = {"jenkins": {"console_failures": [{"text": "boom"}], "console_tail": ""}}
|
|
cfg = {"owner": "b", "repo": "r", "base_branch": "main",
|
|
"allowed_path_prefixes": ["ariadne/"], "allowed_suffixes": [".py"]}
|
|
prompt = hermes_code_prompt.build_patch_prompt("a/1", bundle, cfg, {"ariadne/a.py": "x"})
|
|
|
|
assert "A linter reported these violations" not in prompt
|
|
assert "MINIMAL source fix" in prompt
|
|
|
|
|
|
# --- category 2: undefined or misspelled names ---------------------------------
|
|
|
|
|
|
def test_python_nameerror_takes_its_file_from_the_traceback_frame() -> None:
|
|
"""The error line names the symbol; the frame above names the file."""
|
|
|
|
found = module.extract_name_defects(
|
|
_bundle(' File "ariadne/app.py", line 42, in build', "NameError: name 'setttings' is not defined"),
|
|
CFG,
|
|
)
|
|
assert found[0]["symbol"] == "setttings"
|
|
assert (found[0]["path"], found[0]["line"]) == ("ariadne/app.py", 42)
|
|
assert found[0]["category"] == module.UNDEFINED_NAME
|
|
|
|
|
|
def test_import_errors_are_recognised() -> None:
|
|
for text in (
|
|
"ModuleNotFoundError: No module named 'httpx'",
|
|
"ImportError: cannot import name 'safe_error' from 'ariadne.utils.errors'",
|
|
):
|
|
found = module.extract_name_defects(
|
|
_bundle(' File "ariadne/app.py", line 3, in <module>', text), CFG
|
|
)
|
|
assert found, text
|
|
assert found[0]["category"] == module.UNDEFINED_NAME
|
|
|
|
|
|
def test_go_undefined_names_carry_their_own_location() -> None:
|
|
found = module.extract_name_defects(_bundle("src/main.go:12:5: undefined: doThing"), CFG)
|
|
assert (found[0]["symbol"], found[0]["path"], found[0]["line"]) == ("doThing", "src/main.go", 12)
|
|
|
|
|
|
def test_a_name_defect_without_a_known_file_is_dropped() -> None:
|
|
"""With no frame there is no file, so the patcher could not act on it."""
|
|
|
|
assert module.extract_name_defects(_bundle("NameError: name 'x' is not defined"), CFG) == []
|
|
|
|
|
|
def test_name_defects_outside_the_write_allowlist_are_dropped() -> None:
|
|
found = module.extract_name_defects(
|
|
_bundle(' File "vendor/thing.py", line 1, in x', "NameError: name 'y' is not defined"), CFG
|
|
)
|
|
assert found == []
|
|
|
|
|
|
# --- category 3: failing assertions --------------------------------------------
|
|
|
|
|
|
def test_failing_tests_come_from_structured_results() -> None:
|
|
bundle = {
|
|
"jenkins": {
|
|
"console_failures": [],
|
|
"console_tail": "",
|
|
"failed_tests": [
|
|
{
|
|
"name": "test_discount",
|
|
"className": "tests.test_discount",
|
|
"errorDetails": "AssertionError: assert 0.0 == 90.0",
|
|
}
|
|
],
|
|
}
|
|
}
|
|
found = module.extract_assertion_defects(bundle, CFG)
|
|
assert found[0]["test"] == "test_discount"
|
|
assert found[0]["class"] == "tests.test_discount"
|
|
assert "assert 0.0 == 90.0" in found[0]["message"]
|
|
|
|
|
|
def test_an_assertion_defect_carries_no_path() -> None:
|
|
"""Naming the test file would invite weakening the test instead of fixing the cause."""
|
|
|
|
bundle = {"jenkins": {"failed_tests": [{"name": "t", "className": "c", "errorDetails": "boom"}]}}
|
|
assert "path" not in module.extract_assertion_defects(bundle, CFG)[0]
|
|
|
|
|
|
def test_assertion_instruction_forbids_weakening_the_test() -> None:
|
|
bundle = {"jenkins": {"failed_tests": [{"name": "t", "className": "c", "errorDetails": "boom"}]}}
|
|
text = module.defect_instruction(module.extract_assertion_defects(bundle, CFG))
|
|
assert "Fix the code under test" in text
|
|
assert "Do not weaken, skip or delete" in text
|
|
|
|
|
|
def test_malformed_failed_tests_are_skipped() -> None:
|
|
bundle = {"jenkins": {"failed_tests": ["nope", {}, {"name": " "}, {"name": "ok"}]}}
|
|
found = module.extract_assertion_defects(bundle, CFG)
|
|
assert [f["test"] for f in found] == ["ok"]
|
|
|
|
|
|
def test_extract_defects_combines_every_category() -> None:
|
|
bundle = {
|
|
"jenkins": {
|
|
"console_failures": [
|
|
{"text": "ariadne/app.py:12:5: E501 Line too long\n"
|
|
' File "ariadne/app.py", line 42, in build\n'
|
|
"NameError: name 'setttings' is not defined"}
|
|
],
|
|
"console_tail": "",
|
|
"failed_tests": [{"name": "t", "className": "c", "errorDetails": "boom"}],
|
|
}
|
|
}
|
|
cats = {d["category"] for d in module.extract_defects(bundle, CFG)}
|
|
assert cats == {module.LINT_VIOLATION, module.UNDEFINED_NAME, module.FAILING_ASSERTION}
|
|
|
|
|
|
def test_name_and_assertion_extraction_never_raise_on_hostile_input() -> None:
|
|
for bad in (None, 7, {"jenkins": {"console_failures": "no"}}, {"jenkins": {"failed_tests": 5}}):
|
|
assert module.extract_name_defects(bad, CFG) == []
|
|
assert module.extract_assertion_defects(bad, CFG) == []
|
|
assert module.extract_name_defects(_bundle("NameError: name 'x' is not defined"), None) == []
|
|
|
|
|
|
def test_name_extraction_is_bounded() -> None:
|
|
lines = []
|
|
for i in range(40):
|
|
lines.append(f' File "ariadne/f{i}.py", line {i + 1}, in fn')
|
|
lines.append(f"NameError: name 'sym{i}' is not defined")
|
|
assert len(module.extract_name_defects(_bundle(*lines), CFG)) == module._MAX_DEFECTS
|
|
|
|
|
|
def test_duplicate_name_defects_are_collapsed() -> None:
|
|
lines = [' File "ariadne/app.py", line 4, in fn', "NameError: name 'zz' is not defined"] * 3
|
|
assert len(module.extract_name_defects(_bundle(*lines), CFG)) == 1
|
|
|
|
|
|
def test_a_name_defect_with_no_prefixes_configured_is_dropped() -> None:
|
|
"""An empty allowlist must deny, never permit everything."""
|
|
|
|
empty = {"allowed_path_prefixes": [], "allowed_suffixes": [".py"]}
|
|
found = module.extract_name_defects(
|
|
_bundle(' File "ariadne/app.py", line 1, in fn', "NameError: name 'q' is not defined"), empty
|
|
)
|
|
assert found == []
|
|
|
|
|
|
def test_undefined_name_instruction_names_the_symbol() -> None:
|
|
found = module.extract_name_defects(
|
|
_bundle(' File "ariadne/app.py", line 42, in build', "NameError: name 'setttings' is not defined"),
|
|
CFG,
|
|
)
|
|
text = module.defect_instruction(found)
|
|
assert "'setttings' is not defined or not importable" in text
|
|
assert "ariadne/app.py line 42" in text
|
|
|
|
|
|
def test_a_lint_defect_with_a_disallowed_suffix_is_dropped() -> None:
|
|
"""Suffix and prefix are separate gates; both must hold."""
|
|
|
|
cfg = {"allowed_path_prefixes": ["ariadne/"], "allowed_suffixes": [".go"]}
|
|
assert module.extract_lint_defects(_bundle("ariadne/app.py:1:1: E501 Long"), cfg) == []
|
|
|
|
|
|
# --- the operator's control over what Hermes may be asked to fix ---------------
|
|
|
|
|
|
def test_no_setting_means_every_category() -> None:
|
|
"""The default must be obvious rather than silently empty."""
|
|
|
|
assert module.enabled_categories({}) == module.ALL_CATEGORIES
|
|
assert module.enabled_categories({"fix_categories": []}) == module.ALL_CATEGORIES
|
|
assert module.enabled_categories(None) == module.ALL_CATEGORIES
|
|
|
|
|
|
def test_a_category_can_be_switched_off() -> None:
|
|
cfg = {**CFG, "fix_categories": [module.LINT_VIOLATION]}
|
|
assert module.enabled_categories(cfg) == (module.LINT_VIOLATION,)
|
|
|
|
bundle = {
|
|
"jenkins": {
|
|
"console_failures": [{"text": "ariadne/app.py:12:5: E501 Line too long"}],
|
|
"console_tail": "",
|
|
"failed_tests": [{"name": "t", "className": "c", "errorDetails": "boom"}],
|
|
}
|
|
}
|
|
cats = {d["category"] for d in module.extract_defects(bundle, cfg)}
|
|
assert cats == {module.LINT_VIOLATION}
|
|
|
|
|
|
def test_an_unrecognised_category_is_ignored_not_trusted() -> None:
|
|
cfg = {"fix_categories": ["lint_violation", "rewrite_everything"]}
|
|
assert module.enabled_categories(cfg) == (module.LINT_VIOLATION,)
|
|
|
|
|
|
def test_categories_keep_their_declared_order() -> None:
|
|
cfg = {"fix_categories": [module.FAILING_ASSERTION, module.LINT_VIOLATION]}
|
|
assert module.enabled_categories(cfg) == (module.LINT_VIOLATION, module.FAILING_ASSERTION)
|
|
|
|
|
|
SONAR_CFG = {"allowed_path_prefixes": ["ariadne/"], "allowed_suffixes": [".py"]}
|
|
|
|
|
|
def _sonar_bundle(*issues):
|
|
return {"sonarqube": {"project": "ariadne", "issues": list(issues)}}
|
|
|
|
|
|
def test_a_sonarqube_finding_becomes_a_located_defect() -> None:
|
|
"""The finding already names file, line and rule; nothing is inferred."""
|
|
|
|
bundle = _sonar_bundle(
|
|
{
|
|
"path": "ariadne/services/thing.py",
|
|
"line": 160,
|
|
"rule": "python:S1172",
|
|
"message": "Remove the unused function parameter.",
|
|
}
|
|
)
|
|
|
|
assert module.extract_sonar_defects(bundle, SONAR_CFG) == [
|
|
{
|
|
"category": module.SONARQUBE_ISSUE,
|
|
"tool": "sonarqube",
|
|
"path": "ariadne/services/thing.py",
|
|
"line": 160,
|
|
"rule": "python:S1172",
|
|
"message": "Remove the unused function parameter.",
|
|
}
|
|
]
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"issue",
|
|
[
|
|
{"path": "docs/readme.md", "line": 1},
|
|
{"path": "", "line": 1},
|
|
"not-a-dict",
|
|
],
|
|
)
|
|
def test_an_unusable_sonarqube_finding_is_dropped(issue) -> None:
|
|
assert module.extract_sonar_defects(_sonar_bundle(issue), SONAR_CFG) == []
|
|
|
|
|
|
def test_a_bundle_without_findings_yields_nothing() -> None:
|
|
assert module.extract_sonar_defects({}, SONAR_CFG) == []
|
|
assert module.extract_sonar_defects({"sonarqube": "nope"}, SONAR_CFG) == []
|
|
assert module.extract_sonar_defects({"sonarqube": {"issues": None}}, SONAR_CFG) == []
|
|
|
|
|
|
def test_sonarqube_findings_are_capped() -> None:
|
|
issues = [
|
|
{"path": f"ariadne/m{i}.py", "line": i, "rule": "r", "message": "m"} for i in range(40)
|
|
]
|
|
|
|
assert len(module.extract_sonar_defects(_sonar_bundle(*issues), SONAR_CFG)) == 20
|
|
|
|
|
|
def test_the_sonarqube_instruction_forbids_changing_behaviour() -> None:
|
|
"""The build is green; a "fix" that alters behaviour is a regression."""
|
|
|
|
defects = module.extract_sonar_defects(
|
|
_sonar_bundle(
|
|
{"path": "ariadne/a.py", "line": 3, "rule": "python:S1172", "message": "Remove it."}
|
|
),
|
|
SONAR_CFG,
|
|
)
|
|
instruction = module.defect_instruction(defects)
|
|
|
|
assert "ariadne/a.py line 3: python:S1172 Remove it." in instruction
|
|
assert "the build is not failing" in instruction
|
|
assert "Preserve the existing behaviour exactly" in instruction
|
|
assert "do not suppress the rule" in instruction
|
|
|
|
|
|
def test_sonarqube_findings_can_be_disabled_by_category() -> None:
|
|
bundle = _sonar_bundle({"path": "ariadne/a.py", "line": 1, "rule": "r", "message": "m"})
|
|
cfg = {**SONAR_CFG, "fix_categories": ["lint_violation"]}
|
|
|
|
assert module.extract_defects(bundle, cfg) == []
|
|
assert module.extract_defects(bundle, SONAR_CFG)[0]["category"] == module.SONARQUBE_ISSUE
|