Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 2 additions & 3 deletions examples/.github-webhook-server.yaml
Comment thread
myakove marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ tox:
main: "tests,linting" # Commands for main branch
develop: "tests" # Commands for develop branch
feature/*: ["tests", "quick-lint"] # Array format also supported

# Python version for tox execution
tox-python-version: "3.11"
args: "-p -e py314" # Additional CLI arguments passed to tox
python-version: "3.11" # Python version for tox (also accepts top-level tox-python-version)

# Pre-commit hooks
pre-commit: true
Expand Down
7 changes: 7 additions & 0 deletions webhook_server/config/schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,13 @@ properties:
description: GitHub events to listen to
tox:
type: object
properties:
args:
type: string
description: Additional CLI arguments to pass to tox (e.g. "-p -v")
python-version:
type: string
description: Python version for tox execution (e.g. "3.11")
patternProperties:
"^.*$":
oneOf:
Expand Down
14 changes: 12 additions & 2 deletions webhook_server/libs/github_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -797,8 +797,18 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None:
value="verified-job", return_on_none=True, extra_dict=repository_config
)
_tox = self.config.get_value(value="tox", extra_dict=repository_config)
self.tox: dict[str, str] = _tox if isinstance(_tox, dict) else {}
self.tox_python_version: str = self.config.get_value(value="tox-python-version", extra_dict=repository_config)
tox_config = dict(_tox) if isinstance(_tox, dict) else {}
self.tox_args: str = tox_config.pop("args", "")
has_nested_python_version = "python-version" in tox_config
_tox_python_version_nested = tox_config.pop("python-version", "")
_tox_python_version_legacy = self.config.get_value(value="tox-python-version", extra_dict=repository_config)
self.tox_python_version: str = (
_tox_python_version_nested if has_nested_python_version else _tox_python_version_legacy
)
self.tox: dict[str, str] = tox_config

if not has_nested_python_version and _tox_python_version_legacy:
self.logger.warning("'tox-python-version' is deprecated, use 'python-version' under 'tox' instead")
self.slack_webhook_url: str = self.config.get_value(value="slack-webhook-url", extra_dict=repository_config)

_container = self.config.get_value(value="container", return_on_none={}, extra_dict=repository_config)
Expand Down
3 changes: 3 additions & 0 deletions webhook_server/libs/handlers/runner_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,9 @@ async def run_tox(self, pull_request: PullRequest) -> None:
tests = _tox_tests.replace(" ", "")
cmd += f" -e {tests}"

if self.github_webhook.tox_args:
cmd += f" {self.github_webhook.tox_args}"

check_config = CheckConfig(name=TOX_STR, command=cmd, title="Tox")
await self.run_check(pull_request=pull_request, check_config=check_config)

Expand Down
3 changes: 2 additions & 1 deletion webhook_server/tests/manifests/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,11 @@ repositories:
- issue_comment
- check_run
- status
tox-python-version: "3.8"
tox:
main: all # Run all tests in tox.ini when pull request parent branch is main
dev: testenv1,testenv2 # Run testenv1 and testenv2 tests in tox.ini when pull request parent branch is dev
args: "-x --no-header"
python-version: "3.8"

pre-commit: true # Run pre-commit check

Expand Down
54 changes: 54 additions & 0 deletions webhook_server/tests/test_config_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,60 @@ def test_tox_configuration_flexibility(self, valid_minimal_config: dict[str, Any
finally:
shutil.rmtree(temp_dir)

def test_tox_args_nested_under_tox(
self, valid_minimal_config: dict[str, Any], monkeypatch: pytest.MonkeyPatch
) -> None:
"""Test that args is nested under tox config and loaded by Config."""
# Verify tox-args is NOT a standalone schema property (it lives under tox now)
schema_path = os.path.join(
os.path.dirname(os.path.dirname(__file__)),
"config",
"schema.yaml",
)
with open(schema_path) as fh:
schema = yaml.safe_load(fh)

repo_props = schema["properties"]["repositories"]["additionalProperties"]["properties"]
assert "tox-args" not in repo_props, "tox-args must NOT be a standalone property"

# Verify Config loads tox.args correctly
config = valid_minimal_config.copy()
config["repositories"]["test-repo"]["tox"] = {
"main": "all",
"args": "-p -x",
}

temp_dir = self.create_temp_config_dir_and_data(config)

try:
monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_dir)
config_obj = Config()
tox_config = config_obj.root_data["repositories"]["test-repo"]["tox"]
assert tox_config["main"] == "all"
assert tox_config["args"] == "-p -x"
finally:
shutil.rmtree(temp_dir)

def test_tox_args_schema_enforces_string_type(self) -> None:
"""Test that tox schema has explicit properties block enforcing args as string.

Without an explicit properties block, args falls through to the catch-all
patternProperties which allows arrays. The schema must have an explicit
properties.args with type: string to prevent this.
"""
schema_path = os.path.join(
os.path.dirname(os.path.dirname(__file__)),
"config",
"schema.yaml",
)
with open(schema_path) as fh:
schema = yaml.safe_load(fh)

tox_schema = schema["properties"]["repositories"]["additionalProperties"]["properties"]["tox"]
assert "properties" in tox_schema, "tox schema must have an explicit 'properties' block"
assert "args" in tox_schema["properties"], "tox.properties must include 'args'"
assert tox_schema["properties"]["args"]["type"] == "string", "tox.args must enforce type: string"

def test_protected_branches_flexibility(self, valid_minimal_config: dict[str, Any]) -> None:
"""Test that protected-branches accepts both arrays and objects."""
config = valid_minimal_config.copy()
Expand Down
113 changes: 113 additions & 0 deletions webhook_server/tests/test_repo_data_from_config.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import logging as python_logging
from typing import Any
from unittest.mock import patch


def test_repo_data_from_config_repository_found(process_github_webhook):
process_github_webhook._repo_data_from_config(repository_config={})

Expand All @@ -6,6 +11,9 @@ def test_repo_data_from_config_repository_found(process_github_webhook):
assert process_github_webhook.pypi == {"token": "PYPI TOKEN"}
assert process_github_webhook.verified_job
assert process_github_webhook.tox_python_version == "3.8"
assert process_github_webhook.tox_args == "-x --no-header"
assert "args" not in process_github_webhook.tox, "args key must be popped from tox dict"
assert "python-version" not in process_github_webhook.tox, "python-version key must be popped from tox dict"
assert process_github_webhook.slack_webhook_url == "Slack webhook url"
assert process_github_webhook.container_repository_username == "registry username"
assert process_github_webhook.container_repository_password == "registry_password" # pragma: allowlist secret
Expand All @@ -19,3 +27,108 @@ def test_repo_data_from_config_repository_found(process_github_webhook):
assert process_github_webhook.auto_verified_and_merged_users == ["my[bot]"]
assert process_github_webhook.can_be_merged_required_labels == ["my-label1", "my-label2"]
assert process_github_webhook.minimum_lgtm == 0


def test_tox_python_version_nested_no_deprecation_warning(process_github_webhook, caplog):
"""When 'python-version' is set under 'tox', no deprecation warning should be logged."""
with caplog.at_level(python_logging.WARNING):
process_github_webhook._repo_data_from_config(repository_config={})

assert process_github_webhook.tox_python_version == "3.8"
assert not any(
"tox-python-version" in r.getMessage() and "deprecated" in r.getMessage().lower() for r in caplog.records
)


def test_tox_python_version_legacy_deprecation_warning(process_github_webhook, caplog):
"""When standalone 'tox-python-version' is used instead of nested 'tox.python-version',
a deprecation warning should be logged."""
original_get_value = process_github_webhook.config.get_value

def patched_get_value(value: str, *args: Any, **kwargs: Any) -> Any:
# Override tox to return a dict WITHOUT python-version
if value == "tox":
return {"args": "-x --no-header"}
# Return legacy standalone key
if value == "tox-python-version":
return "3.11"
return original_get_value(value, *args, **kwargs)

with patch.object(process_github_webhook.config, "get_value", side_effect=patched_get_value):
with caplog.at_level(python_logging.WARNING):
process_github_webhook._repo_data_from_config(repository_config={})

assert process_github_webhook.tox_python_version == "3.11"
assert any(
"tox-python-version" in r.getMessage() and "deprecated" in r.getMessage().lower() for r in caplog.records
)


def test_tox_python_version_nested_takes_priority_over_legacy(process_github_webhook, caplog):
"""When both nested 'tox.python-version' and legacy 'tox-python-version' are set,
nested takes priority and no deprecation warning is logged."""
original_get_value = process_github_webhook.config.get_value

def patched_get_value(value: str, *args: Any, **kwargs: Any) -> Any:
# tox dict has python-version set
if value == "tox":
return {"args": "-x --no-header", "python-version": "3.12"}
# Legacy key also set
if value == "tox-python-version":
return "3.9"
return original_get_value(value, *args, **kwargs)

with patch.object(process_github_webhook.config, "get_value", side_effect=patched_get_value):
with caplog.at_level(python_logging.WARNING):
process_github_webhook._repo_data_from_config(repository_config={})

assert process_github_webhook.tox_python_version == "3.12"
assert not any(
"tox-python-version" in r.getMessage() and "deprecated" in r.getMessage().lower() for r in caplog.records
)


def test_tox_config_not_mutated_by_repo_data_from_config(process_github_webhook):
"""Calling _repo_data_from_config must NOT mutate the shared tox dict
returned by config.get_value. A shallow copy should be used instead."""
shared_tox_dict = {"main": "all", "args": "-v", "python-version": "3.10"}
original_get_value = process_github_webhook.config.get_value

def patched_get_value(value: str, *args: Any, **kwargs: Any) -> Any:
if value == "tox":
return shared_tox_dict
return original_get_value(value, *args, **kwargs)

with patch.object(process_github_webhook.config, "get_value", side_effect=patched_get_value):
process_github_webhook._repo_data_from_config(repository_config={})

# The original dict must still contain all its original keys
assert "args" in shared_tox_dict, "shared tox dict was mutated: 'args' key was removed"
assert "python-version" in shared_tox_dict, "shared tox dict was mutated: 'python-version' key was removed"
assert shared_tox_dict == {"main": "all", "args": "-v", "python-version": "3.10"}


def test_tox_python_version_empty_string_uses_presence_not_truthiness(process_github_webhook, caplog):
"""When 'python-version' is explicitly set to empty string under 'tox',
it should still take precedence over a legacy 'tox-python-version' value.
Precedence is by key presence, not by truthiness."""
original_get_value = process_github_webhook.config.get_value

def patched_get_value(value: str, *args: Any, **kwargs: Any) -> Any:
if value == "tox":
# python-version is present but empty
return {"args": "-x", "python-version": ""}
if value == "tox-python-version":
return "3.11"
return original_get_value(value, *args, **kwargs)

with patch.object(process_github_webhook.config, "get_value", side_effect=patched_get_value):
with caplog.at_level(python_logging.WARNING):
process_github_webhook._repo_data_from_config(repository_config={})

# Empty string should win over legacy because the key IS present
assert process_github_webhook.tox_python_version == ""
# No deprecation warning since nested key is present
assert not any(
"tox-python-version" in r.getMessage() and "deprecated" in r.getMessage().lower() for r in caplog.records
)
35 changes: 35 additions & 0 deletions webhook_server/tests/test_runner_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def mock_github_webhook(self) -> Mock:
mock_webhook.clone_repo_dir = "/tmp/test-repo"
mock_webhook.tox = {"main": "all"}
mock_webhook.tox_python_version = "3.12"
mock_webhook.tox_args = ""
mock_webhook.pre_commit = True
mock_webhook.build_and_push_container = True
mock_webhook.pypi = {"token": "dummy"}
Expand Down Expand Up @@ -265,6 +266,40 @@ async def test_run_tox_failure(self, runner_handler: RunnerHandler, mock_pull_re
name=TOX_STR, output={"title": "Tox", "summary": "", "text": "dummy output"}
)

@pytest.mark.asyncio
async def test_run_tox_with_tox_args(self, runner_handler: RunnerHandler, mock_pull_request: Mock) -> None:
"""Test run_tox appends tox_args to the command when configured."""
runner_handler.github_webhook.tox_args = "-- -k test_something"
with patch.object(runner_handler, "run_check", new_callable=AsyncMock) as mock_run_check:
await runner_handler.run_tox(mock_pull_request)
mock_run_check.assert_called_once()
check_config = mock_run_check.call_args[1]["check_config"]
assert check_config.command.endswith("-- -k test_something")

@pytest.mark.asyncio
async def test_run_tox_without_tox_args(self, runner_handler: RunnerHandler, mock_pull_request: Mock) -> None:
"""Test run_tox does not append tox_args when not configured."""
runner_handler.github_webhook.tox_args = ""
with patch.object(runner_handler, "run_check", new_callable=AsyncMock) as mock_run_check:
await runner_handler.run_tox(mock_pull_request)
mock_run_check.assert_called_once()
check_config = mock_run_check.call_args[1]["check_config"]
assert "-- -k" not in check_config.command

@pytest.mark.asyncio
async def test_run_tox_with_tox_args_and_specific_tests(
self, runner_handler: RunnerHandler, mock_pull_request: Mock
) -> None:
"""Test run_tox appends tox_args after -e tests."""
runner_handler.github_webhook.tox = {"main": "py312,lint"}
runner_handler.github_webhook.tox_args = "--parallel"
with patch.object(runner_handler, "run_check", new_callable=AsyncMock) as mock_run_check:
await runner_handler.run_tox(mock_pull_request)
mock_run_check.assert_called_once()
check_config = mock_run_check.call_args[1]["check_config"]
assert "-e py312,lint" in check_config.command
assert check_config.command.endswith("--parallel")

@pytest.mark.asyncio
async def test_run_pre_commit_disabled(self, runner_handler: RunnerHandler, mock_pull_request: Mock) -> None:
"""Test run_pre_commit when pre_commit is disabled."""
Expand Down