From fa6fdddb63888b44a2b0b972c387b8be6c13f825 Mon Sep 17 00:00:00 2001 From: YasserYG8 Date: Mon, 6 Jul 2026 17:03:51 +0100 Subject: [PATCH 1/2] fix: improve Windows shell quoting and test environment portability (fixes #31) --- src/becwright/cli.py | 22 +++++++++++----------- tests/test_cli_and_git.py | 11 ++++++----- tests/test_engine_integration.py | 6 ++++-- tests/test_init.py | 6 +++--- tests/test_mcp.py | 14 ++++++++++---- tests/test_report_and_json.py | 6 ++++-- tests/test_staged_content.py | 5 +++-- 7 files changed, 41 insertions(+), 29 deletions(-) diff --git a/src/becwright/cli.py b/src/becwright/cli.py index b991948..3c60652 100644 --- a/src/becwright/cli.py +++ b/src/becwright/cli.py @@ -506,26 +506,26 @@ def _starter_rules(langs: list[str]) -> list[dict]: js_globs = [g for g in source_globs if g.endswith((".js", ".ts"))] rules.append(dict( id="no-debugger-js", paths=js_globs, severity="blocking", - check="becwright run forbid --pattern '\\bdebugger\\b'", + check="becwright run forbid --pattern \"\\bdebugger\\b\"", intent="Do not leave 'debugger;' in JavaScript/TypeScript code.", why="A forgotten 'debugger' halts execution and should not reach production.")) rules.append(dict( id="no-console-log-js", paths=js_globs, severity="warning", - check="becwright run forbid --pattern 'console\\.log\\s*\\('", + check="becwright run forbid --pattern \"console\\.log\\s*\\(\"", intent="Avoid 'console.log(...)' in JavaScript/TypeScript code.", why="Debug console.log statements clutter production output.")) if "go" in langs: rules.append(dict( id="no-debug-go", paths=["**/*.go"], severity="blocking", - check=r"becwright run forbid --pattern 'fmt\.Println\s*\(|panic\s*\('", + check="becwright run forbid --pattern \"fmt\\.Println\\s*\\(|panic\\s*\\(\"", intent="Do not leave debug output or panic calls in Go code.", why="Debug statements and unexpected panic calls should not reach production.")) if "rust" in langs: rules.append(dict( id="no-debug-rust", paths=["**/*.rs"], severity="blocking", - check=r"becwright run forbid --pattern 'dbg!\s*\(|println!\s*\('", + check="becwright run forbid --pattern \"dbg!\\s*\\(|println!\\s*\\(\"", intent="Do not leave debug output in Rust code.", why="Debug macros and leftover println! calls should not reach production.")) @@ -568,12 +568,12 @@ def _starter_rules(langs: list[str]) -> list[dict]: why="A token in the logs lets anyone with access to them steal a session."), dict(id="no-debugger-js", lang="jsts", severity="blocking", triggers=("debugger",), - check="becwright run forbid --pattern '\\bdebugger\\b'", + check="becwright run forbid --pattern \"\\bdebugger\\b\"", intent="Do not leave 'debugger;' in JavaScript/TypeScript code.", why="A forgotten 'debugger' halts execution and should not reach production."), dict(id="no-console-log-js", lang="jsts", severity="warning", triggers=("console.log",), - check="becwright run forbid --pattern 'console\\.log\\s*\\('", + check="becwright run forbid --pattern \"console\\.log\\s*\\(\"", intent="Avoid 'console.log(...)' in JavaScript/TypeScript code.", why="Debug console.log statements clutter production output."), ) @@ -629,17 +629,17 @@ def _max_lines_cap(text: str) -> int | None: dict(id="conventional-commits", severity="blocking", triggers=("conventional commit", "commit convention", "commits convencionales", "convención de commit", "convencion de commit"), - check=(r"becwright run require --pattern " - r"'^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test)(\(.+\))?!?: '"), + check=(r'becwright run require --pattern ' + r'"^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test)(\(.+\))?!?: "'), intent="Commit messages must follow the Conventional Commits format.", why="A consistent commit format keeps history readable and enables automated changelogs."), dict(id="no-ai-attribution", severity="blocking", triggers=("no ai attribution", "sin atribución de ia", "sin atribucion de ia", "co-authored-by", "generated with", "no menciones a claude", "atribución de ia", "atribucion de ia", "no ai credit"), - check=(r"becwright run forbid --ignore-case --pattern " - r"'co-authored-by:.*(claude|anthropic|gpt|copilot)" - r"|generated with.*(claude|chatgpt|copilot)'"), + check=(r'becwright run forbid --ignore-case --pattern ' + r'"co-authored-by:.*(claude|anthropic|gpt|copilot)' + r'|generated with.*(claude|chatgpt|copilot)"'), intent="Commit messages must not include AI attribution or co-author trailers.", why="Keeps the history free of tool boilerplate; commits stand on their content."), ) diff --git a/tests/test_cli_and_git.py b/tests/test_cli_and_git.py index 14b2792..69bdff1 100644 --- a/tests/test_cli_and_git.py +++ b/tests/test_cli_and_git.py @@ -23,8 +23,9 @@ def _init_repo(tmp_path): def _rules_yaml(module): - check = f'PYTHONPATH="{_SRC}" "{sys.executable}" -m becwright.checks.{module}' - return f'rules:\n - id: no-debug\n paths: ["**/*.py"]\n check: \'{check}\'\n severity: blocking\n' + src_posix = _SRC.as_posix() + check = f'"{sys.executable}" -c "import sys; sys.path.insert(0, \'{src_posix}\'); from becwright.checks.{module} import main; sys.exit(main())"' + return f'rules:\n - id: no-debug\n paths: ["**/*.py"]\n check: |-\n {check}\n severity: blocking\n' # --- git.py --- @@ -171,7 +172,7 @@ def test_check_advisory_never_blocks(tmp_path, monkeypatch, capsys): (tmp_path / ".bec").mkdir() # `false` always "fails"; an advisory rule must report but not block the commit. (tmp_path / ".bec" / "rules.yaml").write_text( - "rules:\n - id: adv\n paths: ['**/*.py']\n check: 'false'\n severity: advisory\n", + f"rules:\n - id: adv\n paths: ['**/*.py']\n check: '\"{sys.executable}\" -c \"import sys; sys.exit(1)\"'\n severity: advisory\n", encoding="utf-8") (tmp_path / "a.py").write_text("x = 1\n", encoding="utf-8") _git(tmp_path, "add", "a.py") @@ -232,7 +233,7 @@ def test_check_does_not_flag_opaque_command(tmp_path, monkeypatch, capsys): (tmp_path / ".bec").mkdir() (tmp_path / ".bec" / "rules.yaml").write_text( 'rules:\n - id: r1\n paths: ["**/*.py"]\n' - " check: 'true'\n severity: blocking\n", encoding="utf-8") + f" check: '\"{sys.executable}\" -c \"import sys; sys.exit(0)\"'\n severity: blocking\n", encoding="utf-8") (tmp_path / "a.py").write_text("x = 1\n", encoding="utf-8") _git(tmp_path, "add", "a.py") monkeypatch.chdir(tmp_path) @@ -282,7 +283,7 @@ def test_check_msg_no_message_rules_is_noop(tmp_path, monkeypatch): _init_repo(tmp_path) (tmp_path / ".bec").mkdir() (tmp_path / ".bec" / "rules.yaml").write_text( - "rules:\n - id: f\n paths: ['**/*.py']\n check: 'true'\n severity: blocking\n", + f"rules:\n - id: f\n paths: ['**/*.py']\n check: '\"{sys.executable}\" -c \"import sys; sys.exit(0)\"'\n severity: blocking\n", encoding="utf-8") msg = tmp_path / "MSG" msg.write_text("anything\n", encoding="utf-8") diff --git a/tests/test_engine_integration.py b/tests/test_engine_integration.py index f941168..aa7b143 100644 --- a/tests/test_engine_integration.py +++ b/tests/test_engine_integration.py @@ -8,7 +8,8 @@ def _check_cmd(module: str) -> str: - return f'PYTHONPATH="{_SRC}" "{sys.executable}" -m becwright.checks.{module}' + src_posix = _SRC.as_posix() + return f'"{sys.executable}" -c "import sys; sys.path.insert(0, \'{src_posix}\'); from becwright.checks.{module} import main; sys.exit(main())"' def _rule() -> Rule: @@ -102,7 +103,8 @@ def test_evaluate_ignores_commit_msg_rules(tmp_path): def test_evaluate_times_out_a_hung_check(tmp_path, monkeypatch): monkeypatch.setenv("BECWRIGHT_CHECK_TIMEOUT", "0.3") (tmp_path / "a.py").write_text("x = 1\n", encoding="utf-8") - hung = Rule(id="hangs", paths=("**/*.py",), check="sleep 5", severity="blocking") + check = f'"{sys.executable}" -c "import time; time.sleep(5)"' + hung = Rule(id="hangs", paths=("**/*.py",), check=check, severity="blocking") result = evaluate([hung], ["a.py"], tmp_path) assert result.per_rule[0].passed is False assert "timed out" in result.per_rule[0].output diff --git a/tests/test_init.py b/tests/test_init.py index 3196f5e..7991983 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -69,7 +69,7 @@ def test_starter_rules_go(): assert debug_rule["paths"] == ["**/*.go"] assert debug_rule["severity"] == "blocking" assert debug_rule["check"] == ( - r"becwright run forbid --pattern 'fmt\.Println\s*\(|panic\s*\('" + r'becwright run forbid --pattern "fmt\.Println\s*\(|panic\s*\("' ) @@ -88,7 +88,7 @@ def test_starter_rules_rust(): assert debug_rule["paths"] == ["**/*.rs"] assert debug_rule["severity"] == "blocking" assert debug_rule["check"] == ( - r"becwright run forbid --pattern 'dbg!\s*\(|println!\s*\('" + r'becwright run forbid --pattern "dbg!\s*\(|println!\s*\("' ) def test_starter_rules_empty(): @@ -120,7 +120,7 @@ def test_render_yaml_parses_and_keeps_forbid(tmp_path): rules = load_rules(p) assert {r.id for r in rules} == {"no-hardcoded-secrets", "no-debugger-js", "no-console-log-js"} dbg = next(r for r in rules if r.id == "no-debugger-js") - assert r"--pattern '\bdebugger\b'" in dbg.check + assert r'--pattern "\bdebugger\b"' in dbg.check def test_render_empty_is_valid(tmp_path): diff --git a/tests/test_mcp.py b/tests/test_mcp.py index f279283..b3cbb33 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -1,5 +1,6 @@ import asyncio import subprocess +import sys from pathlib import Path import pytest @@ -19,11 +20,12 @@ def _repo_with_rule(path): _git(path, "init") _git(path, "config", "user.email", "t@t.t") _git(path, "config", "user.name", "t") - check = f'PYTHONPATH="{_SRC}" python -m becwright.checks.forbid --pattern "breakpoint"' + src_posix = _SRC.as_posix() + check = f'"{sys.executable}" -c "import sys; sys.path.insert(0, \'{src_posix}\'); from becwright.checks.forbid import main; sys.exit(main())" --pattern "breakpoint"' (path / ".bec").mkdir(parents=True) (path / ".bec" / "rules.yaml").write_text( "rules:\n - id: no-bp\n paths: ['**/*.py']\n" - f" check: '{check}'\n severity: blocking\n", encoding="utf-8") + f" check: |-\n {check}\n severity: blocking\n", encoding="utf-8") return path @@ -81,8 +83,10 @@ def test_preview_rule_reports_violations(tmp_path): _repo(tmp_path) (tmp_path / "a.py").write_text("breakpoint()\n", encoding="utf-8") _git(tmp_path, "add", "a.py") + src_posix = _SRC.as_posix() + check = f'"{sys.executable}" -c "import sys; sys.path.insert(0, \'{src_posix}\'); from becwright.checks.forbid import main; sys.exit(main())" --pattern breakpoint' out = mcp_server.preview_rule( - check=f'PYTHONPATH="{_SRC}" python -m becwright.checks.forbid --pattern breakpoint', + check=check, paths=["**/*.py"], all_files=True, path=str(tmp_path)) assert out["matched_files"] == 1 and out["passed"] is False assert "a.py" in out["output"] and out["note"] is None @@ -92,8 +96,10 @@ def test_preview_rule_passes_clean(tmp_path): _repo(tmp_path) (tmp_path / "a.py").write_text("x = 1\n", encoding="utf-8") _git(tmp_path, "add", "a.py") + src_posix = _SRC.as_posix() + check = f'"{sys.executable}" -c "import sys; sys.path.insert(0, \'{src_posix}\'); from becwright.checks.forbid import main; sys.exit(main())" --pattern breakpoint' out = mcp_server.preview_rule( - check=f'PYTHONPATH="{_SRC}" python -m becwright.checks.forbid --pattern breakpoint', + check=check, paths=["**/*.py"], path=str(tmp_path)) assert out["passed"] is True and out["matched_files"] == 1 diff --git a/tests/test_report_and_json.py b/tests/test_report_and_json.py index 291a123..df4124e 100644 --- a/tests/test_report_and_json.py +++ b/tests/test_report_and_json.py @@ -1,5 +1,6 @@ import json import subprocess +import sys from pathlib import Path from becwright import cli, report @@ -21,11 +22,12 @@ def _init_repo(path): def _rule_yaml(tmp_path): - check = f'PYTHONPATH="{_SRC}" python -m becwright.checks.forbid --pattern "\\bdebugger\\b"' + src_posix = _SRC.as_posix() + check = f'"{sys.executable}" -c "import sys; sys.path.insert(0, \'{src_posix}\'); from becwright.checks.forbid import main; sys.exit(main())" --pattern "\\bdebugger\\b"' (tmp_path / ".bec").mkdir(parents=True, exist_ok=True) (tmp_path / ".bec" / "rules.yaml").write_text( "rules:\n - id: no-dbg\n intent: no debugger\n why_it_matters: it halts\n" - f" paths: ['**/*.js']\n check: '{check}'\n severity: blocking\n", + f" paths: ['**/*.js']\n check: |-\n {check}\n severity: blocking\n", encoding="utf-8") diff --git a/tests/test_staged_content.py b/tests/test_staged_content.py index a099957..a7da70d 100644 --- a/tests/test_staged_content.py +++ b/tests/test_staged_content.py @@ -19,8 +19,9 @@ def _init_repo(tmp_path): def _rules_yaml(module): - check = f'PYTHONPATH="{_SRC}" "{sys.executable}" -m becwright.checks.{module}' - return f'rules:\n - id: no-debug\n paths: ["**/*.py"]\n check: \'{check}\'\n severity: blocking\n' + src_posix = _SRC.as_posix() + check = f'"{sys.executable}" -c "import sys; sys.path.insert(0, \'{src_posix}\'); from becwright.checks.{module} import main; sys.exit(main())"' + return f'rules:\n - id: no-debug\n paths: ["**/*.py"]\n check: |-\n {check}\n severity: blocking\n' def _setup(tmp_path): From 30cf43e562af9930f41debcefaf5fc417d3eb5a1 Mon Sep 17 00:00:00 2001 From: YasserYG8 Date: Wed, 8 Jul 2026 15:55:45 +0100 Subject: [PATCH 2/2] feat: support global excludes in rules.yaml config (feat #93) --- schema/rules.schema.json | 5 +++++ src/becwright/rules.py | 16 +++++++++++++--- tests/test_cli_and_git.py | 22 ++++++++++++++++++++++ tests/test_rules.py | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 3 deletions(-) diff --git a/schema/rules.schema.json b/schema/rules.schema.json index d4c4a97..f214e91 100644 --- a/schema/rules.schema.json +++ b/schema/rules.schema.json @@ -11,6 +11,11 @@ "minimum": 1, "description": "Format version of this file; absent means 1. becwright refuses a file stamped newer than it understands." }, + "global_exclude": { + "type": "array", + "items": { "type": "string", "minLength": 1 }, + "description": "Global glob patterns of files to exclude from all file-based rules." + }, "rules": { "type": "array", "description": "The rules (BECs) enforced on every commit.", diff --git a/src/becwright/rules.py b/src/becwright/rules.py index 9608c67..c083f18 100644 --- a/src/becwright/rules.py +++ b/src/becwright/rules.py @@ -42,7 +42,7 @@ def is_advisory(self) -> bool: return self.severity == "advisory" -def _to_rule(raw: dict) -> Rule: +def _to_rule(raw: dict, global_exclude: list[str] | None = None) -> Rule: if not isinstance(raw, dict): raise RulesError(f"each rule must be a mapping, got {type(raw).__name__}.") for field in ("id", "check"): @@ -60,11 +60,16 @@ def _to_rule(raw: dict) -> Rule: f"rule '{raw['id']}': invalid target {target!r} " f"(use one of: {', '.join(_VALID_TARGETS)})." ) + + exclude_list = list(raw.get("exclude", [])) + if global_exclude: + exclude_list.extend(global_exclude) + return Rule( id=raw["id"], paths=tuple(raw.get("paths", [])), check=raw["check"], - exclude=tuple(raw.get("exclude", [])), + exclude=tuple(exclude_list), intent=(raw.get("intent") or "").strip(), why_it_matters=(raw.get("why_it_matters") or "").strip(), rejected_alternatives=tuple(raw.get("rejected_alternatives", [])), @@ -83,7 +88,12 @@ def load_rules(rules_path: Path) -> list[Rule]: if not isinstance(data, dict) or not isinstance(data.get("rules", []), list): raise RulesError(f"{rules_path}: expected a top-level 'rules:' list.") _check_schema_version(data.get("schema_version"), rules_path) - return [_to_rule(r) for r in data["rules"]] if data.get("rules") else [] + + global_exclude = data.get("global_exclude", []) + if not isinstance(global_exclude, list) or not all(isinstance(x, str) for x in global_exclude): + raise RulesError(f"{rules_path}: global_exclude must be a list of glob patterns.") + + return [_to_rule(r, global_exclude) for r in data["rules"]] if data.get("rules") else [] def _check_schema_version(value, rules_path: Path) -> None: diff --git a/tests/test_cli_and_git.py b/tests/test_cli_and_git.py index 69bdff1..da96f7d 100644 --- a/tests/test_cli_and_git.py +++ b/tests/test_cli_and_git.py @@ -519,3 +519,25 @@ def test_mcp_subcommand_without_extra(monkeypatch): # Simulate the 'mcp' extra not being installed: force the import to fail. monkeypatch.setitem(sys.modules, "becwright.mcp_server", None) assert cli.main(["mcp"]) == 2 + + +def test_check_respects_global_exclude(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + (tmp_path / ".bec").mkdir() + + src_posix = _SRC.as_posix() + check = f'"{sys.executable}" -c "import sys; sys.path.insert(0, \'{src_posix}\'); from becwright.checks.forbid import main; sys.exit(main())" --pattern "breakpoint"' + + (tmp_path / ".bec" / "rules.yaml").write_text( + "global_exclude:\n - 'ignored/**'\n" + "rules:\n - id: no-bp\n paths: ['**/*.py']\n" + f" check: |-\n {check}\n severity: blocking\n", encoding="utf-8") + + (tmp_path / "ignored").mkdir() + (tmp_path / "ignored" / "app.py").write_text("breakpoint()\n", encoding="utf-8") + _git(tmp_path, "add", "ignored/app.py") + + monkeypatch.chdir(tmp_path) + assert cli.main(["check"]) == 0 + assert "All good" in capsys.readouterr().out + diff --git a/tests/test_rules.py b/tests/test_rules.py index 64e00db..eaa3e89 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -126,3 +126,35 @@ def test_non_positive_schema_version_raises(tmp_path): path = _write(tmp_path, 'schema_version: 0\nrules:\n - id: r1\n check: "true"\n') with pytest.raises(RulesError, match="schema_version"): load_rules(path) + + +def test_global_exclude_merges_with_rule_exclude(tmp_path): + path = _write( + tmp_path, + 'global_exclude:\n - "vendor/**"\n - "build/**"\n' + 'rules:\n - id: r1\n check: "true"\n exclude: ["local/exclude.py"]\n' + ) + rules = load_rules(path) + assert len(rules) == 1 + assert rules[0].exclude == ("local/exclude.py", "vendor/**", "build/**") + + +def test_global_exclude_not_list_raises(tmp_path): + path = _write( + tmp_path, + 'global_exclude: "not-a-list"\n' + 'rules:\n - id: r1\n check: "true"\n' + ) + with pytest.raises(RulesError, match="global_exclude must be a list"): + load_rules(path) + + +def test_global_exclude_non_string_elements_raises(tmp_path): + path = _write( + tmp_path, + 'global_exclude:\n - 123\n' + 'rules:\n - id: r1\n check: "true"\n' + ) + with pytest.raises(RulesError, match="global_exclude must be a list"): + load_rules(path) +