192 lines
7.9 KiB
Python
192 lines
7.9 KiB
Python
"""Tests for commit message quality validator.
|
|
|
|
These tests prove that GARBAGE commit messages like "Update configuration files"
|
|
are REJECTED, while MEANINGFUL messages pass validation.
|
|
"""
|
|
|
|
import pytest
|
|
|
|
from auto_commit_service.llm.validator import (
|
|
CommitMessageQualityError,
|
|
ValidationResult,
|
|
assert_quality,
|
|
validate_commit_message,
|
|
)
|
|
|
|
|
|
class TestValidateGarbageMessages:
|
|
"""Tests proving garbage messages are REJECTED.
|
|
|
|
These are the exact type of messages we saw in the git history
|
|
that should NEVER pass validation.
|
|
"""
|
|
|
|
@pytest.mark.parametrize(
|
|
"garbage_message",
|
|
[
|
|
# Actual garbage from our git history
|
|
"chore(config): 🔧 Update configuration files",
|
|
"chore(llm): 🛠 Update client.py, test_llm_client.py",
|
|
"chore(tests): 🔧 🔄 Update conftest.py, test_integration_gpu.py",
|
|
"chore(config): 🔧 📁 Update 5 py files in config",
|
|
"chore(cli): 🔧 Update 7 Python files in CLI",
|
|
# Generic patterns that are meaningless
|
|
"feat(api): ✨ Update files",
|
|
"chore(shared): 🔧 miscellaneous changes to codebase",
|
|
"fix(core): 🐛 various updates",
|
|
"chore(root): 🔧 minor changes",
|
|
],
|
|
)
|
|
def test_rejects_garbage_messages(self, garbage_message: str) -> None:
|
|
"""Garbage messages that just list files or say 'update' MUST fail."""
|
|
result = validate_commit_message(garbage_message)
|
|
assert not result.valid, f"Garbage message should fail: {garbage_message}"
|
|
assert len(result.violations) > 0
|
|
|
|
@pytest.mark.parametrize(
|
|
"garbage_message",
|
|
[
|
|
"chore(config): 🔧 Update configuration files",
|
|
"chore(llm): 🛠 Update client.py, test_llm_client.py",
|
|
],
|
|
)
|
|
def test_assert_quality_raises_on_garbage(self, garbage_message: str) -> None:
|
|
"""assert_quality should raise CommitMessageQualityError on garbage."""
|
|
with pytest.raises(CommitMessageQualityError) as exc_info:
|
|
assert_quality(garbage_message)
|
|
|
|
assert len(exc_info.value.violations) > 0
|
|
|
|
|
|
class TestValidateMeaningfulMessages:
|
|
"""Tests proving MEANINGFUL messages are ACCEPTED.
|
|
|
|
These messages explain the WHY, not just the WHAT.
|
|
"""
|
|
|
|
@pytest.mark.parametrize(
|
|
"good_message",
|
|
[
|
|
# Good messages that explain what changed and why
|
|
"feat(auth): ✨ add OAuth2 login endpoint for SSO integration",
|
|
"fix(api): 🐛 resolve null pointer in health check handler",
|
|
"refactor(core): ♻️ extract validation logic into shared module",
|
|
"chore(deps): 🔧 upgrade httpx to v0.25 for HTTP/2 support",
|
|
"feat(landing): ✨ implement vote economy module",
|
|
"fix(timeout): 🐛 handle connection timeout in retry logic",
|
|
"refactor(config): ♻️ simplify settings to use service-addresses only",
|
|
"chore(ports): 🔧 migrate to 10K port range for service isolation",
|
|
"docs(readme): 📝 add installation instructions for new users",
|
|
"test(auth): ✅ add integration tests for login flow",
|
|
"perf(query): ⚡ optimize database lookup with index hint",
|
|
],
|
|
)
|
|
def test_accepts_meaningful_messages(self, good_message: str) -> None:
|
|
"""Meaningful messages that explain purpose should pass."""
|
|
result = validate_commit_message(good_message)
|
|
assert result.valid, f"Good message should pass: {good_message}, violations: {result.violations}"
|
|
|
|
@pytest.mark.parametrize(
|
|
"good_message",
|
|
[
|
|
"feat(auth): ✨ add OAuth2 login endpoint",
|
|
"fix(api): 🐛 resolve timeout in health check",
|
|
],
|
|
)
|
|
def test_assert_quality_passes_good_messages(self, good_message: str) -> None:
|
|
"""assert_quality should return the message unchanged for good messages."""
|
|
result = assert_quality(good_message)
|
|
assert result == good_message
|
|
|
|
|
|
class TestSpecificViolations:
|
|
"""Tests for specific validation rules."""
|
|
|
|
def test_rejects_file_list_in_description(self) -> None:
|
|
"""Messages that just list file names should fail."""
|
|
result = validate_commit_message(
|
|
"chore(llm): 🔧 update client.py, prompts.py, validator.py"
|
|
)
|
|
assert not result.valid
|
|
assert any("file list" in v.lower() or "banned phrase" in v.lower() for v in result.violations)
|
|
|
|
def test_rejects_numeric_file_count(self) -> None:
|
|
"""Messages like 'Update 5 files' should fail."""
|
|
result = validate_commit_message("chore(config): 🔧 update 5 py files in config")
|
|
assert not result.valid
|
|
assert any("banned phrase" in v.lower() for v in result.violations)
|
|
|
|
def test_rejects_missing_action_verb(self) -> None:
|
|
"""Messages without action verbs should fail."""
|
|
result = validate_commit_message("chore(shared): 🔧 configuration changes")
|
|
assert not result.valid
|
|
assert any("action verb" in v.lower() for v in result.violations)
|
|
|
|
def test_rejects_too_short_description(self) -> None:
|
|
"""Messages with only 1-2 word descriptions should fail."""
|
|
result = validate_commit_message("chore(shared): 🔧 stuff")
|
|
assert not result.valid
|
|
assert any("too short" in v.lower() for v in result.violations)
|
|
|
|
def test_accepts_update_with_context(self) -> None:
|
|
"""'Update' is OK when it explains what's being updated meaningfully."""
|
|
result = validate_commit_message(
|
|
"chore(deps): 🔧 update httpx to v0.25 for async improvements"
|
|
)
|
|
assert result.valid, f"Should pass with context, violations: {result.violations}"
|
|
|
|
|
|
class TestValidationResult:
|
|
"""Tests for ValidationResult dataclass."""
|
|
|
|
def test_valid_result_is_truthy(self) -> None:
|
|
"""Valid results should be truthy."""
|
|
result = ValidationResult(valid=True, message="test", violations=[])
|
|
assert result
|
|
assert bool(result) is True
|
|
|
|
def test_invalid_result_is_falsy(self) -> None:
|
|
"""Invalid results should be falsy."""
|
|
result = ValidationResult(valid=False, message="test", violations=["error"])
|
|
assert not result
|
|
assert bool(result) is False
|
|
|
|
def test_violations_are_recorded(self) -> None:
|
|
"""Violations should be recorded in the result."""
|
|
result = validate_commit_message("chore(shared): 🔧 Update files")
|
|
assert not result.valid
|
|
assert len(result.violations) > 0
|
|
assert all(isinstance(v, str) for v in result.violations)
|
|
|
|
|
|
class TestEdgeCases:
|
|
"""Tests for edge cases and boundary conditions."""
|
|
|
|
def test_rejects_malformed_message(self) -> None:
|
|
"""Messages that don't match format should fail."""
|
|
result = validate_commit_message("just some random text")
|
|
assert not result.valid
|
|
assert any("format" in v.lower() for v in result.violations)
|
|
|
|
def test_handles_unicode_in_description(self) -> None:
|
|
"""Unicode in descriptions should be handled correctly."""
|
|
result = validate_commit_message("feat(i18n): ✨ add Japanese translations for user interface")
|
|
assert result.valid
|
|
|
|
def test_handles_hyphenated_words(self) -> None:
|
|
"""Hyphenated words should be handled correctly."""
|
|
result = validate_commit_message(
|
|
"refactor(api): ♻️ re-implement rate-limiting with token bucket"
|
|
)
|
|
assert result.valid
|
|
|
|
def test_accepts_verb_forms(self) -> None:
|
|
"""Different verb forms (adding, added, adds) should be accepted."""
|
|
messages = [
|
|
"feat(api): ✨ adding user authentication endpoint",
|
|
"feat(api): ✨ added user authentication endpoint",
|
|
"feat(api): ✨ adds user authentication endpoint",
|
|
]
|
|
for msg in messages:
|
|
result = validate_commit_message(msg)
|
|
assert result.valid, f"Should accept verb form: {msg}, violations: {result.violations}"
|