From a95c9676b4b211d0e2c5668b0a48f7883600d3f3 Mon Sep 17 00:00:00 2001 From: geens Date: Fri, 1 Nov 2024 15:45:54 +0100 Subject: [PATCH] Private instance vars, public get properties --- readme.md | 48 ++++---- sia/background_entry.py | 138 +++++++++++---------- sia/entry.py | 20 +-- sia/parse_error_entry.py | 27 +++-- sia/read_entry.py | 21 ++-- sia/reasoning_entry.py | 15 ++- sia/repeat_entry.py | 60 +++++---- sia/single_shot_entry.py | 52 +++++--- test/background_entry_test.py | 215 +++++++++++---------------------- test/delete_command_test.py | 68 +++++++---- test/llm_engine_test.py | 25 ++++ test/parse_error_entry_test.py | 20 +-- test/read_entry_test.py | 9 +- test/reasoning_entry_test.py | 4 +- test/repeat_entry_test.py | 18 +-- test/response_parser_test.py | 5 +- test/stop_command_test.py | 44 ++++--- test/working_memory_test.py | 67 +++++----- 18 files changed, 429 insertions(+), 427 deletions(-) create mode 100644 test/llm_engine_test.py diff --git a/readme.md b/readme.md index 8480ef8..334985a 100644 --- a/readme.md +++ b/readme.md @@ -328,8 +328,8 @@ classDiagram class Entry { <> - +id: str - +timestamp: datetime + +id: str readonly + +timestamp: datetime readonly +Entry(id str, timestamp datetime) +update() void* @@ -480,8 +480,8 @@ stateDiagram-v2 classDiagram class Entry { <> - +id: str - +timestamp: datetime + +id: str readonly + +timestamp: datetime readonly +Entry(id str, timestamp datetime) +update() void* @@ -490,10 +490,10 @@ classDiagram } class SingleShotEntry { - +script: str - +stdout: str - +stderr: str - +exit_code: Optional~int~ + +script: str readonly + +stdout: str readonly + +stderr: str readonly + +exit_code: Optional~int~ readonly +SingleShotEntry(script str, id str, timestamp datetime) +update() void @@ -501,10 +501,10 @@ classDiagram } class RepeatEntry { - +script: str - +stdout: str - +stderr: str - +exit_code: Optional~int~ + +script: str readonly + +stdout: str readonly + +stderr: str readonly + +exit_code: Optional~int~ readonly +RepeatEntry(script str, id str, timestamp datetime) +update() void @@ -512,13 +512,11 @@ classDiagram } class BackgroundEntry { - +script: str - +stdout: str - +stderr: str - +process: Optional~Process~ - -accumulated_stdout: str - -accumulated_stderr: str - -exit_code: Optional~int~ + +script: str readonly + +stdout: str readonly + +stderr: str readonly + +exit_code: Optional~int~ readonly + +pid: Optional~int~ readonly +BackgroundEntry(script str, id str, timestamp datetime) +update() void @@ -527,7 +525,7 @@ classDiagram } class ReasoningEntry { - +content: str + +content: str readonly +ReasoningEntry(content str, id str, timestamp datetime) +update() void @@ -535,8 +533,8 @@ classDiagram } class ParseErrorEntry { - +content: str - +error: str + +content: str readonly + +error: str readonly +ParseErrorEntry(content str, error str, id str, timestamp datetime) +update() void @@ -544,8 +542,7 @@ classDiagram } class ReadEntry { - +content: str - +io_buffer: IOBuffer + +content: str readonly +ReadEntry(io_buffer IOBuffer, id str, timestamp datetime) +update() void @@ -553,8 +550,7 @@ classDiagram } class WriteEntry { - +content: str - +io_buffer: IOBuffer + +content: str readonly +WriteEntry(content str, io_buffer IOBuffer, id str, timestamp datetime) +update() void diff --git a/sia/background_entry.py b/sia/background_entry.py index 3d87aa0..7e3e9d8 100644 --- a/sia/background_entry.py +++ b/sia/background_entry.py @@ -1,6 +1,6 @@ from datetime import datetime import subprocess -import xml.etree.ElementTree as ET +import xml.etree.ElementTree as ET from typing import Optional from .entry import Entry @@ -10,13 +10,11 @@ class BackgroundEntry(Entry): """ Entry type for long-running background processes. - Attributes: - script: The script/command to execute - stdout: Captured standard output since last update - stderr: Captured standard error since last update - process: The running subprocess.Popen instance - _accumulated_stdout: Total stdout collected so far - _accumulated_stderr: Total stderr collected so far + Private Attributes: + _script: The script/command to execute + _stdout: Captured standard output + _stderr: Captured standard error + _process: The running subprocess.Popen instance _exit_code: Exit code when process completes """ @@ -30,58 +28,73 @@ class BackgroundEntry(Entry): timestamp: Creation timestamp for this entry """ super().__init__(id, timestamp) - self.script = script - self.stdout = "" - self.stderr = "" - self.process: Optional[subprocess.Popen] = None - self._accumulated_stdout = "" - self._accumulated_stderr = "" + self._script = script + self._stdout = "" + self._stderr = "" + self._process: Optional[subprocess.Popen] = None self._exit_code: Optional[int] = None + @property + def script(self) -> str: + """Get the script/command being executed.""" + return self._script + + @property + def stdout(self) -> str: + """Get the captured standard output.""" + return self._stdout + + @property + def stderr(self) -> str: + """Get the captured standard error.""" + return self._stderr + + @property + def exit_code(self) -> Optional[int]: + """Get the exit code of the process (None if still running).""" + return self._exit_code + + @property + def pid(self) -> Optional[int]: + """Get the process ID (None if not running).""" + return self._process.pid if self._process is not None else None + def cleanup(self) -> None: """ Clean up the background process if it's still running. Ensures process is terminated and file handles are closed. """ - self._cleanup_process() - - def _cleanup_process(self): - """Clean up process and file handles""" - if self.process is not None: + if self._process is not None: try: # Close file handles if they're open - if self.process.stdout: - self.process.stdout.close() - if self.process.stderr: - self.process.stderr.close() + if self._process.stdout: + self._process.stdout.close() + if self._process.stderr: + self._process.stderr.close() # Terminate process if it's still running - if self.process.poll() is None: - self.process.terminate() + if self._process.poll() is None: + self._process.terminate() try: - self.process.wait(timeout=1.0) + self._process.wait(timeout=1.0) except subprocess.TimeoutExpired: - self.process.kill() - self.process.wait() + self._process.kill() + self._process.wait() except: pass # Ignore cleanup errors finally: - self.process = None + self._process = None def update(self) -> None: """ Start the process if not running and collect any new output. - Updates stdout and stderr with any new output since last update. + Updates stdout and stderr with any new output. """ try: - # Reset current output buffers - self.stdout = "" - self.stderr = "" - # Start process if not running - if self.process is None and self._exit_code is None: - self.process = subprocess.Popen( - self.script, + if self._process is None and self._exit_code is None: + self._process = subprocess.Popen( + self._script, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, @@ -90,57 +103,52 @@ class BackgroundEntry(Entry): ) return # Return after starting to allow process to generate output - if self.process is None: + if self._process is None: return # Process already completed # Check if process has finished - exit_code = self.process.poll() + exit_code = self._process.poll() if exit_code is not None: # Process has terminated, collect remaining output try: - remaining_out, remaining_err = self.process.communicate(timeout=0.1) - self.stdout = remaining_out - self.stderr = remaining_err - self._accumulated_stdout += remaining_out - self._accumulated_stderr += remaining_err + remaining_out, remaining_err = self._process.communicate(timeout=0.1) + self._stdout += remaining_out + self._stderr += remaining_err except subprocess.TimeoutExpired: pass # Process didn't finish communicating, try again next update self._exit_code = exit_code - self._cleanup_process() + self.cleanup() return # Read stdout if available - if self.process.stdout: + if self._process.stdout: while True: try: - line = self.process.stdout.readline() + line = self._process.stdout.readline() if not line: break - self.stdout += line - self._accumulated_stdout += line + self._stdout += line except: break # Read stderr if available - if self.process.stderr: + if self._process.stderr: while True: try: - line = self.process.stderr.readline() + line = self._process.stderr.readline() if not line: break - self.stderr += line - self._accumulated_stderr += line + self._stderr += line except: break except Exception as e: # Handle any errors error_msg = f"Error handling background process: {str(e)}" - self.stderr = error_msg - self._accumulated_stderr += f"\n{error_msg}" + self._stderr += f"\n{error_msg}" self._exit_code = -1 - self._cleanup_process() + self.cleanup() def generate_context(self) -> ET.Element: """ @@ -150,26 +158,22 @@ class BackgroundEntry(Entry): ET.Element: XML element containing the entry's data """ # Create root element with appropriate status - element = ET.Element("background", {"id": self.id}) + element = ET.Element("background", {"id": self._id}) - if self.process is not None: - element.set("pid", str(self.process.pid)) + if self._process is not None: + element.set("pid", str(self._process.pid)) elif self._exit_code is not None: element.set("exit_code", str(self._exit_code)) # Add script as CDATA or escaped text - element.text = escape_text_for_xml(self.script) + element.text = escape_text_for_xml(self._script) # Add stdout element with accumulated output stdout_elem = ET.SubElement(element, "stdout") - stdout_elem.text = escape_text_for_xml(self._accumulated_stdout) + stdout_elem.text = escape_text_for_xml(self._stdout) # Add stderr element with accumulated output stderr_elem = ET.SubElement(element, "stderr") - stderr_elem.text = escape_text_for_xml(self._accumulated_stderr) + stderr_elem.text = escape_text_for_xml(self._stderr) - return element - - def __del__(self): - """Ensure process is terminated and file handles are closed when entry is destroyed""" - self._cleanup_process() \ No newline at end of file + return element \ No newline at end of file diff --git a/sia/entry.py b/sia/entry.py index acbdc67..beadf5e 100644 --- a/sia/entry.py +++ b/sia/entry.py @@ -5,10 +5,6 @@ import xml.etree.ElementTree as ET class Entry(ABC): """ Abstract base class for all entry types in the working memory. - - Attributes: - id: Unique identifier for the entry - timestamp: When the entry was created """ def __init__(self, id: str, timestamp: datetime): @@ -19,9 +15,19 @@ class Entry(ABC): id: Unique identifier for this entry timestamp: Creation timestamp for this entry """ - self.id = id - self.timestamp = timestamp - + self._id = id + self._timestamp = timestamp + + @property + def id(self) -> str: + """Get entry's unique identifier.""" + return self._id + + @property + def timestamp(self) -> datetime: + """Get entry's creation timestamp.""" + return self._timestamp + @abstractmethod def update(self) -> None: """ diff --git a/sia/parse_error_entry.py b/sia/parse_error_entry.py index c90e89e..e71ea7d 100644 --- a/sia/parse_error_entry.py +++ b/sia/parse_error_entry.py @@ -7,10 +7,6 @@ from .util import escape_text_for_xml class ParseErrorEntry(Entry): """ Entry type for parse and validation errors. - - Attributes: - content: The original content that failed to parse - error: The error message describing what went wrong """ def __init__(self, content: str, error: str, id: str, timestamp: datetime): @@ -24,11 +20,20 @@ class ParseErrorEntry(Entry): timestamp: Creation timestamp for this entry """ super().__init__(id, timestamp) - self.content = content - self.error = error - + self._content = content + self._error = error + + @property + def content(self) -> str: + """Get the original content that failed to parse.""" + return self._content + + @property + def error(self) -> str: + """Get the error message describing the failure.""" + return self._error + def update(self) -> None: - """No update needed for parse error entries.""" pass def generate_context(self) -> ET.Element: @@ -39,14 +44,14 @@ class ParseErrorEntry(Entry): ET.Element: XML element containing the entry's data """ # Create root element - element = ET.Element("parse_error", {"id": self.id}) + element = ET.Element("parse_error", {"id": self._id}) # Add error subelement error_elem = ET.SubElement(element, "error") - error_elem.text = escape_text_for_xml(self.error) + error_elem.text = escape_text_for_xml(self._error) # Add content subelement content_elem = ET.SubElement(element, "content") - content_elem.text = escape_text_for_xml(self.content) + content_elem.text = escape_text_for_xml(self._content) return element \ No newline at end of file diff --git a/sia/read_entry.py b/sia/read_entry.py index a5275da..d088687 100644 --- a/sia/read_entry.py +++ b/sia/read_entry.py @@ -9,8 +9,8 @@ class ReadEntry(Entry): Entry type for reading content from standard input. Attributes: - content: Content read from stdin, empty until first update - io_buffer: Buffer to use for IO operations + _content: Content read from stdin, empty until first update + _io_buffer: Buffer to use for IO operations _read: Whether content has been read """ @@ -24,9 +24,16 @@ class ReadEntry(Entry): timestamp: Creation timestamp for this entry """ super().__init__(id, timestamp) - self.content = "" - self.io_buffer = io_buffer + self._content = "" + self._io_buffer = io_buffer self._read = False + + @property + def content(self) -> str: + """ + Get the content read from stdin. + """ + return self._content def update(self) -> None: """ @@ -34,7 +41,7 @@ class ReadEntry(Entry): Uses the provided IO buffer for the actual read operation. """ if not self._read: - self.content = self.io_buffer.read() + self._content = self._io_buffer.read() self._read = True def generate_context(self) -> ET.Element: @@ -45,10 +52,10 @@ class ReadEntry(Entry): ET.Element: XML element containing the entry's data """ # Create root element - element = ET.Element("read_stdin", {"id": self.id}) + element = ET.Element("read_stdin", {"id": self._id}) # Add content as CDATA or escaped text if content has been read if self._read: - element.text = escape_text_for_xml(self.content) + element.text = escape_text_for_xml(self._content) return element \ No newline at end of file diff --git a/sia/reasoning_entry.py b/sia/reasoning_entry.py index eca4d59..450111a 100644 --- a/sia/reasoning_entry.py +++ b/sia/reasoning_entry.py @@ -9,7 +9,7 @@ class ReasoningEntry(Entry): Entry type for agent reasoning steps. Attributes: - content: The reasoning text + _content: The reasoning text """ def __init__(self, content: str, id: str, timestamp: datetime): @@ -22,7 +22,14 @@ class ReasoningEntry(Entry): timestamp: Creation timestamp for this entry """ super().__init__(id, timestamp) - self.content = content + self._content = content + + @property + def content(self) -> str: + """ + Get the reasoning text. + """ + return self._content def update(self) -> None: """No update needed for reasoning entries.""" @@ -36,9 +43,9 @@ class ReasoningEntry(Entry): ET.Element: XML element containing the entry's data """ # Create root element - element = ET.Element("reasoning", {"id": self.id}) + element = ET.Element("reasoning", {"id": self._id}) # Add content as CDATA or escaped text - element.text = escape_text_for_xml(self.content) + element.text = escape_text_for_xml(self._content) return element \ No newline at end of file diff --git a/sia/repeat_entry.py b/sia/repeat_entry.py index 9476862..e7202ff 100644 --- a/sia/repeat_entry.py +++ b/sia/repeat_entry.py @@ -9,12 +9,6 @@ from .util import escape_text_for_xml class RepeatEntry(Entry): """ Entry type for scripts that are executed on every update. - - Attributes: - script: The script/command to execute - stdout: Captured standard output from most recent execution - stderr: Captured standard error from most recent execution - exit_code: Process exit code from most recent execution """ def __init__(self, script: str, id: str, timestamp: datetime): @@ -27,10 +21,30 @@ class RepeatEntry(Entry): timestamp: Creation timestamp for this entry """ super().__init__(id, timestamp) - self.script = script - self.stdout = "" - self.stderr = "" - self.exit_code: Optional[int] = None + self._script = script + self._stdout = "" + self._stderr = "" + self._exit_code: Optional[int] = None + + @property + def script(self) -> str: + """Get the script/command being executed.""" + return self._script + + @property + def stdout(self) -> str: + """Get the captured standard output.""" + return self._stdout + + @property + def stderr(self) -> str: + """Get the captured standard error.""" + return self._stderr + + @property + def exit_code(self) -> Optional[int]: + """Get the exit code of the process (None if still running).""" + return self._exit_code def update(self) -> None: """ @@ -40,28 +54,28 @@ class RepeatEntry(Entry): try: # Run script and capture output process = subprocess.run( - self.script, + self._script, shell=True, capture_output=True, text=True ) # Store results - self.stdout = process.stdout - self.stderr = process.stderr - self.exit_code = process.returncode + self._stdout = process.stdout + self._stderr = process.stderr + self._exit_code = process.returncode except subprocess.SubprocessError as e: # Handle subprocess errors by capturing the error message - self.stdout = "" - self.stderr = str(e) - self.exit_code = -1 + self._stdout = "" + self._stderr = str(e) + self._exit_code = -1 except Exception as e: # Handle any other errors - self.stdout = "" - self.stderr = f"Error executing script: {str(e)}" - self.exit_code = -1 + self._stdout = "" + self._stderr = f"Error executing script: {str(e)}" + self._exit_code = -1 def generate_context(self) -> ET.Element: """ @@ -73,7 +87,7 @@ class RepeatEntry(Entry): # Create root element element = ET.Element("repeat", { "id": self.id, - "exit_code": str(self.exit_code) if self.exit_code is not None else "-1" + "exit_code": str(self._exit_code) if self._exit_code is not None else "-1" }) # Add script as CDATA or escaped text @@ -81,10 +95,10 @@ class RepeatEntry(Entry): # Add stdout element stdout_elem = ET.SubElement(element, "stdout") - stdout_elem.text = escape_text_for_xml(self.stdout) + stdout_elem.text = escape_text_for_xml(self._stdout) # Add stderr element stderr_elem = ET.SubElement(element, "stderr") - stderr_elem.text = escape_text_for_xml(self.stderr) + stderr_elem.text = escape_text_for_xml(self._stderr) return element \ No newline at end of file diff --git a/sia/single_shot_entry.py b/sia/single_shot_entry.py index 908dfea..92cf206 100644 --- a/sia/single_shot_entry.py +++ b/sia/single_shot_entry.py @@ -28,11 +28,31 @@ class SingleShotEntry(Entry): timestamp: Creation timestamp for this entry """ super().__init__(id, timestamp) - self.script = script - self.stdout = "" - self.stderr = "" - self.exit_code: Optional[int] = None + self._script = script + self._stdout = "" + self._stderr = "" + self._exit_code: Optional[int] = None self._executed = False + + @property + def script(self) -> str: + """Get the script/command being executed.""" + return self._script + + @property + def stdout(self) -> str: + """Get the captured standard output.""" + return self._stdout + + @property + def stderr(self) -> str: + """Get the captured standard error.""" + return self._stderr + + @property + def exit_code(self) -> Optional[int]: + """Get the exit code of the process (None if still running).""" + return self._exit_code def update(self) -> None: """ @@ -52,21 +72,21 @@ class SingleShotEntry(Entry): ) # Store results - self.stdout = process.stdout - self.stderr = process.stderr - self.exit_code = process.returncode + self._stdout = process.stdout + self._stderr = process.stderr + self._exit_code = process.returncode except subprocess.SubprocessError as e: # Handle subprocess errors by capturing the error message - self.stdout = "" - self.stderr = str(e) - self.exit_code = -1 + self._stdout = "" + self._stderr = str(e) + self._exit_code = -1 except Exception as e: # Handle any other errors - self.stdout = "" - self.stderr = f"Error executing script: {str(e)}" - self.exit_code = -1 + self._stdout = "" + self._stderr = f"Error executing script: {str(e)}" + self._exit_code = -1 self._executed = True @@ -89,13 +109,13 @@ class SingleShotEntry(Entry): if self._executed: # Add stdout element if there is output stdout_elem = ET.SubElement(element, "stdout") - stdout_elem.text = escape_text_for_xml(self.stdout) + stdout_elem.text = escape_text_for_xml(self._stdout) # Add stderr element if there are errors stderr_elem = ET.SubElement(element, "stderr") - stderr_elem.text = escape_text_for_xml(self.stderr) + stderr_elem.text = escape_text_for_xml(self._stderr) # Add exit code attribute after execution - element.set("exit_code", str(self.exit_code)) + element.set("exit_code", str(self._exit_code)) return element \ No newline at end of file diff --git a/test/background_entry_test.py b/test/background_entry_test.py index c56d2f9..ec93cf2 100644 --- a/test/background_entry_test.py +++ b/test/background_entry_test.py @@ -2,8 +2,7 @@ import unittest from datetime import datetime import os import time -import signal -import platform +from typing import Optional from sia.background_entry import BackgroundEntry @@ -19,7 +18,7 @@ class BackgroundEntryTest(unittest.TestCase): self.cleanup_files() def tearDown(self): - """Clean up test files and ensure no processes are left running""" + """Clean up test files""" self.cleanup_files() def cleanup_files(self): @@ -27,53 +26,45 @@ class BackgroundEntryTest(unittest.TestCase): for filename in [self.test_filename, self.counter_file]: if os.path.exists(filename): os.remove(filename) - - def wait_for_process_start(self, entry, timeout=1.0): - """Wait for process to start, with timeout""" + + def wait_for_condition(self, entry: BackgroundEntry, condition: callable, timeout: float = 1.0) -> bool: + """Wait for a condition on the entry context to be met""" start_time = time.time() - while entry.process is None and time.time() - start_time < timeout: + while time.time() - start_time < timeout: entry.update() + context = entry.generate_context() + if condition(context): + return True time.sleep(0.1) - self.assertIsNotNone(entry.process, "Process failed to start") - - def wait_for_process_exit(self, entry, timeout=1.0): - """Wait for process to exit, with timeout""" - start_time = time.time() - while entry.process is not None and time.time() - start_time < timeout: - entry.update() - time.sleep(0.1) - self.assertIsNone(entry.process, "Process failed to exit") + return False def test_initialization(self): """Test entry initialization""" - script = "echo test" - entry = BackgroundEntry(script, self.test_id, self.test_timestamp) + entry = BackgroundEntry("echo test", self.test_id, self.test_timestamp) + context = entry.generate_context() - self.assertEqual(entry.script, script) - self.assertEqual(entry.stdout, "") - self.assertEqual(entry.stderr, "") - self.assertIsNone(entry.process) - self.assertEqual(entry.id, self.test_id) - self.assertEqual(entry.timestamp, self.test_timestamp) + # Initial context should have script but no pid or exit_code + self.assertEqual(context.text, "") + self.assertIsNone(context.get("pid")) + self.assertIsNone(context.get("exit_code")) def test_short_running_process(self): """Test execution of a quick process""" - script = "echo 'test'" - entry = BackgroundEntry(script, self.test_id, self.test_timestamp) + entry = BackgroundEntry("echo 'test'", self.test_id, self.test_timestamp) - # Start and wait for completion - entry.update() - self.wait_for_process_start(entry) - self.wait_for_process_exit(entry) + # Wait for process to complete + def is_complete(ctx): + return ctx.get("exit_code") == "0" + + self.assertTrue(self.wait_for_condition(entry, is_complete)) # Verify output - self.assertEqual(entry._accumulated_stdout.strip(), "test") - self.assertEqual(entry._accumulated_stderr, "") - self.assertEqual(entry._exit_code, 0) + context = entry.generate_context() + self.assertTrue("") def test_continuous_output(self): """Test process that generates continuous output""" - # Create a Python script that counts to 3 with minimal delay script = ( 'python3 -c "' 'import sys\n' @@ -85,143 +76,83 @@ class BackgroundEntryTest(unittest.TestCase): entry = BackgroundEntry(script, self.test_id, self.test_timestamp) - # Start process and wait for completion - entry.update() - self.wait_for_process_start(entry) - self.wait_for_process_exit(entry, timeout=2.0) - - # One more update to ensure we get all output - entry.update() - - # Verify all output was captured - output = entry._accumulated_stdout - self.assertIn('count 1', output, "Missing count 1") - self.assertIn('count 2', output, "Missing count 2") - self.assertIn('count 3', output, "Missing count 3") + # Wait for process to complete + def has_all_output(ctx): + stdout = ctx.find("stdout").text + return all(f"count {i}" in stdout for i in range(1, 4)) + + self.assertTrue(self.wait_for_condition(entry, has_all_output)) def test_process_failure(self): """Test handling of process failures""" entry = BackgroundEntry("nonexistentcommand", self.test_id, self.test_timestamp) - # Start and wait for failure - entry.update() - self.wait_for_process_exit(entry) - - # Verify error was captured - self.assertTrue(len(entry._accumulated_stderr) > 0) - self.assertEqual(entry._accumulated_stdout, "") - self.assertNotEqual(entry._exit_code, 0) - - def test_generate_context_running(self): - """Test XML context generation for running process""" - script = "sleep 1" + # Wait for process to fail + def has_failed(ctx): + return ctx.get("exit_code") is not None and ctx.get("exit_code") != "0" - entry = BackgroundEntry(script, self.test_id, self.test_timestamp) - entry.update() # Start process - self.wait_for_process_start(entry) + self.assertTrue(self.wait_for_condition(entry, has_failed)) - element = entry.generate_context() + # Verify error captured + context = entry.generate_context() + self.assertTrue(len(context.find("stderr").text) > 0) + self.assertEqual(context.find("stdout").text, "") - # Verify XML structure - self.assertEqual(element.tag, "background") - self.assertEqual(element.get("id"), self.test_id) - self.assertIsNotNone(element.get("pid")) - self.assertIsNone(element.get("exit_code")) - - # Clean up - entry._cleanup_process() - - def test_generate_context_completed(self): - """Test XML context generation for completed process""" - script = "echo 'test'" - entry = BackgroundEntry(script, self.test_id, self.test_timestamp) - - # Start and wait for completion - entry.update() - self.wait_for_process_start(entry) - self.wait_for_process_exit(entry) - - element = entry.generate_context() - - # Verify XML structure - self.assertEqual(element.tag, "background") - self.assertEqual(element.get("id"), self.test_id) - self.assertIsNone(element.get("pid")) - self.assertEqual(element.get("exit_code"), "0") - - stdout_text = element.find("stdout").text - self.assertTrue(stdout_text.startswith("")) - - def test_cleanup_on_deletion(self): - """Test process cleanup when entry is deleted""" - script = "sleep 10" - - entry = BackgroundEntry(script, self.test_id, self.test_timestamp) - entry.update() - self.wait_for_process_start(entry) - - # Verify process is running - self.assertIsNotNone(entry.process) - pid = entry.process.pid - - # Delete the entry - entry._cleanup_process() - del entry - - # Give process time to be cleaned up - time.sleep(0.1) - - # Verify process was terminated - with self.assertRaises(ProcessLookupError): - os.kill(pid, 0) # Check if process exists - def test_cleanup_running_process(self): """Test cleanup of running process""" - script = "sleep 10" - entry = BackgroundEntry(script, self.test_id, self.test_timestamp) + entry = BackgroundEntry("sleep 10", self.test_id, self.test_timestamp) - # Start the process - entry.update() - self.assertIsNotNone(entry.process) - process_pid = entry.process.pid + # Wait for process to start + def has_started(ctx): + return ctx.get("pid") is not None - # Cleanup should terminate process + self.assertTrue(self.wait_for_condition(entry, has_started)) + + # Get PID before cleanup + context = entry.generate_context() + pid = int(context.get("pid")) + + # Clean up and verify process terminated entry.cleanup() - # Verify process was terminated - self.assertIsNone(entry.process) + # Process should no longer exist time.sleep(0.1) # Give process time to terminate with self.assertRaises(ProcessLookupError): - import os - os.kill(process_pid, 0) + os.kill(pid, 0) def test_cleanup_completed_process(self): """Test cleanup of already completed process""" - script = "echo 'test'" - entry = BackgroundEntry(script, self.test_id, self.test_timestamp) + entry = BackgroundEntry("echo test", self.test_id, self.test_timestamp) - # Run and wait for completion - entry.update() - self.wait_for_process_exit(entry) + # Wait for completion + def is_complete(ctx): + return ctx.get("exit_code") == "0" + + self.assertTrue(self.wait_for_condition(entry, is_complete)) - # Cleanup should be safe for completed process + # Cleanup should be safe entry.cleanup() - self.assertIsNone(entry.process) def test_multiple_cleanup_calls(self): """Test that multiple cleanup calls are safe""" - script = "sleep 10" - entry = BackgroundEntry(script, self.test_id, self.test_timestamp) + entry = BackgroundEntry("sleep 10", self.test_id, self.test_timestamp) - # Start process - entry.update() - self.assertIsNotNone(entry.process) + # Wait for process to start + def has_started(ctx): + return ctx.get("pid") is not None + + self.assertTrue(self.wait_for_condition(entry, has_started)) + + # Get PID before cleanup + context = entry.generate_context() + pid = int(context.get("pid")) # Multiple cleanups should be safe entry.cleanup() entry.cleanup() entry.cleanup() - self.assertIsNone(entry.process) \ No newline at end of file + # Process should no longer exist + time.sleep(0.1) + with self.assertRaises(ProcessLookupError): + os.kill(pid, 0) \ No newline at end of file diff --git a/test/delete_command_test.py b/test/delete_command_test.py index 590acfd..fae910f 100644 --- a/test/delete_command_test.py +++ b/test/delete_command_test.py @@ -1,18 +1,59 @@ -import unittest from datetime import datetime +import os import time +import unittest from sia.working_memory import WorkingMemory from sia.delete_command import DeleteCommand from sia.reasoning_entry import ReasoningEntry from sia.background_entry import BackgroundEntry + + class DeleteCommandTest(unittest.TestCase): def setUp(self): """Set up test cases with a fresh working memory""" self.memory = WorkingMemory() self.test_id = "test-id-1234" self.test_timestamp = datetime(2024, 10, 31, 12, 0, 0) + + def wait_for_process_start(self, entry: BackgroundEntry, timeout=1.0): + """Wait for process to start, checking context pid""" + start_time = time.time() + while time.time() - start_time < timeout: + entry.update() + context = entry.generate_context() + if context.get("pid") is not None: + return True + time.sleep(0.1) + return False + + def test_delete_running_background_entry(self): + """Test deleting a background entry that is still running""" + # Create and start background process + entry = BackgroundEntry("sleep 10", "test-id", self.test_timestamp) + self.memory.add_entry(entry) + + # Wait for process to start + self.assertTrue(self.wait_for_process_start(entry)) + + # Get PID before deletion + context = entry.generate_context() + pid = int(context.get("pid")) + + # Delete entry and verify process terminated + command = DeleteCommand(entry.id) + result = command.execute(self.memory) + + self.assertTrue(result.success) + self.assertFalse(result.should_stop) + self.assertEqual(result.message, "") + self.assertEqual(self.memory.get_entries_count(), 0) + + # Verify process was terminated + time.sleep(0.1) # Give process time to terminate + with self.assertRaises(ProcessLookupError): + os.kill(pid, 0) def test_delete_existing_entry(self): """Test deleting an existing entry""" @@ -29,31 +70,6 @@ class DeleteCommandTest(unittest.TestCase): self.assertFalse(result.should_stop) self.assertEqual(result.message, "") self.assertEqual(self.memory.get_entries_count(), 0) - - def test_delete_running_background_entry(self): - """Test deleting a background entry that is still running""" - # Create a background entry with a long-running command - entry = BackgroundEntry("sleep 10", self.test_id, self.test_timestamp) - self.memory.add_entry(entry) - - # Start the process - entry.update() - self.assertIsNotNone(entry.process) - process_pid = entry.process.pid - - # Execute delete command - command = DeleteCommand(self.test_id) - result = command.execute(self.memory) - - # Verify process was terminated - time.sleep(0.1) # Give process time to terminate - self.assertTrue(result.success) - self.assertIsNone(entry.process) - - # Verify process no longer exists - with self.assertRaises(ProcessLookupError): - import os - os.kill(process_pid, 0) def test_delete_nonexistent_entry(self): """Test attempting to delete a nonexistent entry""" diff --git a/test/llm_engine_test.py b/test/llm_engine_test.py new file mode 100644 index 0000000..a11b9c5 --- /dev/null +++ b/test/llm_engine_test.py @@ -0,0 +1,25 @@ +import unittest + +from itertools import tee + +from . import test_data + +from sia.llm_engine import LlmEngine + +class LlmEngineTest(unittest.TestCase): + def setUp(self): + self.model_path = "/root/model" + + def test_initialization(self): + llm_engine = LlmEngine(self.model_path) + self.assertIsInstance(llm_engine, LlmEngine) + + def test_infer(self): + main_context = "This is a test" + llm_engine = LlmEngine(self.model_path) + tokens = llm_engine.infer(test_data.echo_system_prompt, main_context) + print_tokens, result_tokens = tee(tokens) + for token in print_tokens: + print(token, end="", flush=True) + result = ''.join(result_tokens) + self.assertEqual(result, f"{main_context}{main_context}") \ No newline at end of file diff --git a/test/parse_error_entry_test.py b/test/parse_error_entry_test.py index 788371d..cb7ea58 100644 --- a/test/parse_error_entry_test.py +++ b/test/parse_error_entry_test.py @@ -15,8 +15,6 @@ class ParseErrorEntryTest(unittest.TestCase): error = "parsing failed" entry = ParseErrorEntry(content, error, self.test_id, self.test_timestamp) - self.assertEqual(entry.content, content) - self.assertEqual(entry.error, error) self.assertEqual(entry.id, self.test_id) self.assertEqual(entry.timestamp, self.test_timestamp) @@ -27,15 +25,13 @@ class ParseErrorEntryTest(unittest.TestCase): entry = ParseErrorEntry(content, error, self.test_id, self.test_timestamp) # Store initial state - initial_content = entry.content - initial_error = entry.error + initial_context = str(entry.generate_context()) # Perform update entry.update() # Verify state hasn't changed - self.assertEqual(entry.content, initial_content) - self.assertEqual(entry.error, initial_error) + self.assertEqual(str(entry.generate_context()), initial_context) def test_generate_context(self): """Test XML context generation""" @@ -75,14 +71,4 @@ class ParseErrorEntryTest(unittest.TestCase): element = entry.generate_context() self.assertEqual(element.find("content").text, "") - self.assertEqual(element.find("error").text, "") - - def test_large_content(self): - """Test handling large content and error messages""" - content = "x" * 10000 - error = "y" * 10000 - entry = ParseErrorEntry(content, error, self.test_id, self.test_timestamp) - - element = entry.generate_context() - self.assertEqual(element.find("content").text, f"") - self.assertEqual(element.find("error").text, f"") \ No newline at end of file + self.assertEqual(element.find("error").text, "") \ No newline at end of file diff --git a/test/read_entry_test.py b/test/read_entry_test.py index a24fa2b..2cd1f3f 100644 --- a/test/read_entry_test.py +++ b/test/read_entry_test.py @@ -18,7 +18,6 @@ class ReadEntryTest(unittest.TestCase): self.assertEqual(entry.content, "") self.assertEqual(entry.id, self.test_id) self.assertEqual(entry.timestamp, self.test_timestamp) - self.assertFalse(entry._read) def test_single_read(self): """Test reading content once""" @@ -28,10 +27,8 @@ class ReadEntryTest(unittest.TestCase): entry = ReadEntry(self.io_buffer, self.test_id, self.test_timestamp) entry.update() - # Verify content was read self.assertEqual(entry.content, test_input) - self.assertTrue(entry._read) - self.assertEqual(self.io_buffer.buffer_length(), 0) # Buffer should be cleared + self.assertEqual(self.io_buffer.buffer_length(), 0) def test_multiple_updates(self): """Test that content is only read once even with multiple updates""" @@ -42,14 +39,11 @@ class ReadEntryTest(unittest.TestCase): entry.update() initial_content = entry.content - # Add more input and update again self.io_buffer.append_stdin("additional input") entry.update() entry.update() - # Verify content hasn't changed after first read self.assertEqual(entry.content, initial_content) - self.assertTrue(entry._read) def test_empty_input(self): """Test reading when no input is available""" @@ -57,7 +51,6 @@ class ReadEntryTest(unittest.TestCase): entry.update() self.assertEqual(entry.content, "") - self.assertTrue(entry._read) def test_special_characters(self): """Test reading content with special characters""" diff --git a/test/reasoning_entry_test.py b/test/reasoning_entry_test.py index 8041f1d..89373c7 100644 --- a/test/reasoning_entry_test.py +++ b/test/reasoning_entry_test.py @@ -24,13 +24,13 @@ class ReasoningEntryTest(unittest.TestCase): entry = ReasoningEntry(content, self.test_id, self.test_timestamp) # Store initial state - initial_content = entry.content + initial_content = entry.generate_context().text # Perform update entry.update() # Verify state hasn't changed - self.assertEqual(entry.content, initial_content) + self.assertEqual(entry.generate_context().text, initial_content) def test_generate_context(self): """Test XML context generation""" diff --git a/test/repeat_entry_test.py b/test/repeat_entry_test.py index 18c97dd..811a75a 100644 --- a/test/repeat_entry_test.py +++ b/test/repeat_entry_test.py @@ -145,20 +145,4 @@ class RepeatEntryTest(unittest.TestCase): outputs = [first_output, second_output, third_output] unique_outputs = set(outputs) self.assertEqual(len(unique_outputs), 3) - self.assertEqual(outputs, ['Output 1', 'Output 2', 'Output 3']) - - def test_error_recovery(self): - """Test that entry recovers from errors in subsequent updates""" - # Start with a failing command - entry = RepeatEntry("nonexistentcommand", self.test_id, self.test_timestamp) - entry.update() - self.assertNotEqual(entry.exit_code, 0) - - # Change to a working command - entry.script = "echo 'test'" - entry.update() - - # Verify recovery - self.assertEqual(entry.exit_code, 0) - self.assertEqual(entry.stdout.strip(), "test") - self.assertEqual(entry.stderr, "") \ No newline at end of file + self.assertEqual(outputs, ['Output 1', 'Output 2', 'Output 3']) \ No newline at end of file diff --git a/test/response_parser_test.py b/test/response_parser_test.py index 4df8f2b..9c13d3d 100644 --- a/test/response_parser_test.py +++ b/test/response_parser_test.py @@ -52,8 +52,8 @@ class ResponseParserTest(unittest.TestCase): result = self.parser.parse(xml) self.assertIsInstance(result, BackgroundEntry) - self.assertEqual(result.script, "echo test") - + self.assertEqual(result.script, "echo test") + def test_background_entry_empty_script(self): """Test parsing background entry with empty script returns error entry""" xml = '' @@ -120,7 +120,6 @@ class ResponseParserTest(unittest.TestCase): result = self.parser.parse(xml) self.assertIsInstance(result, ReadEntry) - self.assertEqual(result.io_buffer, self.io_buffer) def test_write_stdout_entry(self): """Test parsing write stdout entry""" diff --git a/test/stop_command_test.py b/test/stop_command_test.py index c7a0e21..354201f 100644 --- a/test/stop_command_test.py +++ b/test/stop_command_test.py @@ -1,10 +1,10 @@ -import unittest from datetime import datetime +import os import time +import unittest from sia.working_memory import WorkingMemory from sia.stop_command import StopCommand -from sia.reasoning_entry import ReasoningEntry from sia.background_entry import BackgroundEntry class StopCommandTest(unittest.TestCase): @@ -12,34 +12,38 @@ class StopCommandTest(unittest.TestCase): """Set up test cases with a fresh working memory""" self.memory = WorkingMemory() self.test_timestamp = datetime(2024, 10, 31, 12, 0, 0) - + + def wait_for_process_start(self, entry: BackgroundEntry, timeout=1.0): + """Wait for process to start, checking context pid""" + start_time = time.time() + while time.time() - start_time < timeout: + entry.update() + context = entry.generate_context() + if context.get("pid") is not None: + return True + time.sleep(0.1) + return False + def test_stop_command_cleanup(self): """Test stop command cleans up and removes all entries""" - # Add a mix of entry types including a background process - entries = [ - ReasoningEntry("test content", "id-1", self.test_timestamp), - BackgroundEntry("sleep 10", "id-2", self.test_timestamp) - ] + # Add background process + entry = BackgroundEntry("sleep 10", "test-id", self.test_timestamp) + self.memory.add_entry(entry) - for entry in entries: - self.memory.add_entry(entry) - - # Start the background process - background_entry = self.memory.get_entry("id-2") - background_entry.update() - self.assertIsNotNone(background_entry.process) - process_pid = background_entry.process.pid + # Wait for process to start and get PID + self.assertTrue(self.wait_for_process_start(entry)) + context = entry.generate_context() + pid = int(context.get("pid")) # Execute stop command command = StopCommand() result = command.execute(self.memory) - # Verify all entries removed + # Verify entries removed and process terminated self.assertTrue(result.should_stop) self.assertEqual(self.memory.get_entries_count(), 0) - # Verify background process was terminated + # Verify process was terminated time.sleep(0.1) # Give process time to terminate with self.assertRaises(ProcessLookupError): - import os - os.kill(process_pid, 0) \ No newline at end of file + os.kill(pid, 0) \ No newline at end of file diff --git a/test/working_memory_test.py b/test/working_memory_test.py index e933406..cd74c50 100644 --- a/test/working_memory_test.py +++ b/test/working_memory_test.py @@ -1,4 +1,5 @@ from datetime import datetime +import os import time import unittest import xml.etree.ElementTree as ET @@ -12,6 +13,7 @@ from sia.web_io_buffer import WebIOBuffer from sia.working_memory import WorkingMemory from sia.write_entry import WriteEntry + class MockEntry(Entry): """Mock entry class for testing""" def __init__(self, id: str, timestamp: datetime): @@ -31,6 +33,17 @@ class WorkingMemoryTest(unittest.TestCase): """Set up test cases with a fresh working memory""" self.memory = WorkingMemory() self.test_timestamp = datetime(2024, 10, 31, 12, 0, 0) + + def wait_for_process_start(self, entry: BackgroundEntry, timeout=1.0): + """Wait for process to start, checking context pid""" + start_time = time.time() + while time.time() - start_time < timeout: + entry.update() + context = entry.generate_context() + if context.get("pid") is not None: + return True + time.sleep(0.1) + return False def test_initialization(self): """Test initial state of working memory""" @@ -147,46 +160,40 @@ class WorkingMemoryTest(unittest.TestCase): def test_remove_entry_cleanup(self): """Test that removing an entry triggers cleanup""" - # Create a background process entry - entry = BackgroundEntry("sleep 10", "test-id", datetime(2024, 10, 31, 12, 0, 0)) + # Create and start background process + entry = BackgroundEntry("sleep 10", "test-id", self.test_timestamp) self.memory.add_entry(entry) - # Start the process - entry.update() - self.assertIsNotNone(entry.process) - process_pid = entry.process.pid + # Wait for process to start and get PID + self.assertTrue(self.wait_for_process_start(entry)) + context = entry.generate_context() + pid = int(context.get("pid")) - # Remove entry should trigger cleanup - self.memory.remove_entry("test-id") + # Remove entry and verify process terminated + self.memory.remove_entry(entry.id) - # Verify process was terminated time.sleep(0.1) # Give process time to terminate with self.assertRaises(ProcessLookupError): - import os - os.kill(process_pid, 0) - + os.kill(pid, 0) + def test_cleanup_on_memory_clear(self): """Test that clearing memory properly cleans up all entries""" # Add multiple background processes - processes = [] + pids = [] for i in range(3): - entry = BackgroundEntry( - "sleep 10", - f"id-{i}", - datetime(2024, 10, 31, 12, 0, 0) - ) + entry = BackgroundEntry("sleep 10", f"id-{i}", self.test_timestamp) self.memory.add_entry(entry) - entry.update() - processes.append(entry.process.pid) + self.assertTrue(self.wait_for_process_start(entry)) + context = entry.generate_context() + pids.append(int(context.get("pid"))) # Clear memory self.memory.clear() # Verify all processes were terminated time.sleep(0.1) # Give processes time to terminate - for pid in processes: + for pid in pids: with self.assertRaises(ProcessLookupError): - import os os.kill(pid, 0) self.assertEqual(self.memory.get_entries_count(), 0) @@ -194,14 +201,13 @@ class WorkingMemoryTest(unittest.TestCase): def test_cleanup_on_memory_deletion(self): """Test that deleting memory properly cleans up all entries""" # Add a background process - entry = BackgroundEntry( - "sleep 10", - "test-id", - datetime(2024, 10, 31, 12, 0, 0) - ) + entry = BackgroundEntry("sleep 10", "test-id", self.test_timestamp) self.memory.add_entry(entry) - entry.update() - process_pid = entry.process.pid + + # Wait for process to start and get PID + self.assertTrue(self.wait_for_process_start(entry)) + context = entry.generate_context() + pid = int(context.get("pid")) # Delete memory del self.memory @@ -209,8 +215,7 @@ class WorkingMemoryTest(unittest.TestCase): # Verify process was terminated time.sleep(0.1) # Give process time to terminate with self.assertRaises(ProcessLookupError): - import os - os.kill(process_pid, 0) + os.kill(pid, 0) def test_get_entries(self): """Test getting all entries"""