Skip to content

Commit

Permalink
Fix pr #6284: Fix issue #6037: [Bug]: [Resolver] crashes on main
Browse files Browse the repository at this point in the history
  • Loading branch information
openhands-agent committed Jan 15, 2025
1 parent 90611a0 commit cbe1150
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 53 deletions.
2 changes: 1 addition & 1 deletion openhands/runtime/utils/log_streamer.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def _stream_logs(self):
self.log('error', f'Error streaming docker logs to stdout: {e}')

def __del__(self):
if self.stdout_thread and self.stdout_thread.is_alive():
if hasattr(self, 'stdout_thread') and self.stdout_thread and self.stdout_thread.is_alive():
self.close(timeout=5)

def close(self, timeout: float = 5.0):
Expand Down
119 changes: 67 additions & 52 deletions tests/unit/test_log_streamer.py
Original file line number Diff line number Diff line change
@@ -1,57 +1,72 @@
import unittest
import pytest
from unittest.mock import Mock

from openhands.runtime.utils.log_streamer import LogStreamer


class TestLogStreamer(unittest.TestCase):
def setUp(self):
self.mock_container = Mock()
self.mock_log_fn = Mock()

def test_init_failure_handling(self):
"""Test that LogStreamer handles initialization failures gracefully."""
self.mock_container.logs.side_effect = Exception('Test error')

streamer = LogStreamer(self.mock_container, self.mock_log_fn)
self.assertIsNone(streamer.stdout_thread)
self.assertIsNone(streamer.log_generator)
self.mock_log_fn.assert_called_with(
'error', 'Failed to initialize log streaming: Test error'
)

def test_stream_logs_without_generator(self):
"""Test that _stream_logs handles missing log generator gracefully."""
streamer = LogStreamer(self.mock_container, self.mock_log_fn)
streamer.log_generator = None
streamer._stream_logs()
self.mock_log_fn.assert_called_with('error', 'Log generator not initialized')

def test_cleanup_without_thread(self):
"""Test that cleanup works even if stdout_thread is not initialized."""
streamer = LogStreamer(self.mock_container, self.mock_log_fn)
streamer.stdout_thread = None
streamer.close() # Should not raise any exceptions

def test_normal_operation(self):
"""Test normal operation of LogStreamer."""
mock_logs = [b'test log 1\n', b'test log 2\n']
self.mock_container.logs.return_value = mock_logs

streamer = LogStreamer(self.mock_container, self.mock_log_fn)
self.assertIsNotNone(streamer.stdout_thread)
self.assertIsNotNone(streamer.log_generator)

# Let the thread process the logs
streamer.close()

# Verify logs were processed
expected_calls = [
('debug', '[inside container] test log 1'),
('debug', '[inside container] test log 2'),
]
actual_calls = [
(args[0], args[1]) for args, _ in self.mock_log_fn.call_args_list
]
for expected in expected_calls:
self.assertIn(expected, actual_calls)
@pytest.fixture
def mock_container():
return Mock()


@pytest.fixture
def mock_log_fn():
return Mock()


def test_init_failure_handling(mock_container, mock_log_fn):
"""Test that LogStreamer handles initialization failures gracefully."""
mock_container.logs.side_effect = Exception('Test error')

streamer = LogStreamer(mock_container, mock_log_fn)
assert streamer.stdout_thread is None
assert streamer.log_generator is None
mock_log_fn.assert_called_with(
'error', 'Failed to initialize log streaming: Test error'
)


def test_stream_logs_without_generator(mock_container, mock_log_fn):
"""Test that _stream_logs handles missing log generator gracefully."""
streamer = LogStreamer(mock_container, mock_log_fn)
streamer.log_generator = None
streamer._stream_logs()
mock_log_fn.assert_called_with('error', 'Log generator not initialized')


def test_cleanup_without_thread(mock_container, mock_log_fn):
"""Test that cleanup works even if stdout_thread is not initialized."""
streamer = LogStreamer(mock_container, mock_log_fn)
streamer.stdout_thread = None
streamer.close() # Should not raise any exceptions


def test_normal_operation(mock_container, mock_log_fn):
"""Test normal operation of LogStreamer."""
mock_logs = [b'test log 1\n', b'test log 2\n']
mock_container.logs.return_value = mock_logs

streamer = LogStreamer(mock_container, mock_log_fn)
assert streamer.stdout_thread is not None
assert streamer.log_generator is not None

# Let the thread process the logs
streamer.close()

# Verify logs were processed
expected_calls = [
('debug', '[inside container] test log 1'),
('debug', '[inside container] test log 2'),
]
actual_calls = [
(args[0], args[1]) for args, _ in mock_log_fn.call_args_list
]
for expected in expected_calls:
assert expected in actual_calls


def test_del_without_thread(mock_container, mock_log_fn):
"""Test that __del__ works even if stdout_thread is not initialized."""
streamer = LogStreamer(mock_container, mock_log_fn)
delattr(streamer, 'stdout_thread') # Simulate case where thread was never created
streamer.__del__() # Should not raise any exceptions

0 comments on commit cbe1150

Please sign in to comment.