Skip to content

Commit d32d49f

Browse files
fix: Update test to work with new TempFileManager - Fix test_integration_with_wait_for_process to use temp_file_manager.temp_files instead of removed executor.temp_files attribute - Resolves test failure caused by TempFileManager integration (Cursor)
1 parent da1f5bb commit d32d49f

File tree

1 file changed

+59
-58
lines changed

1 file changed

+59
-58
lines changed

mcp_tools/tests/test_memory_management.py

Lines changed: 59 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ def executor_with_limits(self):
3636
"periodic_status_interval": 30.0,
3737
"periodic_status_max_command_length": 60,
3838
}.get(key, default)
39-
39+
4040
executor = CommandExecutor()
4141
yield executor
42-
42+
4343
# Cleanup
4444
executor.stop_cleanup_task()
4545

@@ -57,33 +57,33 @@ def executor_with_auto_cleanup(self):
5757
"periodic_status_interval": 30.0,
5858
"periodic_status_max_command_length": 60,
5959
}.get(key, default)
60-
60+
6161
executor = CommandExecutor()
6262
yield executor
63-
63+
6464
# Cleanup
6565
executor.stop_cleanup_task()
6666

6767
def test_lru_eviction_when_limit_exceeded(self, executor_with_limits):
6868
"""Test that oldest completed processes are evicted when limit is exceeded."""
6969
executor = executor_with_limits
70-
70+
7171
# Add processes up to the limit
7272
for i in range(3):
7373
token = f"token_{i}"
7474
result = {"status": "completed", "output": f"output_{i}"}
7575
executor.completed_processes[token] = result
7676
executor.completed_process_timestamps[token] = time.time()
77-
77+
7878
assert len(executor.completed_processes) == 3
79-
79+
8080
# Add one more process - should trigger eviction
8181
token_new = "token_new"
8282
result_new = {"status": "completed", "output": "output_new"}
8383
executor.completed_processes[token_new] = result_new
8484
executor.completed_process_timestamps[token_new] = time.time()
8585
executor._enforce_completed_process_limit()
86-
86+
8787
# Should still have 3 processes, oldest should be evicted
8888
assert len(executor.completed_processes) == 3
8989
assert "token_0" not in executor.completed_processes # Oldest evicted
@@ -93,21 +93,21 @@ def test_lru_eviction_when_limit_exceeded(self, executor_with_limits):
9393
def test_ttl_cleanup_removes_expired_processes(self, executor_with_limits):
9494
"""Test that expired processes are removed based on TTL."""
9595
executor = executor_with_limits
96-
96+
9797
# Add some processes with different timestamps
9898
current_time = time.time()
99-
99+
100100
# Old process (expired)
101101
executor.completed_processes["old_token"] = {"status": "completed"}
102102
executor.completed_process_timestamps["old_token"] = current_time - 5 # 5 seconds ago
103-
103+
104104
# Recent process (not expired)
105105
executor.completed_processes["new_token"] = {"status": "completed"}
106106
executor.completed_process_timestamps["new_token"] = current_time - 1 # 1 second ago
107-
107+
108108
# Run cleanup
109109
cleanup_count = executor._cleanup_expired_processes()
110-
110+
111111
# Old process should be removed, new one should remain
112112
assert cleanup_count == 1
113113
assert "old_token" not in executor.completed_processes
@@ -117,18 +117,18 @@ def test_ttl_cleanup_removes_expired_processes(self, executor_with_limits):
117117
def test_manual_cleanup_with_force_all(self, executor_with_limits):
118118
"""Test manual cleanup with force_all option."""
119119
executor = executor_with_limits
120-
120+
121121
# Add some processes
122122
for i in range(3):
123123
token = f"token_{i}"
124124
executor.completed_processes[token] = {"status": "completed"}
125125
executor.completed_process_timestamps[token] = time.time()
126-
126+
127127
assert len(executor.completed_processes) == 3
128-
128+
129129
# Force cleanup all
130130
result = executor.cleanup_completed_processes(force_all=True)
131-
131+
132132
assert result["initial_count"] == 3
133133
assert result["cleaned_count"] == 3
134134
assert result["remaining_count"] == 0
@@ -139,19 +139,19 @@ def test_manual_cleanup_with_force_all(self, executor_with_limits):
139139
def test_manual_cleanup_without_force(self, executor_with_limits):
140140
"""Test manual cleanup without force_all (TTL-based only)."""
141141
executor = executor_with_limits
142-
142+
143143
current_time = time.time()
144-
144+
145145
# Add expired and non-expired processes
146146
executor.completed_processes["expired"] = {"status": "completed"}
147147
executor.completed_process_timestamps["expired"] = current_time - 5
148-
148+
149149
executor.completed_processes["fresh"] = {"status": "completed"}
150150
executor.completed_process_timestamps["fresh"] = current_time
151-
151+
152152
# Manual cleanup (TTL-based)
153153
result = executor.cleanup_completed_processes(force_all=False)
154-
154+
155155
assert result["initial_count"] == 2
156156
assert result["cleaned_count"] == 1 # Only expired one
157157
assert result["remaining_count"] == 1
@@ -162,17 +162,17 @@ def test_manual_cleanup_without_force(self, executor_with_limits):
162162
def test_memory_stats_reporting(self, executor_with_limits):
163163
"""Test memory statistics reporting."""
164164
executor = executor_with_limits
165-
165+
166166
# Add some processes with different ages
167167
current_time = time.time()
168168
executor.completed_processes["token1"] = {"status": "completed"}
169169
executor.completed_process_timestamps["token1"] = current_time - 10
170-
170+
171171
executor.completed_processes["token2"] = {"status": "completed"}
172172
executor.completed_process_timestamps["token2"] = current_time - 5
173-
173+
174174
stats = executor.get_memory_stats()
175-
175+
176176
assert stats["completed_processes_count"] == 2
177177
assert stats["max_completed_processes"] == 3
178178
assert stats["completed_process_ttl"] == 2
@@ -187,9 +187,9 @@ def test_memory_stats_reporting(self, executor_with_limits):
187187
def test_memory_stats_empty_processes(self, executor_with_limits):
188188
"""Test memory statistics when no completed processes exist."""
189189
executor = executor_with_limits
190-
190+
191191
stats = executor.get_memory_stats()
192-
192+
193193
assert stats["completed_processes_count"] == 0
194194
assert "oldest_process_age" not in stats
195195
assert "newest_process_age" not in stats
@@ -199,24 +199,24 @@ def test_memory_stats_empty_processes(self, executor_with_limits):
199199
async def test_background_cleanup_task(self, executor_with_auto_cleanup):
200200
"""Test that background cleanup task works correctly."""
201201
executor = executor_with_auto_cleanup
202-
202+
203203
# Ensure cleanup task is running
204204
executor._ensure_cleanup_task_running()
205-
205+
206206
# Add an expired process (older than TTL of 1 second)
207207
current_time = time.time()
208208
executor.completed_processes["expired"] = {"status": "completed"}
209209
executor.completed_process_timestamps["expired"] = current_time - 2 # Expired (2 seconds ago)
210-
210+
211211
# Add a fresh process
212212
executor.completed_processes["fresh"] = {"status": "completed"}
213213
executor.completed_process_timestamps["fresh"] = current_time
214-
214+
215215
assert len(executor.completed_processes) == 2
216-
216+
217217
# Wait for cleanup task to run (should run every 0.5 seconds, TTL is 1 second)
218218
await asyncio.sleep(1.0) # Wait 1 second for cleanup to happen
219-
219+
220220
# Expired process should be cleaned up
221221
assert "expired" not in executor.completed_processes
222222
assert "fresh" in executor.completed_processes
@@ -225,22 +225,22 @@ async def test_background_cleanup_task(self, executor_with_auto_cleanup):
225225
def test_lru_behavior_on_access(self, executor_with_limits):
226226
"""Test that accessing completed processes updates LRU order."""
227227
executor = executor_with_limits
228-
228+
229229
# Add processes
230230
for i in range(3):
231231
token = f"token_{i}"
232232
executor.completed_processes[token] = {"status": "completed"}
233233
executor.completed_process_timestamps[token] = time.time()
234-
234+
235235
# Access the first token (should move to end)
236236
result = executor.completed_processes["token_0"]
237237
executor.completed_processes.move_to_end("token_0")
238-
238+
239239
# Add one more process to trigger eviction
240240
executor.completed_processes["token_new"] = {"status": "completed"}
241241
executor.completed_process_timestamps["token_new"] = time.time()
242242
executor._enforce_completed_process_limit()
243-
243+
244244
# token_1 should be evicted (oldest), token_0 should remain (recently accessed)
245245
assert len(executor.completed_processes) == 3
246246
assert "token_1" not in executor.completed_processes # Evicted
@@ -252,24 +252,24 @@ def test_lru_behavior_on_access(self, executor_with_limits):
252252
async def test_cleanup_task_start_stop(self, executor_with_limits):
253253
"""Test starting and stopping cleanup task."""
254254
executor = executor_with_limits
255-
255+
256256
# Initially no task
257257
assert executor.cleanup_task is None
258-
258+
259259
# Start task (now that we have an event loop)
260260
executor.auto_cleanup_enabled = True
261261
executor.start_cleanup_task()
262262
assert executor.cleanup_task is not None
263263
assert not executor.cleanup_task.done()
264-
264+
265265
# Stop task
266266
executor.stop_cleanup_task()
267267
assert executor.cleanup_task is None
268268

269269
def test_cleanup_task_not_started_when_disabled(self, executor_with_limits):
270270
"""Test that cleanup task is not started when auto_cleanup_enabled is False."""
271271
executor = executor_with_limits
272-
272+
273273
# Should not start task when disabled
274274
executor.auto_cleanup_enabled = False
275275
executor.start_cleanup_task()
@@ -279,12 +279,12 @@ def test_zero_ttl_disables_ttl_cleanup(self, executor_with_limits):
279279
"""Test that TTL cleanup is disabled when TTL is set to 0."""
280280
executor = executor_with_limits
281281
executor.completed_process_ttl = 0
282-
282+
283283
# Add old process
284284
current_time = time.time()
285285
executor.completed_processes["old"] = {"status": "completed"}
286286
executor.completed_process_timestamps["old"] = current_time - 100
287-
287+
288288
# Cleanup should not remove anything when TTL is 0
289289
cleanup_count = executor._cleanup_expired_processes()
290290
assert cleanup_count == 0
@@ -294,35 +294,35 @@ def test_zero_ttl_disables_ttl_cleanup(self, executor_with_limits):
294294
async def test_destructor_cleanup(self, executor_with_limits):
295295
"""Test that destructor properly cleans up background task."""
296296
executor = executor_with_limits
297-
297+
298298
# Start task (now that we have an event loop)
299299
executor.auto_cleanup_enabled = True
300300
executor.start_cleanup_task()
301301
assert executor.cleanup_task is not None
302-
302+
303303
# Call destructor
304304
executor.__del__()
305-
305+
306306
# Task should be cleaned up (this test mainly ensures no exceptions)
307307
# We can't easily test the task state after __del__ due to cleanup
308308

309309
@pytest.mark.asyncio
310310
async def test_integration_with_wait_for_process(self, executor_with_limits):
311311
"""Test integration with wait_for_process method."""
312312
executor = executor_with_limits
313-
313+
314314
# Mock a completed process scenario
315315
token = "test_token"
316316
pid = 12345
317-
317+
318318
# Set up mock process data
319319
mock_process = MagicMock()
320320
mock_process.returncode = 0
321-
321+
322322
async def mock_wait():
323323
return None
324324
mock_process.wait = mock_wait
325-
325+
326326
executor.process_tokens[token] = pid
327327
executor.running_processes[pid] = {
328328
"process": mock_process,
@@ -332,24 +332,25 @@ async def mock_wait():
332332
"token": token,
333333
"start_time": time.time()
334334
}
335-
executor.temp_files[pid] = ("/tmp/stdout", "/tmp/stderr")
336-
335+
# Register temp files with the new TempFileManager
336+
executor.temp_file_manager.temp_files[pid] = ("/tmp/stdout", "/tmp/stderr", time.time())
337+
337338
# Mock file reading
338339
with patch.object(executor, '_read_temp_file') as mock_read:
339340
mock_read.return_value = "test output"
340-
341+
341342
with patch.object(executor, '_cleanup_temp_files') as mock_cleanup:
342343
mock_cleanup.return_value = None
343-
344+
344345
# Call wait_for_process
345346
result = await executor.wait_for_process(token)
346-
347+
347348
# Verify result is stored with timestamp and limits enforced
348349
assert token in executor.completed_processes
349350
assert token in executor.completed_process_timestamps
350351
assert result["status"] == "completed"
351352
assert result["output"] == "test output"
352-
353+
353354
# Verify the result is the same as what's stored
354355
stored_result = executor.completed_processes[token]
355-
assert stored_result == result
356+
assert stored_result == result

0 commit comments

Comments
 (0)