Refactor: remove the use of global variable in test_sandbox (#2985)

* remove dependency of global config in sandbox test

* fix sandbox typo

* try to reduce code duplication

tests/unit/test_sandbox.py

@@ -1,17 +1,25 @@
 import os
 import pathlib
 import tempfile
-from unittest.mock import patch
 
 import pytest
 
-from opendevin.core.config import AppConfig, config
+from opendevin.core.config import AppConfig, SandboxConfig
 from opendevin.runtime.docker.local_box import LocalBox
 from opendevin.runtime.docker.ssh_box import DockerSSHBox, split_bash_commands
 from opendevin.runtime.plugins import AgentSkillsRequirement, JupyterRequirement
 
 
-def create_docker_box_from_app_config(config: AppConfig, path: str) -> DockerSSHBox:
+def create_docker_box_from_app_config(
+    path: str, config: AppConfig = None
+) -> DockerSSHBox:
+    if config is None:
+        config = AppConfig(
+            sandbox=SandboxConfig(
+                box_type='ssh',
+                persist_sandbox=False,
+            )
+        )
     return DockerSSHBox(
         config=config.sandbox,
         persist_sandbox=config.persist_sandbox,
@@ -36,9 +44,17 @@ def temp_dir(monkeypatch):
 
 def test_env_vars(temp_dir):
     os.environ['SANDBOX_ENV_FOOBAR'] = 'BAZ'
+    ssh_box = create_docker_box_from_app_config(temp_dir)
+
+    local_box_config = AppConfig(
+        sandbox=SandboxConfig(
+            box_type='local',
+        )
+    )
+    local_box = LocalBox(local_box_config.sandbox, temp_dir)
     for box in [
-        create_docker_box_from_app_config(config, temp_dir),
-        LocalBox(config.sandbox, temp_dir),
+        ssh_box,
+        local_box,
     ]:
         box.add_to_env(key='QUUX', value='abc"def')
         assert box._env['FOOBAR'] == 'BAZ'
@@ -109,7 +125,7 @@ EOF
 def test_ssh_box_run_as_devin(temp_dir):
     # get a temporary directory
     for box in [
-        create_docker_box_from_app_config(config, temp_dir)
+        create_docker_box_from_app_config(temp_dir),
     ]:  # FIXME: permission error on mkdir test for exec box
         exit_code, output = box.execute('ls -l')
         assert exit_code == 0, 'The exit code should be 0 for ' + box.__class__.__name__
@@ -140,7 +156,7 @@ def test_ssh_box_run_as_devin(temp_dir):
 
 
 def test_ssh_box_multi_line_cmd_run_as_devin(temp_dir):
-    box = create_docker_box_from_app_config(config, temp_dir)
+    box = create_docker_box_from_app_config(temp_dir)
     exit_code, output = box.execute('pwd && ls -l')
     assert exit_code == 0, 'The exit code should be 0 for ' + box.__class__.__name__
     expected_lines = ['/workspace', 'total 0']
@@ -152,7 +168,7 @@ def test_ssh_box_multi_line_cmd_run_as_devin(temp_dir):
 
 
 def test_ssh_box_stateful_cmd_run_as_devin(temp_dir):
-    box = create_docker_box_from_app_config(config, temp_dir)
+    box = create_docker_box_from_app_config(temp_dir)
     exit_code, output = box.execute('mkdir test')
     assert exit_code == 0, 'The exit code should be 0.'
     assert output.strip() == ''
@@ -172,7 +188,7 @@ def test_ssh_box_stateful_cmd_run_as_devin(temp_dir):
 
 
 def test_ssh_box_failed_cmd_run_as_devin(temp_dir):
-    box = create_docker_box_from_app_config(config, temp_dir)
+    box = create_docker_box_from_app_config(temp_dir)
     exit_code, output = box.execute('non_existing_command')
     assert exit_code != 0, (
         'The exit code should not be 0 for a failed command for '
@@ -182,7 +198,7 @@ def test_ssh_box_failed_cmd_run_as_devin(temp_dir):
 
 
 def test_single_multiline_command(temp_dir):
-    box = create_docker_box_from_app_config(config, temp_dir)
+    box = create_docker_box_from_app_config(temp_dir)
     exit_code, output = box.execute('echo \\\n -e "foo"')
     assert exit_code == 0, 'The exit code should be 0 for ' + box.__class__.__name__
     # FIXME: why is there a `>` in the output? Probably PS2?
@@ -193,7 +209,7 @@ def test_single_multiline_command(temp_dir):
 
 
 def test_multiline_echo(temp_dir):
-    box = create_docker_box_from_app_config(config, temp_dir)
+    box = create_docker_box_from_app_config(temp_dir)
     exit_code, output = box.execute('echo -e "hello\nworld"')
     assert exit_code == 0, 'The exit code should be 0 for ' + box.__class__.__name__
     # FIXME: why is there a `>` in the output?
@@ -204,7 +220,7 @@ def test_multiline_echo(temp_dir):
 
 
 def test_sandbox_whitespace(temp_dir):
-    box = create_docker_box_from_app_config(config, temp_dir)
+    box = create_docker_box_from_app_config(temp_dir)
     exit_code, output = box.execute('echo -e "\\n\\n\\n"')
     assert exit_code == 0, 'The exit code should be 0 for ' + box.__class__.__name__
     assert output == '\r\n\r\n\r\n', (
@@ -214,7 +230,7 @@ def test_sandbox_whitespace(temp_dir):
 
 
 def test_sandbox_jupyter_plugin(temp_dir):
-    box = create_docker_box_from_app_config(config, temp_dir)
+    box = create_docker_box_from_app_config(temp_dir)
     box.init_plugins([JupyterRequirement])
     exit_code, output = box.execute('echo "print(1)" | execute_cli')
     print(output)
@@ -225,7 +241,7 @@ def test_sandbox_jupyter_plugin(temp_dir):
     box.close()
 
 
-def _test_sandbox_jupyter_agentskills_fileop_pwd_impl(box):
+def _test_sandbox_jupyter_agentskills_fileop_pwd_impl(box, config: AppConfig):
     box.init_plugins([AgentSkillsRequirement, JupyterRequirement])
     exit_code, output = box.execute('mkdir test')
     print(output)
@@ -311,10 +327,30 @@ DO NOT re-run the same failed edit command. Running it again will lead to the sa
 
 def test_sandbox_jupyter_agentskills_fileop_pwd(temp_dir):
     # get a temporary directory
-    with patch.object(config.sandbox, 'enable_auto_lint', new=True):
-        assert config.sandbox.enable_auto_lint
-        box = create_docker_box_from_app_config(config, temp_dir)
-        _test_sandbox_jupyter_agentskills_fileop_pwd_impl(box)
+    config = AppConfig(
+        sandbox=SandboxConfig(
+            box_type='ssh',
+            persist_sandbox=False,
+            enable_auto_lint=False,
+        )
+    )
+    assert not config.sandbox.enable_auto_lint
+    box = create_docker_box_from_app_config(temp_dir, config)
+    _test_sandbox_jupyter_agentskills_fileop_pwd_impl(box, config)
+
+
+def test_sandbox_jupyter_agentskills_fileop_pwd_with_lint(temp_dir):
+    # get a temporary directory
+    config = AppConfig(
+        sandbox=SandboxConfig(
+            box_type='ssh',
+            persist_sandbox=False,
+            enable_auto_lint=True,
+        )
+    )
+    assert config.sandbox.enable_auto_lint
+    box = create_docker_box_from_app_config(temp_dir, config)
+    _test_sandbox_jupyter_agentskills_fileop_pwd_impl(box, config)
 
 
 @pytest.mark.skipif(
@@ -323,10 +359,14 @@ def test_sandbox_jupyter_agentskills_fileop_pwd(temp_dir):
 )
 def test_agnostic_sandbox_jupyter_agentskills_fileop_pwd(temp_dir):
     for base_sandbox_image in ['ubuntu:22.04', 'debian:11']:
-        # get a temporary directory
-        with patch.object(
-            config.sandbox, 'container_image', new=base_sandbox_image
-        ), patch.object(config.sandbox, 'enable_auto_lint', new=False):
-            assert not config.sandbox.enable_auto_lint
-            box = create_docker_box_from_app_config(config, temp_dir)
-            _test_sandbox_jupyter_agentskills_fileop_pwd_impl(box)
+        config = AppConfig(
+            sandbox=SandboxConfig(
+                box_type='ssh',
+                container_image=base_sandbox_image,
+                persist_sandbox=False,
+                enable_auto_lint=False,
+            )
+        )
+        assert not config.sandbox.enable_auto_lint
+        box = create_docker_box_from_app_config(temp_dir, config)
+        _test_sandbox_jupyter_agentskills_fileop_pwd_impl(box, config)