Add experimental --command-borders flag for visual command output separation#4496
Merged
cidrblock merged 6 commits intoansible:mainfrom Aug 4, 2025
Merged
Add experimental --command-borders flag for visual command output separation#4496cidrblock merged 6 commits intoansible:mainfrom
cidrblock merged 6 commits intoansible:mainfrom
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)
Contributor
There was a problem hiding this comment.
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
CommandBordersandBorderedStreamclasses 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
Contributor
|
Looks awesome @cidrblock! |
alisonlhart
approved these changes
Aug 4, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an experimental
--command-bordersCLI 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,BorderedStreamclasses 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-bordersflag 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