From fa6fdddb63888b44a2b0b972c387b8be6c13f825 Mon Sep 17 00:00:00 2001 From: YasserYG8 Date: Mon, 6 Jul 2026 17:03:51 +0100 Subject: [PATCH] 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):