Genio Preperation/Tests Module #6262
Conversation
Add comprehensive test coverage for the genio module: - 21 unit tests covering file read/write, pattern matching, error handling - 13 functional tests for real-world workflow scenarios - Update TEST_SIZE counts in check.py (unit: 950, functional-parallel: 366) Reference: avocado-framework/aautils#51 Assisted-By: Cursor-Claude-4-Sonnet Signed-off-by: Harvey Lynden <hlynden@redhat.com>
- Enhance RST docstrings for all public functions - Add deprecation warning for autils migration - Remove from .pylintrc_utils ignore list to enable pylint checking - Add to autils_migration_announcement workflow - Update spell.ignore with new technical terms Reference: avocado-framework/aautils#51 Assisted-By: Cursor-Claude-4-Sonnet Signed-off-by: Harvey Lynden <hlynden@redhat.com>
WalkthroughThis pull request updates the genio module by removing it from pylint's ignore list and adding it to the migration announcement workflow. The genio.py source file receives comprehensive docstring refinements, clarification of function behaviors (ask now returns "y" when auto=True, is_pattern_in_file uses regex search), a deprecation warning on import, and exception documentation. Test coverage is expanded with new functional test suites covering file operations, pattern matching, file comparison, and real-world workflows, while existing unit tests are refactored. Test size expectations are updated accordingly, and spell-check ignore entries are added for new terms. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @harvey0100, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the genio module by enhancing its documentation and test coverage. The docstrings for most functions in avocado/utils/genio.py have been updated with more detailed descriptions, parameter/return types, raises directives, and Example:: blocks, greatly improving clarity. The unit tests in selftests/unit/utils/genio.py have been completely refactored into more granular, function-specific test classes, covering a wider range of scenarios and edge cases. Additionally, a new file selftests/functional/utils/genio.py introduces functional tests that validate the module's behavior in real-world workflows. The removal of genio.py from the pylint ignore list ensures future static analysis, and minor updates to selftests/check.py and spell.ignore complete the changes. These improvements collectively enhance the robustness, maintainability, and usability of the genio module.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @selftests/check.py:
- Around line 29-31: The TEST_SIZE constant is out of sync: update the "unit"
entry in TEST_SIZE (referenced as TEST_SIZE or the dict containing "unit") to
match the real count of tests (959) so it reflects the actual number of tests in
selftests/unit/ (also verify genio.py additions were fully counted); change the
value from 950 to 959 and run the unit test counter or ls on selftests/unit to
confirm no other discrepancies.
🧹 Nitpick comments (2)
selftests/unit/utils/genio.py (2)
85-102: Consider using a more portable path for cross-platform testing.Line 102 tests error handling by attempting to read "/" as a file. While this works on Unix-like systems, it may behave differently on Windows (where "/" might be interpreted as the root of the current drive).
🔎 Suggested improvement for cross-platform compatibility
def test_read_all_lines_graceful_on_error(self): """Test that errors return empty list instead of raising.""" # Nonexistent file self.assertEqual(genio.read_all_lines("/nonexistent/path/file.txt"), []) - # Directory path causes error - self.assertEqual(genio.read_all_lines("/"), []) + # Directory path causes error - use temp dir for cross-platform compatibility + import tempfile + tmpdir = tempfile.mkdtemp() + self.assertEqual(genio.read_all_lines(tmpdir), []) + os.rmdir(tmpdir)
210-232: Consider using try/finally for cleanup.The directory cleanup at line 230 should be in a finally block to ensure it runs even if the assertion fails.
🔎 Suggested improvement for guaranteed cleanup
def test_pattern_in_file_raises_for_invalid_file(self): """Test that GenIOError is raised for directory or nonexistent file.""" prefix = temp_dir_prefix(self) tempdirname = tempfile.mkdtemp(prefix=prefix) - with self.assertRaises(genio.GenIOError): - genio.is_pattern_in_file(tempdirname, "something") - os.rmdir(tempdirname) + try: + with self.assertRaises(genio.GenIOError): + genio.is_pattern_in_file(tempdirname, "something") + finally: + os.rmdir(tempdirname) with self.assertRaises(genio.GenIOError): genio.is_pattern_in_file("/nonexistent/file.txt", "pattern")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/autils_migration_announcement.yml.pylintrc_utilsavocado/utils/genio.pyselftests/check.pyselftests/functional/utils/genio.pyselftests/unit/utils/genio.pyspell.ignore
🧰 Additional context used
🧬 Code graph analysis (3)
selftests/functional/utils/genio.py (2)
selftests/utils.py (1)
TestCaseTmpDir(167-174)avocado/utils/genio.py (7)
write_file(156-174)read_one_line(75-94)append_one_line(240-259)read_all_lines(97-125)read_line_with_matching_pattern(128-153)are_files_equal(291-312)append_file(220-237)
selftests/unit/utils/genio.py (1)
avocado/utils/genio.py (12)
GenIOError(30-31)ask(34-52)read_one_line(75-94)read_all_lines(97-125)read_line_with_matching_pattern(128-153)write_file(156-174)write_one_line(177-194)write_file_or_fail(197-217)append_file(220-237)append_one_line(240-259)is_pattern_in_file(262-288)are_files_equal(291-312)
avocado/utils/genio.py (1)
avocado/utils/deprecation.py (1)
warning(18-25)
🪛 Ruff (0.14.10)
selftests/functional/utils/genio.py
54-54: String contains ambiguous р (CYRILLIC SMALL LETTER ER). Did you mean p (LATIN SMALL LETTER P)?
(RUF001)
54-54: String contains ambiguous е (CYRILLIC SMALL LETTER IE). Did you mean e (LATIN SMALL LETTER E)?
(RUF001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (41)
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: Linux with Python 3.9
- GitHub Check: Linux with Python 3.10
- GitHub Check: Linux with Python 3.12
- GitHub Check: Linux with Python 3.13
- GitHub Check: Linux with Python 3.11
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: Egg task ubuntu:24.04
- GitHub Check: Fedora selftests
- GitHub Check: Version task ubuntu:22.04
- GitHub Check: Windows with Python 3.13
- GitHub Check: Egg task fedora:40
- GitHub Check: Podman spawner with 3rd party runner plugin
- GitHub Check: Version task ubi:8.8
- GitHub Check: Static checks
- GitHub Check: macOS with Python 3.11
- GitHub Check: Version task debian:12.4
- GitHub Check: Code Coverage (3.11)
🔇 Additional comments (27)
spell.ignore (1)
823-828: LGTM! Appropriate technical terms added.All six new entries are legitimate technical terms related to the genio module changes and testing infrastructure.
.pylintrc_utils (1)
10-10: LGTM! Enabling pylint analysis for genio.py.Removing genio.py from the ignore list appropriately enables pylint checking as part of the autils migration preparation.
.github/workflows/autils_migration_announcement.yml (1)
15-15: LGTM! Workflow updated to track genio.py changes.Adding genio.py to the migration announcement workflow appropriately ensures PRs modifying this module will trigger migration notifications.
selftests/unit/utils/genio.py (12)
1-10: LGTM! Clean imports and setup.The addition of
unittest.mockis appropriate for testing user interaction in theaskfunction.
12-19: LGTM! Comprehensive GenIOError validation.Good coverage of exception subclassing and message handling.
22-35: LGTM! Well-tested ask function behavior.Excellent use of mocking to test both auto-return and user-prompt scenarios without requiring actual user input.
38-61: LGTM! Thorough read_file testing.Good coverage including multiline content, unicode characters (日本語 🎉), empty files, and error cases.
64-82: LGTM! Proper read_one_line testing.Tests correctly verify newline stripping and empty file handling.
105-125: LGTM! Pattern matching tests are comprehensive.Good coverage of both matching and non-matching scenarios with proper newline verification.
128-143: LGTM! Excellent write_file testing with proper cleanup.Tests cover creation, overwriting, and unicode handling with appropriate temporary directory management.
146-159: LGTM! Thorough newline normalization testing.Correctly verifies that write_one_line adds exactly one newline regardless of input formatting.
162-178: LGTM! Good GenIOError raising verification.Tests both success and failure paths with appropriate exception message validation.
181-194: LGTM! Comprehensive append testing.Tests cover both creating new files and appending to existing files.
197-207: LGTM! Proper append_one_line testing.Correctly verifies single newline addition and normalization behavior.
235-254: LGTM! Thorough file equality testing.Good coverage of identical content, same file comparison, and different content scenarios with proper cleanup.
selftests/functional/utils/genio.py (5)
14-66: LGTM! Comprehensive file operations coverage.The test suite effectively validates core file I/O operations with excellent edge case coverage including large files, unicode content, and special characters. The tests are well-isolated using temporary directories and verify exact content matching.
68-112: LGTM! Pattern matching tests are well-designed.The tests correctly exercise both substring matching (
read_line_with_matching_pattern) and regex matching (is_pattern_in_file) functions with realistic scenarios like log analysis and configuration file parsing.
114-133: LGTM! File comparison workflow is well-tested.The test effectively validates both positive and negative cases for file equality checking, simulating a realistic copy-verification workflow.
135-197: LGTM! Real-world scenarios provide excellent integration testing.The workflow tests effectively simulate realistic use cases: configuration management, log analysis, incremental file building, and structured data verification. These tests provide valuable end-to-end validation of the genio module.
51-57: Static analysis hint is a false positive.Ruff flags the Cyrillic characters on line 54 as potentially ambiguous, but this is intentional. The test explicitly validates unicode handling with diverse character sets (Japanese, Cyrillic, emoji), which is exactly what this test method is designed to verify.
avocado/utils/genio.py (7)
30-52: LGTM! Docstring improvements enhance clarity.The enhanced RST-format docstrings for
GenIOErrorandaskprovide clearer documentation of behavior and parameters. The explicit note thatauto=Truereturns "y" is particularly helpful.
55-94: LGTM! Exception documentation improves API clarity.The enhanced docstrings now explicitly document exceptions and behavior nuances (like newline stripping in
read_one_line). This makes the API more predictable and easier to use correctly.
97-153: LGTM! Clear distinction between substring and regex matching.The docstrings effectively clarify that
read_line_with_matching_patternuses simple substring matching (not regex), which is important for users to understand. Theread_all_linesdocumentation also clearly explains its error-swallowing behavior.
156-217: LGTM! Write function documentation is comprehensive.The enhanced docstrings clearly document overwrite behavior, automatic newline handling in
write_one_line, and the distinction betweenwrite_fileandwrite_file_or_failexception handling strategies.
220-259: LGTM! Append function documentation is clear and complete.The enhanced docstrings document file creation behavior and newline handling. The multiple usage examples in
append_one_lineeffectively demonstrate typical incremental logging patterns.
262-288: LGTM! Regex behavior is now clearly documented.The enhanced docstring explicitly clarifies that
is_pattern_in_fileuses regex matching with MULTILINE mode, distinguishing it from the substring-basedread_line_with_matching_pattern. This is an important clarification for API users.
291-321: LGTM! Deprecation warning properly implemented.The
are_files_equaldocstring enhancement clearly explains the hash-based comparison approach. The module-level deprecation warning is correctly implemented using the project's deprecation utility, with an appropriate timeline message. The import positioning at module end (with pylint disable) is the correct pattern for ensuring the warning is emitted when the module is imported.
| "unit": 950, | ||
| "jobs": 11, | ||
| "functional-parallel": 353, | ||
| "functional-parallel": 366, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Count actual unit tests in selftests/unit/
echo "=== Counting unit tests ==="
python3 -m pytest --collect-only selftests/unit/ 2>/dev/null | grep -c "<Function\|<Method" || echo "Using fallback method..."
echo ""
echo "=== Counting genio unit tests specifically ==="
grep -E "^\s+def test_" selftests/unit/utils/genio.py | wc -l
echo ""
echo "=== Counting functional-parallel tests ==="
python3 -m pytest --collect-only selftests/functional/ 2>/dev/null | grep -c "<Function\|<Method" || echo "Using fallback method..."
echo ""
echo "=== Counting genio functional tests specifically ==="
if [ -f "selftests/functional/utils/genio.py" ]; then
grep -E "^\s+def test_" selftests/functional/utils/genio.py | wc -l
else
echo "No functional genio tests found"
fiRepository: avocado-framework/avocado
Length of output: 302
🏁 Script executed:
#!/bin/bash
# Read the check.py file to understand TEST_SIZE
echo "=== check.py context around line 29-31 ==="
head -40 selftests/check.py | tail -20
echo ""
echo "=== Look for TEST_SIZE documentation ==="
grep -B5 -A5 "TEST_SIZE" selftests/check.py | head -30
echo ""
echo "=== Count all unit test files ==="
find selftests/unit -name "test_*.py" -o -name "*_test.py" | wc -l
echo ""
echo "=== List all unit test files ==="
find selftests/unit -name "test_*.py" -o -name "*_test.py" | sort
echo ""
echo "=== Count test functions across all unit tests ==="
find selftests/unit -name "*.py" -exec grep -l "def test_" {} \; | wc -lRepository: avocado-framework/avocado
Length of output: 1994
🏁 Script executed:
#!/bin/bash
# Count all test functions in selftests/unit directory
echo "=== Total test functions in selftests/unit/ ==="
find selftests/unit -name "*.py" -type f | xargs grep -h "^\s*def test_" | wc -l
echo ""
# Count test methods in classes
echo "=== Total test methods in classes (indented with def test_) ==="
find selftests/unit -name "*.py" -type f | xargs grep -h "^\s\s*def test_" | wc -l
echo ""
# Combined count
echo "=== Combined count of all test functions/methods ==="
find selftests/unit -name "*.py" -type f | xargs grep -h "def test_" | wc -l
echo ""
# Show git status to see what was changed
echo "=== Check git log for recent changes ==="
git log --oneline -10 selftests/check.py 2>/dev/null || echo "Git log not available"
echo ""
# Check if there's a way to see what changed in TEST_SIZE
echo "=== Show recent changes to TEST_SIZE values ==="
git diff HEAD~1 selftests/check.py 2>/dev/null | grep -A2 -B2 "unit" | head -20 || echo "Git diff not available"Repository: avocado-framework/avocado
Length of output: 390
Update TEST_SIZE to match actual test count.
The unit TEST_SIZE value of 950 does not match the actual test count of 959 in selftests/unit/. Additionally, while the PR description mentions 21 unit tests added and genio.py contains 21 test functions, the TEST_SIZE only increased by 16 (934→950), a 5-test discrepancy. Update TEST_SIZE to accurately reflect the current test count.
🤖 Prompt for AI Agents
In @selftests/check.py around lines 29-31, The TEST_SIZE constant is out of
sync: update the "unit" entry in TEST_SIZE (referenced as TEST_SIZE or the dict
containing "unit") to match the real count of tests (959) so it reflects the
actual number of tests in selftests/unit/ (also verify genio.py additions were
fully counted); change the value from 950 to 959 and run the unit test counter
or ls on selftests/unit to confirm no other discrepancies.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6262 +/- ##
==========================================
+ Coverage 73.58% 73.70% +0.11%
==========================================
Files 206 206
Lines 22497 22499 +2
==========================================
+ Hits 16555 16583 +28
+ Misses 5942 5916 -26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Improvement of Functional/Unit tests, includes deprecation and new RST docstrings.
Summary by CodeRabbit
Deprecation
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.