Skip to content

fix: improve Windows shell quoting and test environment portability (… #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DataDave-Dev merged 1 commit into from
Jul 6, 2026
Merged

fix: improve Windows shell quoting and test environment portability (… #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions src/becwright/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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."))

Expand Down Expand Up @@ -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."),
)
Expand Down Expand Up @@ -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."),
)
Expand Down
11 changes: 6 additions & 5 deletions tests/test_cli_and_git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ---
Expand Down Expand Up @@ -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")
Comment on lines 173 to 176

@coderabbitai coderabbitai Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Stale comment no longer matches the code.

The comment # \false` always "fails"...still refers to the old shellfalsecommand, but the check value was rewritten tosys.executable -c "import sys; sys.exit(1)"`. Update the comment to reflect the actual mechanism.

As per coding guidelines, "Use comments only for complex code; if the code is self-explanatory, do not comment it, and do not add comments that merely restate the obvious" — a comment must at least remain accurate.

📝 Proposed fix 
-    # `false` always "fails"; an advisory rule must report but not block the commit.
+    # The check exits 1 (always "fails"); an advisory rule must report but not block the commit.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# `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")
# The check exits 1 (always "fails"); an advisory rule must report but not block the commit.
(tmp_path / ".bec" / "rules.yaml").write_text(
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")
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_cli_and_git.py` around lines 173 - 176, The inline comment near
the advisory rule setup is stale and refers to the old false command even though
the check now runs sys.executable with a Python exit(1) snippet. Update or
remove the comment in the test setup around the rules.yaml write in the relevant
test so it accurately describes the current advisory check mechanism and does
not restate outdated behavior.

Source: Coding guidelines

(tmp_path / "a.py").write_text("x = 1\n", encoding="utf-8")
_git(tmp_path, "add", "a.py")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down
6 changes: 4 additions & 2 deletions tests/test_engine_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions tests/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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*\("'
)


Expand All @@ -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():
Expand Down Expand Up @@ -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):
Expand Down
14 changes: 10 additions & 4 deletions tests/test_mcp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import asyncio
import subprocess
import sys
from pathlib import Path

import pytest
Expand All @@ -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


Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down
6 changes: 4 additions & 2 deletions tests/test_report_and_json.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import json
import subprocess
import sys
from pathlib import Path

from becwright import cli, report
Expand All @@ -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")


Expand Down
5 changes: 3 additions & 2 deletions tests/test_staged_content.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Loading