diff --git a/examples/.github-webhook-server.yaml b/examples/.github-webhook-server.yaml index 2da78707f..6611628d5 100644 --- a/examples/.github-webhook-server.yaml +++ b/examples/.github-webhook-server.yaml @@ -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 diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 08063d5c7..426392eed 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -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: diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 3c3e9f411..443387004 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -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) diff --git a/webhook_server/libs/handlers/runner_handler.py b/webhook_server/libs/handlers/runner_handler.py index 5370b04db..d11e43861 100644 --- a/webhook_server/libs/handlers/runner_handler.py +++ b/webhook_server/libs/handlers/runner_handler.py @@ -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) diff --git a/webhook_server/tests/manifests/config.yaml b/webhook_server/tests/manifests/config.yaml index f89d0c35f..05d7596b0 100644 --- a/webhook_server/tests/manifests/config.yaml +++ b/webhook_server/tests/manifests/config.yaml @@ -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 diff --git a/webhook_server/tests/test_config_schema.py b/webhook_server/tests/test_config_schema.py index b6b6526fb..1ebb9fc6c 100644 --- a/webhook_server/tests/test_config_schema.py +++ b/webhook_server/tests/test_config_schema.py @@ -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() diff --git a/webhook_server/tests/test_repo_data_from_config.py b/webhook_server/tests/test_repo_data_from_config.py index ba1812cc6..fc773be61 100644 --- a/webhook_server/tests/test_repo_data_from_config.py +++ b/webhook_server/tests/test_repo_data_from_config.py @@ -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={}) @@ -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 @@ -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 + ) diff --git a/webhook_server/tests/test_runner_handler.py b/webhook_server/tests/test_runner_handler.py index 465cf5de4..59a1f3fb8 100644 --- a/webhook_server/tests/test_runner_handler.py +++ b/webhook_server/tests/test_runner_handler.py @@ -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"} @@ -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."""