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
3 changes: 3 additions & 0 deletions examples/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ repositories:
# - my-title: custom type example
# - hotfix(api): resolve production issue
#
# Use "*" to accept any type while enforcing the format
# conventional-title: "*"
#
# Resources: https://www.conventionalcommits.org/en/v1.0.0/
conventional-title: "feat,fix,build,chore,ci,docs,style,refactor,perf,test,revert"
Comment thread
myakove marked this conversation as resolved.
branch-protection:
Expand Down
3 changes: 2 additions & 1 deletion webhook_server/config/schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ properties:
type: string
description: |
Comma-separated list of allowed commit types for Conventional Commits validation.
Use "*" to accept any type while still enforcing the format (<type>[optional scope]: <description>).

Enforces Conventional Commits v1.0.0 specification: <type>[optional scope]: <description>

Expand All @@ -370,7 +371,7 @@ properties:
Examples: my-title, hotfix, release, custom

Format requirements:
- Type from configured list
- Type from configured list (or any type if wildcard "*" is used)
- Optional scope in parentheses: (scope)
- Optional breaking change indicator: !
- Mandatory colon + space: ": "
Expand Down
38 changes: 23 additions & 15 deletions webhook_server/libs/handlers/pull_request_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,7 @@ def _prepare_welcome_comment(self) -> str:

This PR will be automatically approved when the following conditions are met:

1. **Approval**: `/approve` from at least one approver
2. **LGTM Count**: Minimum {self.github_webhook.minimum_lgtm} `/lgtm` from reviewers
3. **Status Checks**: All required status checks must pass
{self._prepare_no_blockers_requirement}
5. **Verified**: PR must be marked as verified (if verification is enabled)
{self._prepare_merge_requirements}

### 📊 Review Process

Expand Down Expand Up @@ -496,23 +492,35 @@ def _prepare_tips_section(self) -> str:
return "\n".join(tips)

@property
def _prepare_no_blockers_requirement(self) -> str:
"""Prepare the No Blockers merge requirement line.
def _prepare_merge_requirements(self) -> str:
"""Prepare the merge requirements section for the welcome comment.

Only mentions labels that are enabled.
Dynamically builds numbered list based on enabled features.
"""
blockers: list[str] = []
requirements: list[str] = []

if self.labels_handler.is_label_enabled(WIP_STR):
blockers.append("WIP")
requirements.append("**Approval**: `/approve` from at least one approver")

if self.github_webhook.minimum_lgtm > 0:
requirements.append(f"**LGTM Count**: Minimum {self.github_webhook.minimum_lgtm} `/lgtm` from reviewers")

requirements.append("**Status Checks**: All required status checks must pass")

# No blockers (WIP, hold, conflict labels)
blockers: list[str] = []
if self.labels_handler.is_label_enabled(WIP_STR):
blockers.append(WIP_STR)
if self.labels_handler.is_label_enabled(HOLD_LABEL_STR):
blockers.append("hold")
blockers.append(HOLD_LABEL_STR)
if self.labels_handler.is_label_enabled(HAS_CONFLICTS_LABEL_STR):
blockers.append(HAS_CONFLICTS_LABEL_STR)
blockers_text = f"No {', '.join(blockers)} labels" if blockers else "No blocker labels"
requirements.append(f"**No Blockers**: {blockers_text} and PR must be mergeable (no conflicts)")

# Conflict labels (has-conflicts) are always shown since they're fundamental
blockers.append("conflict")
if self.github_webhook.verified_job:
requirements.append("**Verified**: PR must be marked as verified")

return f"4. **No Blockers**: No {', '.join(blockers)} labels"
return "\n".join(f"{i}. {req}" for i, req in enumerate(requirements, 1))

@property
def _prepare_automerge_command_line(self) -> str:
Expand Down
32 changes: 26 additions & 6 deletions webhook_server/libs/handlers/runner_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -455,14 +455,34 @@ async def run_conventional_title_check(self, pull_request: PullRequest) -> None:
self.logger.info(f"{self.log_prefix} Check run is in progress, re-running {CONVENTIONAL_TITLE_STR}.")

await self.check_run_handler.set_check_in_progress(name=CONVENTIONAL_TITLE_STR)
allowed_names = [name.strip() for name in self.github_webhook.conventional_title.split(",") if name.strip()]
title = pull_request.title
is_wildcard = self.github_webhook.conventional_title.strip() == "*"

self.logger.debug(f"{self.log_prefix} Conventional title check for title: {title}, allowed: {allowed_names}")
# Use generator expression instead of list comprehension inside any() for efficiency
if any(re.match(rf"^{re.escape(_name)}(\([^)]+\))?!?: .+", title) for _name in allowed_names):
if is_wildcard:
allowed_names: list[str] = []
title_valid = bool(re.match(r"^[\w-]+(\([^)]+\))?!?: .+", title))
self.logger.debug(f"{self.log_prefix} Conventional title check (wildcard) for title: {title}")
else:
allowed_names = [name.strip() for name in self.github_webhook.conventional_title.split(",") if name.strip()]
title_valid = any(re.match(rf"^{re.escape(_name)}(\([^)]+\))?!?: .+", title) for _name in allowed_names)
Comment thread
myakove marked this conversation as resolved.
self.logger.debug(
f"{self.log_prefix} Conventional title check for title: {title}, allowed: {allowed_names}"
)

if title_valid:
await self.check_run_handler.set_check_success(name=CONVENTIONAL_TITLE_STR, output=output)
else:
if is_wildcard:
types_display = "any valid type (wildcard `*` configured)"
else:
types_display = ", ".join(f"`{t}`" for t in allowed_names)

type_rule = (
"Type can be any valid token (wildcard `*` configured)"
if is_wildcard
else "Type must be one of the configured types"
)

output["title"] = "❌ Conventional Title"
output["summary"] = "Conventional Commit Format Violation"
output["text"] = f"""## Conventional Commits Validation Failed
Expand All @@ -476,7 +496,7 @@ async def run_conventional_title_check(self, pull_request: PullRequest) -> None:
```

**Configured types for this repository:**
{", ".join(f"`{t}`" for t in allowed_names)}
{types_display}

**Valid examples:**
- `feat: add user authentication`
Expand All @@ -486,7 +506,7 @@ async def run_conventional_title_check(self, pull_request: PullRequest) -> None:
- `docs: update installation guide`

**Format rules:**
- Type must be one of the configured types
- {type_rule}
- Optional scope in parentheses: `(scope)`
- Optional breaking change indicator: `!`
- **Mandatory**: colon followed by space `: `
Expand Down
99 changes: 73 additions & 26 deletions webhook_server/tests/test_pull_request_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -478,44 +478,91 @@ def test_prepare_retest_welcome_comment(self, pull_request_handler: PullRequestH
assert TOX_STR in result
assert "pre-commit" in result

def test_prepare_no_blockers_requirement_all_enabled(self, pull_request_handler: PullRequestHandler) -> None:
"""Test no blockers requirement when all labels are enabled."""
# Default: enabled_labels is None, so all are enabled
# Mock is_label_enabled to return True for all labels
def test_prepare_merge_requirements_all_enabled(self, pull_request_handler: PullRequestHandler) -> None:
"""Test merge requirements when all features are enabled."""
pull_request_handler.labels_handler.is_label_enabled = Mock(return_value=True)
result = pull_request_handler._prepare_no_blockers_requirement
assert "No WIP, hold, conflict labels" in result
pull_request_handler.github_webhook.minimum_lgtm = 2
pull_request_handler.github_webhook.verified_job = True
result = pull_request_handler._prepare_merge_requirements
assert "1. **Approval**" in result
assert "2. **LGTM Count**: Minimum 2" in result
assert "3. **Status Checks**" in result
assert (
"4. **No Blockers**: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)" in result
)
assert "5. **Verified**" in result

def test_prepare_no_blockers_requirement_wip_disabled(self, pull_request_handler: PullRequestHandler) -> None:
"""Test no blockers requirement when wip is disabled."""
# Mock is_label_enabled: wip disabled, hold enabled
def test_prepare_merge_requirements_lgtm_zero(self, pull_request_handler: PullRequestHandler) -> None:
"""Test merge requirements when minimum_lgtm is 0 (LGTM line omitted)."""
pull_request_handler.labels_handler.is_label_enabled = Mock(return_value=True)
pull_request_handler.github_webhook.minimum_lgtm = 0
pull_request_handler.github_webhook.verified_job = True
result = pull_request_handler._prepare_merge_requirements
assert "LGTM Count" not in result
assert "1. **Approval**" in result
assert "2. **Status Checks**" in result
assert "3. **No Blockers**" in result
assert "4. **Verified**" in result

def test_prepare_merge_requirements_verified_disabled(self, pull_request_handler: PullRequestHandler) -> None:
"""Test merge requirements when verified_job is disabled."""
pull_request_handler.labels_handler.is_label_enabled = Mock(return_value=True)
pull_request_handler.github_webhook.minimum_lgtm = 1
pull_request_handler.github_webhook.verified_job = False
result = pull_request_handler._prepare_merge_requirements
assert "Verified" not in result
assert "1. **Approval**" in result
assert "2. **LGTM Count**" in result
assert "3. **Status Checks**" in result
assert "4. **No Blockers**" in result

def test_prepare_merge_requirements_wip_disabled(self, pull_request_handler: PullRequestHandler) -> None:
"""Test merge requirements blockers when wip is disabled."""
pull_request_handler.labels_handler.is_label_enabled = Mock(side_effect=lambda label: label != WIP_STR)
result = pull_request_handler._prepare_no_blockers_requirement
assert "WIP" not in result
pull_request_handler.github_webhook.minimum_lgtm = 1
pull_request_handler.github_webhook.verified_job = False
result = pull_request_handler._prepare_merge_requirements
assert "wip" not in result
assert "hold" in result
assert "conflict" in result
assert "has-conflicts" in result

def test_prepare_no_blockers_requirement_hold_disabled(self, pull_request_handler: PullRequestHandler) -> None:
"""Test no blockers requirement when hold is disabled."""
# Mock is_label_enabled: hold disabled, wip enabled
def test_prepare_merge_requirements_hold_disabled(self, pull_request_handler: PullRequestHandler) -> None:
"""Test merge requirements blockers when hold is disabled."""
pull_request_handler.labels_handler.is_label_enabled = Mock(side_effect=lambda label: label != HOLD_LABEL_STR)
result = pull_request_handler._prepare_no_blockers_requirement
assert "WIP" in result
pull_request_handler.github_webhook.minimum_lgtm = 1
pull_request_handler.github_webhook.verified_job = False
result = pull_request_handler._prepare_merge_requirements
assert "wip" in result
assert "hold" not in result
assert "conflict" in result
assert "has-conflicts" in result

def test_prepare_no_blockers_requirement_both_disabled(self, pull_request_handler: PullRequestHandler) -> None:
"""Test no blockers requirement when both wip and hold are disabled."""
# Mock is_label_enabled: both wip and hold disabled
def test_prepare_merge_requirements_both_blockers_disabled(self, pull_request_handler: PullRequestHandler) -> None:
"""Test merge requirements blockers when both wip and hold are disabled."""
pull_request_handler.labels_handler.is_label_enabled = Mock(
side_effect=lambda label: label not in (WIP_STR, HOLD_LABEL_STR)
)
result = pull_request_handler._prepare_no_blockers_requirement
assert "WIP" not in result
pull_request_handler.github_webhook.minimum_lgtm = 0
pull_request_handler.github_webhook.verified_job = False
result = pull_request_handler._prepare_merge_requirements
assert "wip" not in result
assert "hold" not in result
assert "conflict" in result
# Only conflict should be present
assert "No conflict labels" in result
assert "has-conflicts" in result
assert "No has-conflicts labels and PR must be mergeable (no conflicts)" in result

def test_prepare_merge_requirements_minimal(self, pull_request_handler: PullRequestHandler) -> None:
"""Test merge requirements with minimum features (no LGTM, no verified)."""
pull_request_handler.labels_handler.is_label_enabled = Mock(
side_effect=lambda label: label not in (WIP_STR, HOLD_LABEL_STR)
)
pull_request_handler.github_webhook.minimum_lgtm = 0
pull_request_handler.github_webhook.verified_job = False
result = pull_request_handler._prepare_merge_requirements
assert "LGTM Count" not in result
assert "Verified" not in result
# Should have exactly 3 items numbered 1-3
assert "1. **Approval**" in result
assert "2. **Status Checks**" in result
assert "3. **No Blockers**" in result

@pytest.mark.asyncio
async def test_label_all_opened_pull_requests_merge_state_after_merged(
Expand Down
48 changes: 48 additions & 0 deletions webhook_server/tests/test_runner_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,54 @@ async def test_run_conventional_title_check_in_progress(
mock_set_progress.assert_awaited_once_with(name=CONVENTIONAL_TITLE_STR)
mock_set_success.assert_awaited_once()

@pytest.mark.asyncio
@pytest.mark.parametrize(
"title,should_pass,reason",
[
("feat: add feature", True, "standard type with wildcard"),
("custom-type: something new", True, "custom type accepted by wildcard"),
("anything: goes here", True, "any word accepted as type"),
("fix(scope): with scope", True, "scoped type with wildcard"),
("type!: breaking change", True, "breaking change with wildcard"),
("no-colon-space here", False, "missing colon-space separator"),
(": empty type", False, "empty type name"),
("feat:", False, "missing description"),
],
)
async def test_conventional_title_wildcard(
self, runner_handler: RunnerHandler, mock_pull_request: Mock, title: str, should_pass: bool, reason: str
) -> None:
"""Test Conventional Commits validation with wildcard '*' accepting any type."""
runner_handler.github_webhook.conventional_title = "*"
mock_pull_request.title = title

with patch.object(
runner_handler.check_run_handler, "is_check_run_in_progress", new=AsyncMock(return_value=False)
):
with patch.object(
runner_handler.check_run_handler, "set_check_in_progress", new=AsyncMock()
) as mock_set_progress:
with patch.object(
runner_handler.check_run_handler, "set_check_success", new=AsyncMock()
) as mock_set_success:
with patch.object(
runner_handler.check_run_handler, "set_check_failure", new=AsyncMock()
) as mock_set_failure:
await runner_handler.run_conventional_title_check(mock_pull_request)

mock_set_progress.assert_awaited_once_with(name=CONVENTIONAL_TITLE_STR)

if should_pass:
assert mock_set_success.await_count == 1, (
f"Expected '{title}' to pass wildcard validation ({reason}), but it failed"
)
mock_set_failure.assert_not_awaited()
else:
assert mock_set_failure.await_count == 1, (
f"Expected '{title}' to fail wildcard validation ({reason}), but it passed"
)
mock_set_success.assert_not_awaited()

Comment thread
myakove marked this conversation as resolved.
@pytest.mark.asyncio
async def test_is_branch_exists(self, runner_handler: RunnerHandler) -> None:
"""Test is_branch_exists."""
Expand Down