Move TaskExecutor tests to a separate test file

src/serena/agent.py

@@ -151,7 +151,7 @@ class SerenaAgent:
         log.info(f"Number of exposed tools: {len(self._exposed_tools)}")
 
         # create executor for starting the language server and running tools in another thread
-        # This executor is used to achieve linear task execution, so it is important to use a single-threaded executor.
+        # This executor is used to achieve linear task execution
         self._task_executor = TaskExecutor("SerenaAgentTaskExecutor")
 
         # Initialize the prompt factory

test/serena/test_serena_agent.py

@@ -61,14 +61,6 @@ def serena_agent(request: pytest.FixtureRequest, serena_config):
     return SerenaAgent(project=project_name, serena_config=serena_config)
 
 
-@pytest.fixture
-def basic_serena_agent(serena_config):
-    """
-    Fixture for a basic SerenaAgent without a project
-    """
-    return SerenaAgent(project=None, serena_config=serena_config)
-
-
 class TestSerenaAgent:
     @pytest.mark.parametrize(
         "serena_agent,symbol_name,expected_kind,expected_file",
@@ -292,103 +284,3 @@ class TestSerenaAgent:
 
         symbols = json.loads(result)
         assert not symbols, f"Expected to find no symbols for {name_path}. Symbols found: {symbols}"
-
-    class Task:
-        def __init__(self, delay: float, exception: bool = False):
-            self.delay = delay
-            self.exception = exception
-            self.did_run = False
-
-        def run(self):
-            self.did_run = True
-            time.sleep(self.delay)
-            if self.exception:
-                raise ValueError("Task failed")
-            return True
-
-    def test_task_queue_sequence(self, basic_serena_agent):
-        """
-        Tests that a sequence of tasks is executed correctly
-        """
-        future1 = basic_serena_agent.issue_task(self.Task(1).run, name="task1")
-        future2 = basic_serena_agent.issue_task(self.Task(1).run, name="task2")
-        assert future1.result() is True
-        assert future2.result() is True
-
-    def test_task_queue_exception(self, basic_serena_agent):
-        """
-        Tests that tasks that raise exceptions are handled correctly, i.e. that
-          * the exception is propagated,
-          * subsequent tasks are still executed.
-        """
-        future1 = basic_serena_agent.issue_task(self.Task(1, exception=True).run, name="task1")
-        future2 = basic_serena_agent.issue_task(self.Task(1).run, name="task2")
-        have_exception = False
-        try:
-            assert future1.result()
-        except Exception as e:
-            assert isinstance(e, ValueError)
-            have_exception = True
-        assert have_exception
-        assert future2.result() is True
-
-    def test_task_queue_cancel_current(self, basic_serena_agent):
-        """
-        Tests that tasks that are cancelled are handled correctly, i.e. that
-          * subsequent tasks are executed as soon as cancellation ensues.
-          * the cancelled task raises CancelledError when result() is called.
-        """
-        start_time = time.time()
-        future1 = basic_serena_agent.issue_task(self.Task(10).run, name="task1")
-        future2 = basic_serena_agent.issue_task(self.Task(1).run, name="task2")
-        time.sleep(1)
-        future1.cancel()
-        assert future2.result() is True
-        end_time = time.time()
-        assert (end_time - start_time) < 9, "Cancelled task did not stop in time"
-        have_cancelled_error = False
-        try:
-            future1.result()
-        except Exception as e:
-            assert e.__class__.__name__ == "CancelledError"
-            have_cancelled_error = True
-        assert have_cancelled_error
-
-    def test_task_queue_cancel_future(self, basic_serena_agent):
-        """
-        Tests that when a future task is cancelled, it is never run at all
-        """
-        task1 = self.Task(10)
-        task2 = self.Task(1)
-        future1 = basic_serena_agent.issue_task(task1.run, name="task1")
-        future2 = basic_serena_agent.issue_task(task2.run, name="task2")
-        time.sleep(1)
-        future2.cancel()
-        future1.cancel()
-        try:
-            future2.result()
-        except:
-            pass
-        assert task1.did_run
-        assert not task2.did_run
-
-    def test_task_queue_cancellation_via_task_info(self, basic_serena_agent):
-        start_time = time.time()
-        basic_serena_agent.issue_task(self.Task(10).run, "task1")
-        basic_serena_agent.issue_task(self.Task(1).run, "task2")
-        task_infos = basic_serena_agent.get_current_tasks()
-        task_infos2 = basic_serena_agent.get_current_tasks()
-
-        # test expected tasks
-        assert len(task_infos) == 2
-        assert "task1" in task_infos[0].name
-        assert "task2" in task_infos[1].name
-
-        # test task identifiers being stable
-        assert task_infos2[0].task_id == task_infos[0].task_id
-
-        # test cancellation
-        task_infos[0].cancel()
-        assert task_infos[1].future.result()
-        end_time = time.time()
-        assert (end_time - start_time) < 9, "Cancelled task did not stop in time"

test/serena/test_task_executor.py

@@ -0,0 +1,119 @@
+import time
+
+import pytest
+
+from serena.task_executor import TaskExecutor
+
+
+@pytest.fixture
+def executor():
+    """
+    Fixture for a basic SerenaAgent without a project
+    """
+    return TaskExecutor("TestExecutor")
+
+
+class Task:
+    def __init__(self, delay: float, exception: bool = False):
+        self.delay = delay
+        self.exception = exception
+        self.did_run = False
+
+    def run(self):
+        self.did_run = True
+        time.sleep(self.delay)
+        if self.exception:
+            raise ValueError("Task failed")
+        return True
+
+
+def test_task_executor_sequence(executor):
+    """
+    Tests that a sequence of tasks is executed correctly
+    """
+    future1 = executor.issue_task(Task(1).run, name="task1")
+    future2 = executor.issue_task(Task(1).run, name="task2")
+    assert future1.result() is True
+    assert future2.result() is True
+
+
+def test_task_executor_exception(executor):
+    """
+    Tests that tasks that raise exceptions are handled correctly, i.e. that
+      * the exception is propagated,
+      * subsequent tasks are still executed.
+    """
+    future1 = executor.issue_task(Task(1, exception=True).run, name="task1")
+    future2 = executor.issue_task(Task(1).run, name="task2")
+    have_exception = False
+    try:
+        assert future1.result()
+    except Exception as e:
+        assert isinstance(e, ValueError)
+        have_exception = True
+    assert have_exception
+    assert future2.result() is True
+
+
+def test_task_executor_cancel_current(executor):
+    """
+    Tests that tasks that are cancelled are handled correctly, i.e. that
+      * subsequent tasks are executed as soon as cancellation ensues.
+      * the cancelled task raises CancelledError when result() is called.
+    """
+    start_time = time.time()
+    future1 = executor.issue_task(Task(10).run, name="task1")
+    future2 = executor.issue_task(Task(1).run, name="task2")
+    time.sleep(1)
+    future1.cancel()
+    assert future2.result() is True
+    end_time = time.time()
+    assert (end_time - start_time) < 9, "Cancelled task did not stop in time"
+    have_cancelled_error = False
+    try:
+        future1.result()
+    except Exception as e:
+        assert e.__class__.__name__ == "CancelledError"
+        have_cancelled_error = True
+    assert have_cancelled_error
+
+
+def test_task_executor_cancel_future(executor):
+    """
+    Tests that when a future task is cancelled, it is never run at all
+    """
+    task1 = Task(10)
+    task2 = Task(1)
+    future1 = executor.issue_task(task1.run, name="task1")
+    future2 = executor.issue_task(task2.run, name="task2")
+    time.sleep(1)
+    future2.cancel()
+    future1.cancel()
+    try:
+        future2.result()
+    except:
+        pass
+    assert task1.did_run
+    assert not task2.did_run
+
+
+def test_task_executor_cancellation_via_task_info(executor):
+    start_time = time.time()
+    executor.issue_task(Task(10).run, "task1")
+    executor.issue_task(Task(1).run, "task2")
+    task_infos = executor.get_current_tasks()
+    task_infos2 = executor.get_current_tasks()
+
+    # test expected tasks
+    assert len(task_infos) == 2
+    assert "task1" in task_infos[0].name
+    assert "task2" in task_infos[1].name
+
+    # test task identifiers being stable
+    assert task_infos2[0].task_id == task_infos[0].task_id
+
+    # test cancellation
+    task_infos[0].cancel()
+    assert task_infos[1].future.result()
+    end_time = time.time()
+    assert (end_time - start_time) < 9, "Cancelled task did not stop in time"