-
Notifications
You must be signed in to change notification settings - Fork 80
LCORE-1608: improved docstrings in unit tests #1457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,11 @@ | |
|
|
||
| @pytest.fixture(name="token_file") | ||
| def token_file_fixture(tmp_path: Path) -> Path: | ||
| """Create a temporary token file for testing.""" | ||
| """Create a temporary token file for testing. | ||
|
|
||
| Returns: | ||
| Path: Path to the created token file. | ||
| """ | ||
| token_file = tmp_path / "token" | ||
| token_file.write_text("test-token") | ||
| return token_file | ||
|
|
@@ -70,7 +74,15 @@ def test_enabled_missing_required_fields( | |
|
|
||
|
|
||
| def test_valid_enabled_configuration(token_file: Path) -> None: | ||
| """Test valid enabled Splunk configuration passes validation.""" | ||
| """Test valid enabled Splunk configuration passes validation. | ||
|
|
||
| Verify that an enabled SplunkConfiguration with all required fields is | ||
| accepted and preserves the provided values. | ||
|
|
||
| Asserts that each field (enabled, url, token_path, index, source, timeout, | ||
| verify_ssl) on the created configuration equals the value passed to the | ||
| constructor. | ||
| """ | ||
|
Comment on lines
+77
to
+85
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider maintaining consistent docstring verbosity within the file. The expanded docstring provides detailed information but creates a notable verbosity inconsistency compared to other test functions in this file (lines 24, 36, 66, 106, 114, 122), which all use concise one-line docstrings. For better maintainability and readability, consider either:
Additionally, enumerating every assertion in the docstring may duplicate information already clear from the test code itself, potentially adding maintenance overhead when assertions change. 🤖 Prompt for AI Agents |
||
| cfg = SplunkConfiguration( | ||
| enabled=True, | ||
| url="https://splunk.example.com:8088", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider standardizing exception documentation style.
The docstrings use varying phrasings and formatting for expected exceptions:
pydantic.ValidationError" (with backticks)For consistency and alignment with Python docstring conventions, consider adopting the structured "Raises:" format used in
test_byok_rag_configuration_empty_rag_type(lines 108-116) across all validation tests.📝 Example standardized format
Also applies to: 88-94, 131-137, 152-158
🤖 Prompt for AI Agents