From 17e1f1dc7299eec81270f1c2e8f522d25c4a77ff Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Thu, 19 Jun 2025 13:54:05 +0300 Subject: [PATCH] feat: standardize config naming and add optional issue creation for new PRs --- README.md | 18 +- examples/.github-webhook-server.yaml | 15 +- examples/config.yaml | 13 +- webhook_server/config/schema.yaml | 18 +- webhook_server/libs/github_api.py | 10 +- webhook_server/libs/pull_request_handler.py | 14 +- webhook_server/tests/manifests/config.yaml | 6 +- .../tests/test_branch_protection.py | 9 +- webhook_server/tests/test_config_schema.py | 100 +++++++--- .../tests/test_pull_request_handler.py | 186 ++++++++++++++++++ webhook_server/tests/test_schema_validator.py | 16 +- webhook_server/tests/test_webhook.py | 4 +- .../utils/github_repository_settings.py | 18 +- webhook_server/utils/webhook.py | 2 +- 14 files changed, 350 insertions(+), 79 deletions(-) create mode 100644 webhook_server/tests/test_pull_request_handler.py diff --git a/README.md b/README.md index 08a138e3..0c8c4bd9 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,7 @@ Create `config.yaml` in your data directory: # yaml-language-server: $schema=https://raw.githubusercontent.com/myk-org/github-webhook-server/refs/heads/main/webhook_server/config/schema.yaml github-app-id: 123456 -webhook_ip: https://your-domain.com +webhook-ip: https://your-domain.com github-tokens: - ghp_your_github_token @@ -244,10 +244,10 @@ repositories: my-project: name: my-org/my-project log-level: DEBUG - slack_webhook_url: https://hooks.slack.com/services/YOUR/SLACK/WEBHOOK + slack-webhook-url: https://hooks.slack.com/services/YOUR/SLACK/WEBHOOK # CI/CD Features - verified_job: true + verified-job: true pre-commit: true # Testing Configuration @@ -364,18 +364,18 @@ uv run pytest webhook_server/tests/test_config_schema.py::TestConfigSchema::test | ------------ | ---------------------------------------------------------------------------------------- | | **Server** | `ip-bind`, `port`, `max-workers`, `log-level`, `log-file` | | **Security** | `webhook-secret`, `verify-github-ips`, `verify-cloudflare-ips`, `disable-ssl-warnings` | -| **GitHub** | `github-app-id`, `github-tokens`, `webhook_ip` | -| **Defaults** | `docker`, `default-status-checks`, `auto-verified-and-merged-users`, `branch_protection` | +| **GitHub** | `github-app-id`, `github-tokens`, `webhook-ip` | +| **Defaults** | `docker`, `default-status-checks`, `auto-verified-and-merged-users`, `branch-protection`, `create-issue-for-new-pr` | #### Repository Level Options | Category | Options | | ----------------- | --------------------------------------------------------------------- | -| **Basic** | `name`, `log-level`, `log-file`, `slack_webhook_url`, `events` | -| **Features** | `verified_job`, `pre-commit`, `pypi`, `tox`, `container` | -| **Pull Requests** | `minimum-lgtm`, `conventional-title`, `can-be-merged-required-labels` | +| **Basic** | `name`, `log-level`, `log-file`, `slack-webhook-url`, `events` | +| **Features** | `verified-job`, `pre-commit`, `pypi`, `tox`, `container` | +| **Pull Requests** | `minimum-lgtm`, `conventional-title`, `can-be-merged-required-labels`, `create-issue-for-new-pr` | | **Automation** | `set-auto-merge-prs`, `auto-verified-and-merged-users` | -| **Protection** | `protected-branches`, `branch_protection` | +| **Protection** | `protected-branches`, `branch-protection` | ## Deployment diff --git a/examples/.github-webhook-server.yaml b/examples/.github-webhook-server.yaml index 71c67514..3043771c 100644 --- a/examples/.github-webhook-server.yaml +++ b/examples/.github-webhook-server.yaml @@ -7,10 +7,10 @@ log-level: DEBUG # Options: INFO, DEBUG log-file: /path/to/repository-specific.log # Slack integration -slack_webhook_url: https://hooks.slack.com/services/YOUR/SLACK/WEBHOOK +slack-webhook-url: https://hooks.slack.com/services/YOUR/SLACK/WEBHOOK # Job verification settings -verified_job: true # Enable/disable verified job functionality +verified-job: true # Enable/disable verified job functionality # PyPI publishing configuration pypi: @@ -70,12 +70,12 @@ github-tokens: - ghp_your_repository_specific_token_here # Branch protection rules -branch_protection: +branch-protection: strict: true require_code_owner_reviews: true - dismiss_stale_reviews: true - required_approving_review_count: 2 - required_linear_history: false + dismiss_stale_reviews: false + required_approving_review_count: 1 + required_linear_history: true required_conversation_resolution: true # Auto-merge configuration @@ -94,3 +94,6 @@ conventional-title: "feat,fix,docs,style,refactor,test,chore" # Minimum LGTM count required minimum-lgtm: 2 + +# Issue creation for new pull requests +create-issue-for-new-pr: true # Create tracking issues for new PRs diff --git a/examples/config.yaml b/examples/config.yaml index b8fb1692..8a535550 100644 --- a/examples/config.yaml +++ b/examples/config.yaml @@ -11,7 +11,7 @@ github-tokens: - - -webhook_ip: +webhook-ip: docker: # Used to pull images from docker.io username: @@ -26,7 +26,9 @@ auto-verified-and-merged-users: - "renovate[bot]" - "pre-commit-ci[bot]" -branch_protection: +create-issue-for-new-pr: true # Global default: create tracking issues for new PRs + +branch-protection: strict: True require_code_owner_reviews: True dismiss_stale_reviews: False @@ -39,8 +41,8 @@ repositories: name: my-org/my-repository log-level: DEBUG # Override global log-level for repository log-file: my-repository.log # Override global log-file for repository - slack_webhook_url: # Send notification to slack on several operations - verified_job: true + slack-webhook-url: # Send notification to slack on several operations + verified-job: true pypi: token: @@ -88,7 +90,7 @@ repositories: - my-label2 conventional-title: "ci,docs,feat,fix,refactor,test,release" # Check PR title start with any of these words + : - branch_protection: + branch-protection: strict: True require_code_owner_reviews: True dismiss_stale_reviews: False @@ -97,5 +99,6 @@ repositories: required_conversation_resolution: True minimum-lgtm: 0 # The minimum PR lgtm required before approve the PR + create-issue-for-new-pr: true # Override global setting: create tracking issues for new PRs (default: true) set-auto-merge-prs: - main diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 8412db0f..a0393e45 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -18,7 +18,7 @@ properties: items: type: string description: Global GitHub token for all repositories - webhook_ip: + webhook-ip: type: string description: IP or FQDN address of the webhook server for adding to the repositories @@ -62,8 +62,12 @@ properties: type: array items: type: string + create-issue-for-new-pr: + type: boolean + description: Create a tracking issue for new pull requests (global default) + default: true - branch_protection: + branch-protection: type: object properties: strict: @@ -95,10 +99,10 @@ properties: log-file: type: string description: Override global log-file for repository - slack_webhook_url: + slack-webhook-url: type: string description: Slack webhook URL - verified_job: + verified-job: type: boolean default: true description: Enable verified job functionality @@ -187,7 +191,7 @@ properties: items: type: string description: Override GitHub tokens for this repository - branch_protection: + branch-protection: type: object properties: strict: @@ -219,3 +223,7 @@ properties: type: integer description: Minimum number of LGTM required before approving PR default: 0 + create-issue-for-new-pr: + type: boolean + description: Create a tracking issue for new pull requests + default: true diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 8176fbc6..8a2f806f 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -243,7 +243,7 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None: ) self.tox: dict[str, str] = self.config.get_value(value="tox", extra_dict=repository_config) self.tox_python_version: str = self.config.get_value(value="tox-python-version", extra_dict=repository_config) - self.slack_webhook_url: str = self.config.get_value(value="slack_webhook_url", extra_dict=repository_config) + self.slack_webhook_url: str = self.config.get_value(value="slack-webhook-url", extra_dict=repository_config) self.build_and_push_container: dict[str, Any] = self.config.get_value( value="container", return_on_none={}, extra_dict=repository_config @@ -275,6 +275,14 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None: self.minimum_lgtm: int = self.config.get_value( value="minimum-lgtm", return_on_none=0, extra_dict=repository_config ) + # Load global create_issue_for_new_pr setting as fallback + global_create_issue_for_new_pr: bool = self.config.get_value( + value="create-issue-for-new-pr", return_on_none=True + ) + # Repository-specific setting overrides global setting + self.create_issue_for_new_pr: bool = self.config.get_value( + value="create-issue-for-new-pr", return_on_none=global_create_issue_for_new_pr, extra_dict=repository_config + ) async def get_pull_request(self, number: int | None = None) -> PullRequest | None: if number: diff --git a/webhook_server/libs/pull_request_handler.py b/webhook_server/libs/pull_request_handler.py index 503ce5ed..9e496a72 100644 --- a/webhook_server/libs/pull_request_handler.py +++ b/webhook_server/libs/pull_request_handler.py @@ -183,6 +183,13 @@ def _prepare_welcome_comment(self) -> str: > **Note**: You are an auto-verified user. Your PRs will be automatically verified and may be auto-merged when all requirements are met. """ + # Check if issue creation is enabled + issue_creation_note = "" + if self.github_webhook.create_issue_for_new_pr: + issue_creation_note = "* **Issue Creation**: A tracking issue is created for this PR and will be closed when the PR is merged or closed\n" + else: + issue_creation_note = "* **Issue Creation**: Disabled for this repository\n" + return f""" {self.github_webhook.issue_url_for_welcome_msg} @@ -193,8 +200,7 @@ def _prepare_welcome_comment(self) -> str: ### 🔄 Automatic Actions * **Reviewer Assignment**: Reviewers are automatically assigned based on the OWNERS file in the repository root * **Size Labeling**: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes -* **Issue Creation**: A tracking issue is created for this PR and will be closed when the PR is merged or closed -* **Pre-commit Checks**: [pre-commit](https://pre-commit.ci/) runs automatically if `.pre-commit-config.yaml` exists +{issue_creation_note}* **Pre-commit Checks**: [pre-commit](https://pre-commit.ci/) runs automatically if `.pre-commit-config.yaml` exists * **Branch Labeling**: Branch-specific labels are applied to track the target branch * **Auto-verification**: Auto-verified users have their PRs automatically marked as verified @@ -430,6 +436,10 @@ async def process_opened_or_synchronize_pull_request(self, pull_request: PullReq self.logger.error(f"{self.log_prefix} Async task failed: {result}") async def create_issue_for_new_pull_request(self, pull_request: PullRequest) -> None: + if not self.github_webhook.create_issue_for_new_pr: + self.logger.info(f"{self.log_prefix} Issue creation for new PRs is disabled for this repository") + return + if self.github_webhook.parent_committer in self.github_webhook.auto_verified_and_merged_users: self.logger.info( f"{self.log_prefix} Committer {self.github_webhook.parent_committer} is part of " diff --git a/webhook_server/tests/manifests/config.yaml b/webhook_server/tests/manifests/config.yaml index 67768274..0837396f 100644 --- a/webhook_server/tests/manifests/config.yaml +++ b/webhook_server/tests/manifests/config.yaml @@ -6,7 +6,7 @@ github-tokens: - GITHIB TOKEN1 - GITHIB TOKEN2 -webhook_ip: HTTP://IP OR URL:PORT +webhook-ip: HTTP://IP OR URL:PORT webhook-secret: test-webhook-secret # pragma: allowlist secret docker: # Used to pull images from docker.io @@ -27,8 +27,8 @@ repositories: name: my-org/test-repo log-level: DEBUG # Override global log-level for repository log-file: test-repo.log # Override global log-file for repository - slack_webhook_url: Slack webhook url # Send notification to slack on several operations - verified_job: true + slack-webhook-url: Slack webhook url # Send notification to slack on several operations + verified-job: true pypi: token: PYPI TOKEN diff --git a/webhook_server/tests/test_branch_protection.py b/webhook_server/tests/test_branch_protection.py index e2677685..f88de8b1 100644 --- a/webhook_server/tests/test_branch_protection.py +++ b/webhook_server/tests/test_branch_protection.py @@ -1,4 +1,5 @@ import os +from typing import Any import pytest from webhook_server.libs.config import Config @@ -9,14 +10,14 @@ @pytest.fixture() -def branch_protection_rules(request, mocker): +def branch_protection_rules(request: pytest.FixtureRequest, mocker: Any) -> dict[str, Any]: config_path = "webhook_server.libs.config.Config" os.environ["WEBHOOK_SERVER_DATA_DIR"] = "webhook_server/tests/manifests" repo_name = "test-repo" config = Config(repository=repo_name) root_data = config.root_data - root_data.setdefault("branch_protection", request.param.get("global", {})) - root_data["repositories"][repo_name].setdefault("branch_protection", request.param.get("repo")) + root_data.setdefault("branch-protection", request.param.get("global", {})) + root_data["repositories"][repo_name].setdefault("branch-protection", request.param.get("repo")) mocker.patch(f"{config_path}.root_data", new_callable=mocker.PropertyMock, return_value=root_data) @@ -93,7 +94,7 @@ def branch_protection_rules(request, mocker): ], indirect=["branch_protection_rules"], ) -def test_branch_protection_setup(branch_protection_rules, expected): +def test_branch_protection_setup(branch_protection_rules: dict[str, Any], expected: dict[str, Any]) -> None: mismatch = {} for key in expected: if branch_protection_rules[key] != expected[key]: diff --git a/webhook_server/tests/test_config_schema.py b/webhook_server/tests/test_config_schema.py index 95247fc2..bbb2b8e9 100644 --- a/webhook_server/tests/test_config_schema.py +++ b/webhook_server/tests/test_config_schema.py @@ -3,7 +3,7 @@ from typing import Any import pytest -import yaml # type: ignore +import yaml from webhook_server.libs.config import Config @@ -17,7 +17,7 @@ def valid_minimal_config(self) -> dict[str, Any]: return { "github-app-id": 123456, "github-tokens": ["token1"], - "webhook_ip": "http://localhost:5000", + "webhook-ip": "http://localhost:5000", "repositories": {"test-repo": {"name": "org/test-repo"}}, } @@ -29,7 +29,7 @@ def valid_full_config(self) -> dict[str, Any]: "log-file": "webhook.log", "github-app-id": 123456, "github-tokens": ["token1", "token2"], - "webhook_ip": "http://localhost:5000", + "webhook-ip": "http://localhost:5000", "ip-bind": "0.0.0.0", "port": 8080, "max-workers": 20, @@ -40,7 +40,7 @@ def valid_full_config(self) -> dict[str, Any]: "docker": {"username": "dockeruser", "password": "dockerpass"}, # pragma: allowlist secret "default-status-checks": ["WIP", "build"], "auto-verified-and-merged-users": ["bot[bot]"], - "branch_protection": { + "branch-protection": { "strict": True, "require_code_owner_reviews": True, "dismiss_stale_reviews": False, @@ -53,8 +53,8 @@ def valid_full_config(self) -> dict[str, Any]: "name": "org/test-repo", "log-level": "INFO", "log-file": "test-repo.log", - "slack_webhook_url": "https://hooks.slack.com/test", - "verified_job": True, + "slack-webhook-url": "https://hooks.slack.com/test", + "verified-job": True, "pypi": {"token": "pypi-token"}, "events": ["push", "pull_request"], "tox": {"main": "all", "dev": ["test1", "test2"]}, @@ -72,7 +72,7 @@ def valid_full_config(self) -> dict[str, Any]: }, "auto-verified-and-merged-users": ["user1"], "github-tokens": ["repo-token"], - "branch_protection": {"strict": False, "required_approving_review_count": 1}, + "branch-protection": {"strict": False, "required_approving_review_count": 1}, "set-auto-merge-prs": ["main"], "can-be-merged-required-labels": ["ready"], "conventional-title": "feat,fix,docs", @@ -102,7 +102,7 @@ def test_valid_minimal_config_loads( config = Config() assert config.root_data["github-app-id"] == 123456 - assert config.root_data["webhook_ip"] == "http://localhost:5000" + assert config.root_data["webhook-ip"] == "http://localhost:5000" assert "test-repo" in config.root_data["repositories"] finally: # Clean up @@ -160,7 +160,7 @@ def test_required_fields_validation(self, monkeypatch: pytest.MonkeyPatch) -> No config_without_repos = { "github-app-id": 123456, "github-tokens": ["token1"], - "webhook_ip": "http://localhost:5000", + "webhook-ip": "http://localhost:5000", } temp_dir = self.create_temp_config_dir_and_data(config_without_repos) @@ -215,9 +215,9 @@ def test_docker_object_validation(self, valid_minimal_config: dict[str, Any]) -> shutil.rmtree(temp_dir) def test_branch_protection_object_validation(self, valid_minimal_config: dict[str, Any]) -> None: - """Test that branch_protection accepts proper boolean and integer values.""" + """Test that branch-protection accepts proper boolean and integer values.""" config = valid_minimal_config.copy() - config["branch_protection"] = { + config["branch-protection"] = { "strict": True, "require_code_owner_reviews": False, "dismiss_stale_reviews": True, @@ -232,7 +232,7 @@ def test_branch_protection_object_validation(self, valid_minimal_config: dict[st config_file = os.path.join(temp_dir, "config.yaml") with open(config_file, "r") as file_handle: data = yaml.safe_load(file_handle) - branch_protection = data["branch_protection"] + branch_protection = data["branch-protection"] assert branch_protection["strict"] is True assert branch_protection["require_code_owner_reviews"] is False assert branch_protection["required_approving_review_count"] == 2 @@ -246,7 +246,7 @@ def test_repository_structure_validation(self, valid_minimal_config: dict[str, A config = valid_minimal_config.copy() config["repositories"] = { "repo1": {"name": "org/repo1"}, - "repo2": {"name": "org/repo2", "verified_job": False, "minimum-lgtm": 1}, + "repo2": {"name": "org/repo2", "verified-job": False, "minimum-lgtm": 1}, } temp_dir = self.create_temp_config_dir_and_data(config) @@ -344,7 +344,7 @@ def test_boolean_fields_validation(self, valid_minimal_config: dict[str, Any]) - """Test that boolean fields accept proper boolean values.""" config = valid_minimal_config.copy() config.update({"verify-github-ips": True, "verify-cloudflare-ips": False, "disable-ssl-warnings": True}) - config["repositories"]["test-repo"].update({"verified_job": False, "pre-commit": True}) + config["repositories"]["test-repo"].update({"verified-job": False, "pre-commit": True}) temp_dir = self.create_temp_config_dir_and_data(config) @@ -355,7 +355,7 @@ def test_boolean_fields_validation(self, valid_minimal_config: dict[str, Any]) - assert data["verify-github-ips"] is True assert data["verify-cloudflare-ips"] is False assert data["disable-ssl-warnings"] is True - assert data["repositories"]["test-repo"]["verified_job"] is False + assert data["repositories"]["test-repo"]["verified-job"] is False assert data["repositories"]["test-repo"]["pre-commit"] is True finally: import shutil @@ -434,8 +434,10 @@ def test_malformed_yaml_handling(self, monkeypatch: pytest.MonkeyPatch) -> None: shutil.rmtree(temp_dir) - def test_default_values_behavior(self, valid_minimal_config: dict[str, Any]) -> None: - """Test that default values are handled correctly when not specified.""" + def test_default_values_behavior( + self, valid_minimal_config: dict[str, Any], monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test that default values are properly applied when not specified.""" # Test that optional fields can be omitted config = valid_minimal_config.copy() @@ -443,13 +445,63 @@ def test_default_values_behavior(self, valid_minimal_config: dict[str, Any]) -> temp_dir = self.create_temp_config_dir_and_data(config) try: - config_file = os.path.join(temp_dir, "config.yaml") - with open(config_file, "r") as file_handle: - data = yaml.safe_load(file_handle) - # These fields should not be present since they weren't specified - assert "disable-ssl-warnings" not in data - assert "verify-github-ips" not in data - assert "minimum-lgtm" not in data["repositories"]["test-repo"] + monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_dir) + config_obj = Config() + # These fields should not be present since they weren't specified + assert "disable-ssl-warnings" not in config_obj.root_data + assert "verify-github-ips" not in config_obj.root_data + assert "minimum-lgtm" not in config_obj.root_data["repositories"]["test-repo"] + finally: + import shutil + + shutil.rmtree(temp_dir) + + def test_create_issue_for_new_pr_configuration(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Test create-issue-for-new-pr configuration at global and repository levels.""" + # Test global configuration + global_config = { + "github-app-id": 123456, + "github-tokens": ["token1"], + "webhook-ip": "http://localhost:8080", + "create-issue-for-new-pr": False, # Global setting + "repositories": { + "test-repo": { + "name": "test-org/test-repo", + # No repository-specific setting - should use global + } + }, + } + + temp_dir = self.create_temp_config_dir_and_data(global_config) + try: + monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_dir) + config = Config() + assert config.root_data["create-issue-for-new-pr"] is False + finally: + import shutil + + shutil.rmtree(temp_dir) + + # Test repository-specific override + repo_override_config = { + "github-app-id": 123456, + "github-tokens": ["token1"], + "webhook-ip": "http://localhost:8080", + "create-issue-for-new-pr": False, # Global setting + "repositories": { + "test-repo": { + "name": "test-org/test-repo", + "create-issue-for-new-pr": True, # Repository override + } + }, + } + + temp_dir = self.create_temp_config_dir_and_data(repo_override_config) + try: + monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_dir) + config = Config() + assert config.root_data["create-issue-for-new-pr"] is False + assert config.root_data["repositories"]["test-repo"]["create-issue-for-new-pr"] is True finally: import shutil diff --git a/webhook_server/tests/test_pull_request_handler.py b/webhook_server/tests/test_pull_request_handler.py new file mode 100644 index 00000000..9dc1612a --- /dev/null +++ b/webhook_server/tests/test_pull_request_handler.py @@ -0,0 +1,186 @@ +import pytest +from unittest.mock import AsyncMock, Mock + +from webhook_server.libs.pull_request_handler import PullRequestHandler + + +class TestCreateIssueForNewPR: + """Test the create-issue-for-new-pr configuration option.""" + + @pytest.mark.asyncio + async def test_create_issue_when_enabled(self) -> None: + """Test that issue is created when create-issue-for-new-pr is enabled.""" + # Mock github_webhook + mock_webhook = Mock() + mock_webhook.create_issue_for_new_pr = True + mock_webhook.parent_committer = "testuser" + mock_webhook.auto_verified_and_merged_users = [] + mock_webhook.log_prefix = "[TEST]" + mock_webhook.logger = Mock() + mock_webhook.repository = Mock() + mock_webhook.repository.create_issue = AsyncMock() + + # Mock owners_file_handler + mock_owners_handler = Mock() + + # Mock pull request + mock_pr = Mock() + mock_pr.title = "Test PR" + mock_pr.number = 123 + mock_pr.user.login = "testuser" + + # Create handler and test + handler = PullRequestHandler(mock_webhook, mock_owners_handler) + await handler.create_issue_for_new_pull_request(mock_pr) + + # Verify issue was created + mock_webhook.repository.create_issue.assert_called_once() + call_args = mock_webhook.repository.create_issue.call_args + assert call_args[1]["title"] == "Test PR - 123" + assert call_args[1]["body"] == "[Auto generated]\nNumber: [#123]" + assert call_args[1]["assignee"] == "testuser" + + @pytest.mark.asyncio + async def test_create_issue_when_disabled(self) -> None: + """Test that issue is not created when create-issue-for-new-pr is disabled.""" + # Mock github_webhook + mock_webhook = Mock() + mock_webhook.create_issue_for_new_pr = False + mock_webhook.log_prefix = "[TEST]" + mock_webhook.logger = Mock() + mock_webhook.repository = Mock() + mock_webhook.repository.create_issue = AsyncMock() + + # Mock owners_file_handler + mock_owners_handler = Mock() + + # Mock pull request + mock_pr = Mock() + mock_pr.title = "Test PR" + + # Create handler and test + handler = PullRequestHandler(mock_webhook, mock_owners_handler) + await handler.create_issue_for_new_pull_request(mock_pr) + + # Verify issue was not created + mock_webhook.repository.create_issue.assert_not_called() + mock_webhook.logger.info.assert_called_with("[TEST] Issue creation for new PRs is disabled for this repository") + + @pytest.mark.asyncio + async def test_create_issue_for_auto_verified_user(self) -> None: + """Test that issue is not created for auto-verified users even when enabled.""" + # Mock github_webhook + mock_webhook = Mock() + mock_webhook.create_issue_for_new_pr = True + mock_webhook.parent_committer = "autouser" + mock_webhook.auto_verified_and_merged_users = ["autouser"] + mock_webhook.log_prefix = "[TEST]" + mock_webhook.logger = Mock() + mock_webhook.repository = Mock() + mock_webhook.repository.create_issue = AsyncMock() + + # Mock owners_file_handler + mock_owners_handler = Mock() + + # Mock pull request + mock_pr = Mock() + mock_pr.title = "Test PR" + + # Create handler and test + handler = PullRequestHandler(mock_webhook, mock_owners_handler) + await handler.create_issue_for_new_pull_request(mock_pr) + + # Verify issue was not created + mock_webhook.repository.create_issue.assert_not_called() + mock_webhook.logger.info.assert_called_with( + "[TEST] Committer autouser is part of ['autouser'], will not create issue." + ) + + @pytest.mark.asyncio + async def test_create_issue_uses_global_config_when_repo_not_set(self) -> None: + """Test that global create-issue-for-new-pr setting is used when repository doesn't override it.""" + # Mock github_webhook with global setting + mock_webhook = Mock() + mock_webhook.create_issue_for_new_pr = False # Global setting + mock_webhook.parent_committer = "testuser" + mock_webhook.auto_verified_and_merged_users = [] + mock_webhook.log_prefix = "[TEST]" + mock_webhook.logger = Mock() + mock_webhook.repository = Mock() + mock_webhook.repository.create_issue = AsyncMock() + + # Mock owners_file_handler + mock_owners_handler = Mock() + + # Mock pull request + mock_pr = Mock() + mock_pr.title = "Test PR" + + # Create handler and test + handler = PullRequestHandler(mock_webhook, mock_owners_handler) + await handler.create_issue_for_new_pull_request(mock_pr) + + # Verify issue was not created (using global setting) + mock_webhook.repository.create_issue.assert_not_called() + mock_webhook.logger.info.assert_called_with("[TEST] Issue creation for new PRs is disabled for this repository") + + @pytest.mark.asyncio + async def test_create_issue_repo_config_overrides_global(self) -> None: + """Test that repository-specific create-issue-for-new-pr setting overrides global setting.""" + # Mock github_webhook with repository override + mock_webhook = Mock() + mock_webhook.create_issue_for_new_pr = True # Repository overrides global False + mock_webhook.parent_committer = "testuser" + mock_webhook.auto_verified_and_merged_users = [] + mock_webhook.log_prefix = "[TEST]" + mock_webhook.logger = Mock() + mock_webhook.repository = Mock() + mock_webhook.repository.create_issue = AsyncMock() + + # Mock owners_file_handler + mock_owners_handler = Mock() + + # Mock pull request + mock_pr = Mock() + mock_pr.title = "Test PR" + mock_pr.number = 123 + mock_pr.user.login = "testuser" + + # Create handler and test + handler = PullRequestHandler(mock_webhook, mock_owners_handler) + await handler.create_issue_for_new_pull_request(mock_pr) + + # Verify issue was created (repository setting overrides global) + mock_webhook.repository.create_issue.assert_called_once() + call_args = mock_webhook.repository.create_issue.call_args + assert call_args[1]["title"] == "Test PR - 123" + assert call_args[1]["body"] == "[Auto generated]\nNumber: [#123]" + assert call_args[1]["assignee"] == "testuser" + + @pytest.mark.asyncio + async def test_create_issue_from_github_webhook_server_yaml(self) -> None: + """Test that create-issue-for-new-pr setting from .github-webhook-server.yaml is used.""" + # Mock github_webhook with .github-webhook-server.yaml setting + mock_webhook = Mock() + mock_webhook.create_issue_for_new_pr = False # From .github-webhook-server.yaml + mock_webhook.parent_committer = "testuser" + mock_webhook.auto_verified_and_merged_users = [] + mock_webhook.log_prefix = "[TEST]" + mock_webhook.logger = Mock() + mock_webhook.repository = Mock() + mock_webhook.repository.create_issue = AsyncMock() + + # Mock owners_file_handler + mock_owners_handler = Mock() + + # Mock pull request + mock_pr = Mock() + mock_pr.title = "Test PR" + + # Create handler and test + handler = PullRequestHandler(mock_webhook, mock_owners_handler) + await handler.create_issue_for_new_pull_request(mock_pr) + + # Verify issue was not created (using .github-webhook-server.yaml setting) + mock_webhook.repository.create_issue.assert_not_called() + mock_webhook.logger.info.assert_called_with("[TEST] Issue creation for new PRs is disabled for this repository") diff --git a/webhook_server/tests/test_schema_validator.py b/webhook_server/tests/test_schema_validator.py index f971060c..5c530f9d 100644 --- a/webhook_server/tests/test_schema_validator.py +++ b/webhook_server/tests/test_schema_validator.py @@ -54,7 +54,7 @@ def _validate_required_fields(self, config: dict[str, Any]) -> None: def _validate_root_fields(self, config: dict[str, Any]) -> None: """Validate root-level configuration fields.""" # String fields - string_fields = ["log-level", "log-file", "webhook_ip", "ip-bind", "webhook-secret"] + string_fields = ["log-level", "log-file", "webhook-ip", "ip-bind", "webhook-secret"] for field in string_fields: if field in config and not isinstance(config[field], str): self.errors.append(f"Field '{field}' must be a string") @@ -85,8 +85,8 @@ def _validate_root_fields(self, config: dict[str, Any]) -> None: if "docker" in config: self._validate_docker_config(config["docker"]) - if "branch_protection" in config: - self._validate_branch_protection(config["branch_protection"]) + if "branch-protection" in config: + self._validate_branch_protection(config["branch-protection"]) def _validate_docker_config(self, docker_config: Any) -> None: """Validate docker configuration.""" @@ -104,7 +104,7 @@ def _validate_docker_config(self, docker_config: Any) -> None: def _validate_branch_protection(self, branch_protection_config: Any) -> None: """Validate branch protection configuration.""" if not isinstance(branch_protection_config, dict): - self.errors.append("Field 'branch_protection' must be an object") + self.errors.append("Field 'branch-protection' must be an object") return boolean_branch_protection_fields = [ @@ -116,11 +116,11 @@ def _validate_branch_protection(self, branch_protection_config: Any) -> None: ] for field in boolean_branch_protection_fields: if field in branch_protection_config and not isinstance(branch_protection_config[field], bool): - self.errors.append(f"Branch protection field '{field}' must be a boolean") + self.errors.append(f"Field 'branch-protection.{field}' must be a boolean") if "required_approving_review_count" in branch_protection_config: if not isinstance(branch_protection_config["required_approving_review_count"], int): - self.errors.append("Field 'required_approving_review_count' must be an integer") + self.errors.append("Field 'branch-protection.required_approving_review_count' must be an integer") def _validate_repositories(self, repositories: Any) -> None: """Validate repositories configuration.""" @@ -150,7 +150,7 @@ def _validate_single_repository(self, repo_name: str, repo_config: Any) -> None: "name", "log-level", "log-file", - "slack_webhook_url", + "slack-webhook-url", "tox-python-version", "conventional-title", ] @@ -159,7 +159,7 @@ def _validate_single_repository(self, repo_name: str, repo_config: Any) -> None: self.errors.append(f"Repository '{repo_name}' field '{field}' must be a string") # Boolean fields - boolean_fields = ["verified_job", "pre-commit"] + boolean_fields = ["verified-job", "pre-commit"] for field in boolean_fields: if field in repo_config and not isinstance(repo_config[field], bool): self.errors.append(f"Repository '{repo_name}' field '{field}' must be a boolean") diff --git a/webhook_server/tests/test_webhook.py b/webhook_server/tests/test_webhook.py index 23c0cf23..727350bb 100644 --- a/webhook_server/tests/test_webhook.py +++ b/webhook_server/tests/test_webhook.py @@ -320,7 +320,7 @@ def mock_config(self) -> Mock: """Mock Config object for testing.""" config = Mock() config.root_data = { - "webhook_ip": "http://example.com", + "webhook-ip": "http://example.com", "repositories": { "repo1": {"name": "owner/repo1", "events": ["push"]}, "repo2": {"name": "owner/repo2", "events": ["pull_request"]}, @@ -424,7 +424,7 @@ def test_create_webhook_empty_repositories( ) -> None: """Test webhook creation with no repositories.""" config = Mock() - config.root_data = {"webhook_ip": "http://example.com", "repositories": {}} + config.root_data = {"webhook-ip": "http://example.com", "repositories": {}} mock_executor = Mock() mock_thread_pool.return_value.__enter__.return_value = mock_executor diff --git a/webhook_server/utils/github_repository_settings.py b/webhook_server/utils/github_repository_settings.py index c44de72d..007646c4 100644 --- a/webhook_server/utils/github_repository_settings.py +++ b/webhook_server/utils/github_repository_settings.py @@ -124,7 +124,7 @@ def get_required_status_checks( if data.get("tox"): default_status_checks.append("tox") - if data.get("verified_job", True): + if data.get("verified-job", True): default_status_checks.append("verified") if data.get("container"): @@ -169,25 +169,25 @@ def set_repository_labels(repository: Repository, api_user: str) -> str: "color": label.color, } - for label, color in STATIC_LABELS_DICT.items(): - label_lower: str = label.lower() + for label_name, label_color in STATIC_LABELS_DICT.items(): + label_lower: str = label_name.lower() if label_lower in repository_labels: repo_label: Label = repository_labels[label_lower]["object"] - if repository_labels[label_lower]["color"] == color: + if repository_labels[label_lower]["color"] == label_color: continue else: - LOGGER.debug(f"{repository.name}: Edit repository label {label} with color {color}") - repo_label.edit(name=repo_label.name, color=color) + LOGGER.debug(f"{repository.name}: Edit repository label {label_name} with color {label_color}") + repo_label.edit(name=repo_label.name, color=label_color) else: - LOGGER.debug(f"{repository.name}: Add repository label {label} with color {color}") - repository.create_label(name=label, color=color) + LOGGER.debug(f"{repository.name}: Add repository label {label_name} with color {label_color}") + repository.create_label(name=label_name, color=label_color) return f"[API user {api_user}] - {repository}: Setting repository labels is done" def get_repo_branch_protection_rules(config: Config) -> dict[str, Any]: branch_protection = copy.deepcopy(DEFAULT_BRANCH_PROTECTION) - repo_branch_protection = config.get_value(value="branch_protection", return_on_none={}) + repo_branch_protection = config.get_value(value="branch-protection", return_on_none={}) branch_protection.update(repo_branch_protection) return branch_protection diff --git a/webhook_server/utils/webhook.py b/webhook_server/utils/webhook.py index 015ddc27..8cb79c91 100644 --- a/webhook_server/utils/webhook.py +++ b/webhook_server/utils/webhook.py @@ -70,7 +70,7 @@ def process_github_webhook( def create_webhook(config: Config, apis_dict: dict[str, dict[str, Any]], secret: str | None = None) -> None: LOGGER.info("Preparing webhook configuration") - webhook_ip = config.root_data["webhook_ip"] + webhook_ip = config.root_data["webhook-ip"] futures = [] with ThreadPoolExecutor() as executor: