diff --git a/.changes/next-release/enhancement-ecs-89147.json b/.changes/next-release/enhancement-ecs-89147.json new file mode 100644 index 000000000000..295297713bfd --- /dev/null +++ b/.changes/next-release/enhancement-ecs-89147.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "``ecs``", + "description": "Update ``ecs create-express-gateway-service``, ``ecs update-express-gateway-service``, and ``ecs delete-express-gateway-service`` commands to not output API response when run with the ``--monitor-resources`` flag. Also fixes scrolling bounds calculations when line wrapping is present." +} diff --git a/awscli/customizations/ecs/monitorexpressgatewayservice.py b/awscli/customizations/ecs/monitorexpressgatewayservice.py index 16f5152f372e..3004fe69ecc9 100644 --- a/awscli/customizations/ecs/monitorexpressgatewayservice.py +++ b/awscli/customizations/ecs/monitorexpressgatewayservice.py @@ -182,7 +182,6 @@ class ECSExpressGatewayServiceWatcher: service_arn (str): ARN of the service to monitor mode (str): Monitoring mode - 'RESOURCE' or 'DEPLOYMENT' timeout_minutes (int): Maximum monitoring time in minutes (default: 30) - exit_hook (callable): Optional callback function on exit """ def __init__( @@ -191,7 +190,6 @@ def __init__( service_arn, mode, timeout_minutes=30, - exit_hook=None, display=None, use_color=True, ): @@ -199,7 +197,6 @@ def __init__( self.service_arn = service_arn self.mode = mode self.timeout_minutes = timeout_minutes - self.exit_hook = exit_hook or self._default_exit_hook self.last_described_gateway_service_response = None self.last_execution_time = 0 self.cached_monitor_result = None @@ -207,8 +204,10 @@ def __init__( self.use_color = use_color self.display = display or Display() - def _default_exit_hook(self, x): - return x + @staticmethod + def is_monitoring_available(): + """Check if monitoring is available (requires TTY).""" + return sys.stdout.isatty() def exec(self): """Start monitoring the express gateway service with progress display.""" @@ -226,7 +225,13 @@ async def _execute_with_progress_async( """Execute monitoring loop with animated progress display.""" spinner_chars = "⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏" spinner_index = 0 - current_output = "Waiting for initial data" + + # Initialize with basic service resource + service_resource = ManagedResource("Service", self.service_arn) + initial_output = service_resource.get_status_string( + spinner_char="{SPINNER}", use_color=self.use_color + ) + current_output = initial_output async def update_data(): nonlocal current_output diff --git a/awscli/customizations/ecs/monitormutatinggatewayservice.py b/awscli/customizations/ecs/monitormutatinggatewayservice.py index e0c7d019df93..682eb53e6678 100644 --- a/awscli/customizations/ecs/monitormutatinggatewayservice.py +++ b/awscli/customizations/ecs/monitormutatinggatewayservice.py @@ -171,6 +171,14 @@ def after_call(self, parsed, context, http_response, **kwargs): ).get('serviceArn'): return + # Check monitoring availability + if not self._watcher_class.is_monitoring_available(): + uni_print( + "Monitoring is not available (requires TTY). Skipping monitoring.\n", + out_file=sys.stderr, + ) + return + if not self.session or not self.parsed_globals: uni_print( "Unable to create ECS client. Skipping monitoring.", @@ -188,22 +196,15 @@ def after_call(self, parsed, context, http_response, **kwargs): # Get service ARN from response service_arn = parsed.get('service', {}).get('serviceArn') - # Define exit hook to replace parsed response - def exit_hook(new_response): - if new_response: - parsed.clear() - parsed.update(new_response) + # Clear output when monitoring is invoked + parsed.clear() try: - # Determine if color should be used - use_color = self._should_use_color(self.parsed_globals) - self._watcher_class( ecs_client, service_arn, self.effective_resource_view, - exit_hook=exit_hook, - use_color=use_color, + use_color=self._should_use_color(self.parsed_globals), ).exec() except Exception as e: uni_print( diff --git a/awscli/customizations/ecs/prompt_toolkit_display.py b/awscli/customizations/ecs/prompt_toolkit_display.py index 729412fa514a..e7ad5cd77026 100644 --- a/awscli/customizations/ecs/prompt_toolkit_display.py +++ b/awscli/customizations/ecs/prompt_toolkit_display.py @@ -1,4 +1,5 @@ import asyncio +import re from prompt_toolkit.application import Application from prompt_toolkit.formatted_text import ANSI @@ -8,6 +9,12 @@ from prompt_toolkit.widgets import Frame +def _get_visual_line_length(line): + """Get the visual length of a line, excluding ANSI escape codes.""" + ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') + return len(ansi_escape.sub('', line)) + + class Display: def __init__(self): self.control = FormattedTextControl(text="") @@ -21,6 +28,7 @@ def __init__(self): text="up/down to scroll, q to quit" ) self.content_lines = 0 + self.raw_text = "" kb = KeyBindings() @kb.add('q') @@ -35,12 +43,14 @@ def scroll_up(event): @kb.add('down') def scroll_down(event): - window_height = ( - event.app.output.get_size().rows - 3 - ) # Frame top, frame bottom, status bar - if self.window.vertical_scroll < max( - 0, self.content_lines - window_height - ): + # Frame top, frame bottom, status bar + window_height = event.app.output.get_size().rows - 3 + # Account for frame borders and scrollbar + window_width = event.app.output.get_size().columns - 4 + + total_display_lines = self._calculate_display_lines(window_width) + max_scroll = max(0, total_display_lines - window_height) + if self.window.vertical_scroll < max_scroll: self.window.vertical_scroll += 1 self.app = Application( @@ -58,12 +68,49 @@ def scroll_down(event): def display(self, text, status_text=""): """Update display with ANSI colored text.""" + self.raw_text = text self.control.text = ANSI(text) self.content_lines = len(text.split('\n')) + + self._validate_scroll_position() + if status_text: self.status_control.text = status_text self.app.invalidate() + def _calculate_display_lines(self, window_width): + """Calculate total display lines accounting for line wrapping.""" + total_display_lines = 0 + for line in self.raw_text.split('\n'): + visual_length = _get_visual_line_length(line) + if visual_length == 0: + total_display_lines += 1 + else: + # Calculate how many display lines this text line will occupy + # when wrapped: ceil(visual_length / window_width) + # Using integer math: (visual_length + window_width - 1) // window_width + total_display_lines += max( + 1, (visual_length + window_width - 1) // window_width + ) + return total_display_lines + + def _validate_scroll_position(self): + """Ensure scroll position is valid for current content.""" + if not getattr(self.app, 'output', None): + return + + try: + window_height = self.app.output.get_size().rows - 3 + window_width = self.app.output.get_size().columns - 4 + + total_display_lines = self._calculate_display_lines(window_width) + max_scroll = max(0, total_display_lines - window_height) + if self.window.vertical_scroll > max_scroll: + self.window.vertical_scroll = max_scroll + except (AttributeError, OSError): + # If we can't determine terminal size, leave scroll position unchanged + pass + async def run(self): """Run the display app.""" await self.app.run_async() diff --git a/tests/functional/ecs/test_monitormutatinggatewayservice.py b/tests/functional/ecs/test_monitormutatinggatewayservice.py index 61b39c8f9c03..b4c34281e872 100644 --- a/tests/functional/ecs/test_monitormutatinggatewayservice.py +++ b/tests/functional/ecs/test_monitormutatinggatewayservice.py @@ -32,8 +32,12 @@ def test_add_to_parser(self): class TestMonitorMutatingGatewayService: def setup_method(self): + self.mock_watcher_class = Mock() + self.mock_watcher_class.is_monitoring_available.return_value = True self.handler = MonitorMutatingGatewayService( - 'create-gateway-service', 'DEPLOYMENT' + 'create-gateway-service', + 'DEPLOYMENT', + watcher_class=self.mock_watcher_class, ) def test_init(self): diff --git a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py index 636952311179..37d9fa0a63e1 100644 --- a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py +++ b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py @@ -10,6 +10,8 @@ import pytest from botocore.exceptions import ClientError +from prompt_toolkit.application import create_app_session +from prompt_toolkit.output import DummyOutput from awscli.customizations.ecs.exceptions import MonitoringError from awscli.customizations.ecs.monitorexpressgatewayservice import ( @@ -105,12 +107,34 @@ def test_interactive_mode_requires_tty(self, mock_isatty, capsys): class TestECSExpressGatewayServiceWatcher: """Test the watcher class through public interface""" + @patch('sys.stdout.isatty') + def test_is_monitoring_available_with_tty(self, mock_isatty): + """Test is_monitoring_available returns True when TTY is available""" + mock_isatty.return_value = True + assert ( + ECSExpressGatewayServiceWatcher.is_monitoring_available() is True + ) + + @patch('sys.stdout.isatty') + def test_is_monitoring_available_without_tty(self, mock_isatty): + """Test is_monitoring_available returns False when TTY is not available""" + mock_isatty.return_value = False + assert ( + ECSExpressGatewayServiceWatcher.is_monitoring_available() is False + ) + def setup_method(self): + self.app_session = create_app_session(output=DummyOutput()) + self.app_session.__enter__() self.mock_client = Mock() self.service_arn = ( "arn:aws:ecs:us-west-2:123456789012:service/my-cluster/my-service" ) + def teardown_method(self): + if hasattr(self, 'app_session'): + self.app_session.__exit__(None, None, None) + def _create_watcher_with_mocks(self, resource_view="RESOURCE", timeout=1): """Helper to create watcher with mocked display""" mock_display = Mock() @@ -724,6 +748,14 @@ def test_monitoring_error_creation(self): class TestColorSupport: """Test color support functionality""" + def setup_method(self): + self.app_session = create_app_session(output=DummyOutput()) + self.app_session.__enter__() + + def teardown_method(self): + if hasattr(self, 'app_session'): + self.app_session.__exit__(None, None, None) + def test_should_use_color_on(self): """Test _should_use_color returns True when color is 'on'""" command = ECSMonitorExpressGatewayService(Mock()) diff --git a/tests/unit/customizations/ecs/test_monitormutatinggatewayservice.py b/tests/unit/customizations/ecs/test_monitormutatinggatewayservice.py index e521a3d6e157..37afe0a78324 100644 --- a/tests/unit/customizations/ecs/test_monitormutatinggatewayservice.py +++ b/tests/unit/customizations/ecs/test_monitormutatinggatewayservice.py @@ -72,8 +72,12 @@ class TestMonitorMutatingGatewayService: """Test the event handler for monitoring gateway service mutations.""" def setup_method(self): + self.mock_watcher_class = Mock() + self.mock_watcher_class.is_monitoring_available.return_value = True self.handler = MonitorMutatingGatewayService( - 'create-gateway-service', 'DEPLOYMENT' + 'create-gateway-service', + 'DEPLOYMENT', + watcher_class=self.mock_watcher_class, ) def test_init(self): @@ -242,6 +246,7 @@ def test_after_call_successful_monitoring(self): 'https://ecs.us-west-2.amazonaws.com' ) mock_parsed_globals.verify_ssl = True + mock_parsed_globals.color = 'off' handler.session = mock_session handler.parsed_globals = mock_parsed_globals @@ -272,18 +277,17 @@ def test_after_call_successful_monitoring(self): mock_client, service_arn, 'DEPLOYMENT', - exit_hook=ANY, use_color=False, ) mock_watcher.exec.assert_called_once() + # Verify parsed response was cleared + assert parsed == {} - def test_after_call_exception_handling(self, capsys): - """Test exception handling in after_call method.""" + def test_after_call_monitoring_not_available(self, capsys): + """Test that monitoring is skipped when not available (no TTY).""" # Setup handler state mock_watcher_class = Mock() - mock_watcher = Mock() - mock_watcher.exec.side_effect = Exception("Test exception") - mock_watcher_class.return_value = mock_watcher + mock_watcher_class.is_monitoring_available.return_value = False handler = MonitorMutatingGatewayService( 'create-gateway-service', @@ -299,6 +303,7 @@ def test_after_call_exception_handling(self, capsys): 'https://ecs.us-west-2.amazonaws.com' ) mock_parsed_globals.verify_ssl = True + mock_parsed_globals.color = 'off' handler.session = mock_session handler.parsed_globals = mock_parsed_globals @@ -309,21 +314,39 @@ def test_after_call_exception_handling(self, capsys): # Setup call parameters service_arn = 'arn:aws:ecs:us-west-2:123456789012:service/test-service' parsed = {'service': {'serviceArn': service_arn}} + original_parsed = dict(parsed) context = Mock() http_response = Mock() http_response.status_code = 200 - # Execute - should not raise exception + # Execute handler.after_call(parsed, context, http_response) + # Verify parsed response was not cleared + assert parsed == original_parsed + + # Verify warning message was printed captured = capsys.readouterr() - assert "Encountered an error, terminating monitoring" in captured.err - assert "Test exception" in captured.err + assert ( + "Monitoring is not available (requires TTY). Skipping monitoring.\n" + in captured.err + ) - def test_exit_hook_functionality(self): - """Test that exit hook properly updates parsed response.""" + def test_after_call_exception_handling(self, capsys): + """Test exception handling in after_call method.""" # Setup handler state - self.handler.effective_resource_view = 'DEPLOYMENT' + mock_watcher_class = Mock() + mock_watcher = Mock() + mock_watcher.exec.side_effect = Exception("Test exception") + mock_watcher_class.return_value = mock_watcher + + handler = MonitorMutatingGatewayService( + 'create-gateway-service', + 'DEPLOYMENT', + watcher_class=mock_watcher_class, + ) + handler.effective_resource_view = 'DEPLOYMENT' + mock_session = Mock() mock_parsed_globals = Mock() mock_parsed_globals.region = 'us-west-2' @@ -331,35 +354,27 @@ def test_exit_hook_functionality(self): 'https://ecs.us-west-2.amazonaws.com' ) mock_parsed_globals.verify_ssl = True - self.handler.session = mock_session - self.handler.parsed_globals = mock_parsed_globals + mock_parsed_globals.color = 'off' + handler.session = mock_session + handler.parsed_globals = mock_parsed_globals # Setup mocks mock_client = Mock() mock_session.create_client.return_value = mock_client - # Test exit hook functionality by directly calling it + # Setup call parameters service_arn = 'arn:aws:ecs:us-west-2:123456789012:service/test-service' parsed = {'service': {'serviceArn': service_arn}} + context = Mock() + http_response = Mock() + http_response.status_code = 200 - # Create a simple exit hook function - def test_exit_hook(new_response): - if new_response: - parsed.clear() - parsed.update(new_response) - - # Test with new response - new_response = { - 'service': {'serviceArn': service_arn, 'status': 'ACTIVE'} - } - test_exit_hook(new_response) - - assert parsed == new_response + # Execute - should not raise exception + handler.after_call(parsed, context, http_response) - # Test with None response (should not update) - original_parsed = dict(parsed) - test_exit_hook(None) - assert parsed == original_parsed + captured = capsys.readouterr() + assert "Encountered an error, terminating monitoring" in captured.err + assert "Test exception" in captured.err def test_events(self): """Test that correct events are returned for CLI integration.""" diff --git a/tests/unit/customizations/ecs/test_prompt_toolkit_display.py b/tests/unit/customizations/ecs/test_prompt_toolkit_display.py index c4b1b29823e9..85517dafebb7 100644 --- a/tests/unit/customizations/ecs/test_prompt_toolkit_display.py +++ b/tests/unit/customizations/ecs/test_prompt_toolkit_display.py @@ -1,7 +1,10 @@ import asyncio +import sys from unittest.mock import Mock, patch import pytest +from prompt_toolkit.application import create_app_session +from prompt_toolkit.output import DummyOutput from awscli.customizations.ecs.prompt_toolkit_display import Display @@ -9,7 +12,8 @@ class TestPromptToolkitDisplay: @pytest.fixture def display(self): - return Display() + with create_app_session(output=DummyOutput()): + return Display() def test_init(self, display): """Test Display initialization.""" @@ -18,6 +22,7 @@ def test_init(self, display): assert display.status_control is not None assert display.app is not None assert display.content_lines == 0 + assert display.raw_text == "" def test_display_updates_content(self, display): """Test display method updates content and line count.""" @@ -27,6 +32,7 @@ def test_display_updates_content(self, display): display.display(test_text, test_status) assert display.content_lines == 3 + assert display.raw_text == test_text assert display.status_control.text == test_status def test_display_without_status(self, display): @@ -36,6 +42,7 @@ def test_display_without_status(self, display): display.display(test_text) assert display.content_lines == 1 + assert display.raw_text == test_text @patch('awscli.customizations.ecs.prompt_toolkit_display.ANSI') def test_display_uses_ansi_formatting(self, mock_ansi, display): @@ -45,6 +52,7 @@ def test_display_uses_ansi_formatting(self, mock_ansi, display): display.display(test_text) mock_ansi.assert_called_once_with(test_text) + assert display.raw_text == test_text def test_scroll_bounds_calculation(self, display): """Test that content_lines is calculated correctly for scroll bounds.""" @@ -55,7 +63,7 @@ def test_scroll_bounds_calculation(self, display): assert display.content_lines == 5 @patch('awscli.customizations.ecs.prompt_toolkit_display.Application') - def test_run_calls_app_run_async(self, mock_app_class): + def test_run_calls_app_run_async(self, mock_app_class, display): """Test run method calls app.run_async().""" mock_app = Mock() mock_app_class.return_value = mock_app @@ -66,7 +74,8 @@ async def mock_run_async(): mock_app.run_async = mock_run_async - display = Display() + # Replace the display's app with our mock + display.app = mock_app # Run the async method asyncio.run(display.run()) @@ -93,3 +102,64 @@ def test_long_line_wrapping(self, display): assert display.content_lines == 3 # Verify the content is set correctly assert display.control.text is not None + + def test_display_handles_ansi_content_without_errors(self, display): + """Test that display handles ANSI-colored content without errors.""" + # Content with various ANSI codes + ansi_text = "\x1b[32mGreen\x1b[0m\n\x1b[31mRed\x1b[32mGreen\x1b[0m\n\x1b[1m\x1b[41mBold on Red\x1b[0m" + + # Should not raise any exceptions + display.display(ansi_text) + + # Content should be stored and processed + assert display.raw_text == ansi_text + assert display.content_lines == 3 + + def test_scroll_validation_with_ansi_content(self, display): + """Test scroll position validation works with ANSI-colored content.""" + # Mock the app output for terminal size + mock_output = Mock() + mock_output.get_size.return_value = Mock(rows=20, columns=80) + display.app.output = mock_output + + # Set content with ANSI codes + ansi_text = "\x1b[32mShort line\x1b[0m\n\x1b[31mAnother line\x1b[0m" + + # Should not raise an exception and content should be stored + display.display(ansi_text) + assert display.raw_text == ansi_text + + def test_scroll_validation_handles_missing_output(self, display): + """Test scroll validation gracefully handles missing app output.""" + # Set up display without proper app output + display.app.output = None + + # Should not raise an exception + display.display("Test content") + assert display.raw_text == "Test content" + + def test_scroll_validation_handles_exceptions(self, display): + """Test scroll validation clamps scroll position on terminal size exceptions.""" + mock_output = Mock() + mock_output.get_size.side_effect = OSError("Terminal unavailable") + display.app.output = mock_output + + display.window.vertical_scroll = 5 + + display.display("Test content") + assert display.window.vertical_scroll == 5 + + def test_scroll_validation_with_long_lines(self, display): + """Test scroll validation with lines that wrap.""" + # Mock the app output for terminal size + mock_output = Mock() + mock_output.get_size.return_value = Mock(rows=20, columns=80) + display.app.output = mock_output + + # Create content with very long lines that will wrap + long_line = "A" * 100 # Longer than terminal width + content = f"Short\n{long_line}\nShort again" + + # Should handle wrapping without errors + display.display(content) + assert display.raw_text == content