Repository navigation
refactor: improve error handling and code structure in various modules - #4656
Conversation
- Enhanced error logging in `api.py` to use `LOG.exception` for better traceability. - Simplified the `get_line_style` function in `ansi_output.py` by removing redundant regex patterns. - Refactored the `run_scenarios_parallel` function in `worker.py` to improve readability and maintainability by extracting helper functions for prerun steps and future result processing. - Consolidated variable writing logic in `ansible.py` to a dedicated method for better organization. - Updated schema validation logic in `schema_v3.py` to improve clarity and maintainability. These changes aim to enhance code quality and maintainability across the project.
…r handling - Modified assertions in `test_click_command_ex_with_immediate_exit_failure` to check for exceptions with `exc_info=False` in non-debug mode. - Updated `test_click_command_ex_with_immediate_exit_failure_debug_mode` to assert exceptions with `exc_info=True` in debug mode. - Adjusted `test_click_command_ex_failure_no_context` to verify non-debug behavior with exceptions instead of errors. These changes enhance the accuracy of error handling tests in the click command functionality.
- Introduced `_group_command_parts` method to logically group command parts before wrapping. - Added `_wrap_lines` method to handle line wrapping and indentation, improving readability. - Simplified the logic for constructing command lines, ensuring better handling of flags and values. These changes improve the clarity and maintainability of command output formatting.
- Added tests to ensure that drivers and verifiers log exceptions when they fail to initialize. - Implemented `_BrokenDriver` and `_BrokenVerifier` classes to simulate failures and verify logging behavior. - Updated assertions to confirm that the appropriate exceptions are logged and that the system behaves as expected when encountering plugin failures. These changes improve the robustness of error handling in the testing framework.
| # Match ANSI escape sequences at the start of the line | ||
| # Patterns: \x1b[...m or \033[...m | ||
| match = re.match(r"^(\x1b\[[0-9;]*m|\033\[[0-9;]*m)", line) | ||
| match = re.match(r"^(\x1b\[[0-9;]*m)", line) |
There was a problem hiding this comment.
Potential behavior change: ANSI escape sequence pattern
The regex pattern was simplified from:
r"^(\x1b[[0-9;]*m|\033[[0-9;]*m)"
to:
r"^(\x1b[[0-9;]*m)"
While \x1b and \033 represent the same ESC character (ASCII 27), they are different string literals in Python. The regex matches against the literal string bytes, not the character value.
Concern: If any terminal emulators, log parsers, or external libraries emit ANSI sequences using octal notation (\033[...m), they will no longer be matched, potentially causing incorrect text styling.
Questions:
- Was the \033 alternative originally added to handle specific input sources?
- Have we verified that all ANSI input in our codebase uses hex notation exclusively?
Suggested verification:
Check test fixtures for octal notation
grep -r "\\033\[" tests/
Check if external libraries emit octal form
python3 -c "import sys; print(repr(sys.stdout.write('\033[32mtest\033[0m')))"
If verification confirms octal notation is never encountered, this simplification is fine (consider adding a comment explaining why). Otherwise, please restore the original pattern to maintain compatibility:
match = re.match(r"^(\x1b[[0-9;]*m|\033[[0-9;]*m)", line)
(Same applies to line 18 where the check was changed from if escape_seq in ["\x1b[0m", "\033[0m"] to if escape_seq == "\x1b[0m")
api.pyto useLOG.exceptionfor better traceability.get_line_stylefunction inansi_output.pyby removing redundant regex patterns.run_scenarios_parallelfunction inworker.pyto improve readability and maintainability by extracting helper functions for prerun steps and future result processing.ansible.pyto a dedicated method for better organization.schema_v3.pyto improve clarity and maintainability.These changes aim to enhance code quality and maintainability across the project.