-
Notifications
You must be signed in to change notification settings - Fork 669
Add experimental --command-borders flag for visual command output separation #4496
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
Conversation
e3e2401
to
ccc1969
Compare
- Remove useless if-else condition in get_line_style() * Use consistent \x1b prefix for all ANSI codes - Replace magic number 5 with named constant min_reset_color_length - Use lowercase variable name following naming conventions - Remove accidentally committed output.txt file All pre-commit hooks now pass: ✅ ruff (no more RUF034, PLR2004, N806 errors) ✅ cspell (no more unknown words from output.txt) ✅ All other formatting and quality checks
Regenerate comprehensive_output.txt fixture to show correct └─ (corner + horizontal) instead of │─ (vertical + horizontal) in footer lines. Changes: - All 'Return code: 0' footers now use proper └─ corner character - Maintains consistency with restored BOX_BOTTOM_LEFT constant - Integration test passes with updated visual appearance Before: │─ Return code: 0 (debug character) After: └─ Return code: 0 (proper corner character)
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.
Pull Request Overview
Adds an experimental --command-borders
CLI flag that visually separates external command output from Molecule's own output using Unicode box-drawing characters and ANSI escape codes.
- Introduces new
CommandBorders
andBorderedStream
classes for intelligent command-line formatting - Integrates the flag through all Molecule commands and propagates to external command execution
- Provides comprehensive test coverage including end-to-end validation with fixture comparison
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tox.ini | Adds environment variable for fixture update capability |
tests/unit/test_ansi_output_borders.py | Comprehensive unit tests for border functionality (33 test cases) |
tests/unit/test_ansi_output.py | Extended tests for new ANSI output methods |
tests/unit/dependency/*.py | Updates test assertions for new command_borders parameter |
tests/integration/test_command.py | Replaces multiple tests with comprehensive output validation |
tests/fixtures/integration/test_command/comprehensive_output.txt | Golden file for end-to-end output validation |
src/molecule/verifier/testinfra.py | Propagates command_borders to verifier execution |
src/molecule/types.py | Adds command_borders to CommandArgs type definition |
src/molecule/provisioner/ansible_playbook.py | Propagates command_borders and improves error formatting |
src/molecule/dependency/base.py | Propagates command_borders to dependency execution |
src/molecule/constants.py | Adds Unicode box-drawing characters and border width constant |
src/molecule/config.py | Adds command_borders property to configuration |
src/molecule/command/*.py | Propagates command_borders flag through all commands |
src/molecule/click_cfg.py | Defines the new CLI flag |
src/molecule/app.py | Integrates CommandBorders into command execution |
src/molecule/ansi_output.py | Core implementation of border rendering and stream capture |
.config/pydoclint-baseline.txt | Updates documentation baseline for test changes |
Comments suppressed due to low confidence (5)
src/molecule/ansi_output.py:169
- [nitpick] The variable name 'i' is ambiguous and could be more descriptive. Consider renaming it to 'line_index' or 'index' to clarify its purpose in the wrapped lines iteration.
text: Text containing Rich markup tags.
src/molecule/ansi_output.py:601
- [nitpick] The variable name 'i' is ambiguous and could be more descriptive. Consider renaming it to 'part_index' or 'index' to clarify its purpose in the command parts iteration.
lines, i = [], 0
src/molecule/ansi_output.py:633
- [nitpick] The variable name 'i' is ambiguous and could be more descriptive. Consider renaming it to 'line_index' or 'index' to clarify its purpose in the line enumeration.
for i, line in enumerate(lines):
src/molecule/ansi_output.py:635
- [nitpick] The variable name 'k' is ambiguous and could be more descriptive. Consider renaming it to 'break_point' or 'space_index' to clarify its purpose in finding the line break position.
k = line.rfind(" ", 0, max_width)
src/molecule/ansi_output.py:488
- [nitpick] The variable name 'i' is ambiguous and could be more descriptive. Consider renaming it to 'line_index' or 'index' to clarify its purpose in the wrapped lines enumeration.
for i, wrapped_line in enumerate(wrapped_lines):
Add ANSIBLE_DISABLE_WARNINGS=True to test_full_output environment to prevent Ansible deprecation warnings from varying between environments and versions. Benefits: - Prevents test failures due to environment-specific Ansible warnings - Ensures consistent output across different CI environments and local setups - Reduces maintenance overhead for fixture updates due to warning changes - Complements existing WARNING line filtering in sanitization function Defense-in-depth approach: 1. Primary: ANSIBLE_DISABLE_WARNINGS=True prevents warnings at source 2. Secondary: Sanitization function filters any remaining WARNING lines This makes the comprehensive test more stable and focused on validating the command borders feature rather than tracking Ansible deprecation warnings.
…ression Add ANSIBLE_DEPRECATION_WARNINGS=false alongside ANSIBLE_DISABLE_WARNINGS=True to provide comprehensive Ansible warning suppression in test_full_output. This dual approach ensures maximum protection against warnings that could vary between different Ansible versions and environments: - ANSIBLE_DISABLE_WARNINGS=True: Disables general warnings - ANSIBLE_DEPRECATION_WARNINGS=false: Specifically targets deprecation warnings Benefits: - Eliminates deprecation warning variations between Ansible versions - Ensures consistent test output across all environments - Prevents fixture updates due to deprecation warning changes - Complements existing WARNING line filtering for complete coverage This creates a robust triple-layer defense: 1. ANSIBLE_DISABLE_WARNINGS=True (general warnings) 2. ANSIBLE_DEPRECATION_WARNINGS=false (deprecation-specific) 3. Sanitization WARNING line filtering (fallback protection)
Replace ANSIBLE_DISABLE_WARNINGS (not a real env var) with the correct Ansible warning suppression environment variables: - ANSIBLE_DEPRECATION_WARNINGS=false (deprecation warnings) - ANSIBLE_COMMAND_WARNINGS=false (command-specific warnings) - ANSIBLE_ACTION_WARNINGS=false (action/module warnings) This provides comprehensive and accurate warning suppression using Ansible's documented environment variables, ensuring consistent test output across different environments and Ansible versions. Benefits: - Uses officially supported Ansible environment variables - Provides targeted warning suppression by category - Maintains test stability and consistency - Works with existing WARNING line filtering fallback
Looks awesome @cidrblock! |
Summary
Adds an experimental
--command-borders
CLI flag that visually separates external command output from Molecule's own output using Unicode box-drawing characters and ANSI escape codes.Motivation
When running complex scenarios, distinguishing between Molecule's orchestration output and the actual command output (ansible-playbook, dependency installs, etc.) can be challenging. This enhancement provides clear visual boundaries around external command execution without affecting Molecule's core logic or behavior.
Implementation
Core Changes
src/molecule/ansi_output.py
: NewCommandBorders
,BorderedStream
classes with intelligent command-line formattingsrc/molecule/app.py
: Integration point for bordered command executionsrc/molecule/constants.py
: Unicode box-drawing character constantsCLI Integration
src/molecule/click_cfg.py
: New--command-borders
flag definitionsrc/molecule/types.py
: Type definitions for the new flagsrc/molecule/config.py
: Configuration storageCommand Propagation
src/molecule/command/*.py
: Flag propagation through all commandsTesting
tests/unit/test_ansi_output_borders.py
: Comprehensive unit tests (33 test cases)tests/unit/test_ansi_output.py
: Extended coverage for new functionalitytests/integration/test_command.py
: End-to-end validation with fixture comparisonCharacteristics
Visual Example
Note: a better balance between stdout and stderr will follow, it was too much given the scope of this PR #4497