Fix(test,CI): runtime build tests (#3126)

* fix runtime build test

* get runtime_build test to run in CI

* move test involving docker from `test_ipython` to `test_sandbox`

.github/workflows/run-unit-tests.yml

@@ -125,7 +125,7 @@ jobs:
         run: make build
 
       - name: Run Tests
-        run: poetry run pytest --forked --cov=agenthub --cov=opendevin --cov-report=xml ./tests/unit -k "not test_sandbox and not test_runtime"
+        run: poetry run pytest --forked --cov=agenthub --cov=opendevin --cov-report=xml ./tests/unit -k "not test_sandbox.py and not test_runtime.py"
 
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v4
@@ -159,7 +159,7 @@ jobs:
         run: make build
 
       - name: Run Tests
-        run: poetry run pytest --forked --cov=agenthub --cov=opendevin --cov-report=xml ./tests/unit -k "not test_sandbox and not test_runtime"
+        run: poetry run pytest --forked --cov=agenthub --cov=opendevin --cov-report=xml ./tests/unit -k "not test_sandbox.py and not test_runtime.py"
 
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v4

tests/unit/test_ipython.py

@@ -7,8 +7,6 @@ import pytest
 from opendevin.core.config import SandboxConfig
 from opendevin.events.action import IPythonRunCellAction
 from opendevin.events.observation import IPythonRunCellObservation
-from opendevin.runtime.docker.ssh_box import DockerSSHBox
-from opendevin.runtime.plugins import JupyterRequirement
 from opendevin.runtime.server.runtime import ServerRuntime
 
 
@@ -75,28 +73,3 @@ async def test_run_python_backticks():
         assert (
             test_code == result.content
         ), f'The output should contain the expected print output, got: {result.content}'
-
-
-def test_sandbox_jupyter_plugin_backticks(temp_dir):
-    box = DockerSSHBox(
-        config=SandboxConfig(),
-        persist_sandbox=False,
-        workspace_mount_path=temp_dir,
-        sandbox_workspace_dir='/workspace',
-        cache_dir='/tmp/cache',
-        run_as_devin=True,
-    )
-    box.init_plugins([JupyterRequirement])
-    test_code = "print('Hello, `World`!')"
-    expected_write_command = (
-        "cat > /tmp/opendevin_jupyter_temp.py <<'EOL'\n" f'{test_code}\n' 'EOL'
-    )
-    expected_execute_command = 'cat /tmp/opendevin_jupyter_temp.py | execute_cli'
-    exit_code, output = box.execute(expected_write_command)
-    exit_code, output = box.execute(expected_execute_command)
-    print(output)
-    assert exit_code == 0, 'The exit code should be 0 for ' + box.__class__.__name__
-    assert output.strip() == 'Hello, `World`!', (
-        'The output should be the same as the input for ' + box.__class__.__name__
-    )
-    box.close()

tests/unit/test_runtime_build.py

@@ -9,11 +9,13 @@ import toml
 
 from opendevin.runtime.utils.runtime_build import (
     _generate_dockerfile,
+    _get_package_version,
     _put_source_code_to_dir,
     build_runtime_image,
     get_new_image_name,
 )
 
+OD_VERSION = f'od_v{_get_package_version()}'
 RUNTIME_IMAGE_PREFIX = 'od_runtime'
 
 
@@ -51,16 +53,15 @@ def test_generate_dockerfile_scratch():
         skip_init=False,
     )
     assert base_image in dockerfile_content
-    assert 'RUN apt update && apt install -y wget sudo' in dockerfile_content
+    assert 'apt-get update' in dockerfile_content
+    assert 'apt-get install -y wget sudo apt-utils' in dockerfile_content
     assert (
         'RUN /opendevin/miniforge3/bin/mamba install conda-forge::poetry -y'
         in dockerfile_content
     )
 
     # Check the update command
-    assert (
-        f'RUN mv /opendevin/{source_code_dirname} /opendevin/code' in dockerfile_content
-    )
+    assert f'mv /opendevin/{source_code_dirname} /opendevin/code' in dockerfile_content
     assert (
         '/opendevin/miniforge3/bin/mamba run -n base poetry install'
         in dockerfile_content
@@ -96,29 +97,41 @@ def test_generate_dockerfile_skip_init():
 def test_get_new_image_name_eventstream():
     base_image = 'debian:11'
     new_image_name = get_new_image_name(base_image)
-    assert new_image_name == f'{RUNTIME_IMAGE_PREFIX}:debian_tag_11'
+    assert new_image_name == f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_debian_tag_11'
 
     base_image = 'ubuntu:22.04'
     new_image_name = get_new_image_name(base_image)
-    assert new_image_name == f'{RUNTIME_IMAGE_PREFIX}:ubuntu_tag_22.04'
+    assert (
+        new_image_name == f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_ubuntu_tag_22.04'
+    )
 
     base_image = 'ubuntu'
     new_image_name = get_new_image_name(base_image)
-    assert new_image_name == f'{RUNTIME_IMAGE_PREFIX}:ubuntu_tag_latest'
+    assert (
+        new_image_name == f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_ubuntu_tag_latest'
+    )
 
 
 def test_get_new_image_name_eventstream_dev_mode():
-    base_image = f'{RUNTIME_IMAGE_PREFIX}:debian_tag_11'
+    base_image = f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_debian_tag_11'
     new_image_name = get_new_image_name(base_image, dev_mode=True)
-    assert new_image_name == f'{RUNTIME_IMAGE_PREFIX}_dev:debian_tag_11'
+    assert (
+        new_image_name == f'{RUNTIME_IMAGE_PREFIX}_dev:{OD_VERSION}_image_debian_tag_11'
+    )
 
-    base_image = f'{RUNTIME_IMAGE_PREFIX}:ubuntu_tag_22.04'
+    base_image = f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_ubuntu_tag_22.04'
     new_image_name = get_new_image_name(base_image, dev_mode=True)
-    assert new_image_name == f'{RUNTIME_IMAGE_PREFIX}_dev:ubuntu_tag_22.04'
+    assert (
+        new_image_name
+        == f'{RUNTIME_IMAGE_PREFIX}_dev:{OD_VERSION}_image_ubuntu_tag_22.04'
+    )
 
-    base_image = f'{RUNTIME_IMAGE_PREFIX}:ubuntu_tag_latest'
+    base_image = f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_ubuntu_tag_latest'
     new_image_name = get_new_image_name(base_image, dev_mode=True)
-    assert new_image_name == f'{RUNTIME_IMAGE_PREFIX}_dev:ubuntu_tag_latest'
+    assert (
+        new_image_name
+        == f'{RUNTIME_IMAGE_PREFIX}_dev:{OD_VERSION}_image_ubuntu_tag_latest'
+    )
 
 
 def test_get_new_image_name_eventstream_dev_invalid_base_image():
@@ -142,11 +155,11 @@ def test_build_runtime_image_from_scratch(mock_docker_client, mock_build_sandbox
     mock_docker_client.images.list.return_value = []
 
     image_name = build_runtime_image(base_image, mock_docker_client)
-    assert image_name == f'{RUNTIME_IMAGE_PREFIX}:debian_tag_11'
+    assert image_name == f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_debian_tag_11'
 
     mock_build_sandbox_image.assert_called_once_with(
         base_image,
-        f'{RUNTIME_IMAGE_PREFIX}:debian_tag_11',
+        f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_debian_tag_11',
         mock_docker_client,
         skip_init=False,
     )
@@ -159,11 +172,11 @@ def test_build_runtime_image_exist_no_update_source(
 ):
     base_image = 'debian:11'
     mock_docker_client.images.list.return_value = [
-        MagicMock(tags=[f'{RUNTIME_IMAGE_PREFIX}:debian_tag_11'])
+        MagicMock(tags=[f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_debian_tag_11'])
     ]
 
     image_name = build_runtime_image(base_image, mock_docker_client)
-    assert image_name == f'{RUNTIME_IMAGE_PREFIX}:debian_tag_11'
+    assert image_name == f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_debian_tag_11'
 
     mock_build_sandbox_image.assert_not_called()
 
@@ -175,17 +188,17 @@ def test_build_runtime_image_exist_with_update_source(
 ):
     base_image = 'debian:11'
     mock_docker_client.images.list.return_value = [
-        MagicMock(tags=[f'{RUNTIME_IMAGE_PREFIX}:debian_tag_11'])
+        MagicMock(tags=[f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_debian_tag_11'])
     ]
 
     image_name = build_runtime_image(
         base_image, mock_docker_client, update_source_code=True
     )
-    assert image_name == f'{RUNTIME_IMAGE_PREFIX}_dev:debian_tag_11'
+    assert image_name == f'{RUNTIME_IMAGE_PREFIX}_dev:{OD_VERSION}_image_debian_tag_11'
 
     mock_build_sandbox_image.assert_called_once_with(
-        f'{RUNTIME_IMAGE_PREFIX}:debian_tag_11',
-        f'{RUNTIME_IMAGE_PREFIX}_dev:debian_tag_11',
+        f'{RUNTIME_IMAGE_PREFIX}:{OD_VERSION}_image_debian_tag_11',
+        f'{RUNTIME_IMAGE_PREFIX}_dev:{OD_VERSION}_image_debian_tag_11',
         mock_docker_client,
         skip_init=True,
     )

tests/unit/test_sandbox.py

@@ -331,3 +331,28 @@ def test_agnostic_sandbox_jupyter_agentskills_fileop_pwd(temp_dir):
         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_plugin_backticks(temp_dir):
+    config = AppConfig(
+        sandbox=SandboxConfig(
+            box_type='ssh',
+            persist_sandbox=False,
+            enable_auto_lint=False,
+        )
+    )
+    box = create_docker_box_from_app_config(temp_dir, config)
+    box.init_plugins([JupyterRequirement])
+    test_code = "print('Hello, `World`!')"
+    expected_write_command = (
+        "cat > /tmp/opendevin_jupyter_temp.py <<'EOL'\n" f'{test_code}\n' 'EOL'
+    )
+    expected_execute_command = 'cat /tmp/opendevin_jupyter_temp.py | execute_cli'
+    exit_code, output = box.execute(expected_write_command)
+    exit_code, output = box.execute(expected_execute_command)
+    print(output)
+    assert exit_code == 0, 'The exit code should be 0 for ' + box.__class__.__name__
+    assert output.strip() == 'Hello, `World`!', (
+        'The output should be the same as the input for ' + box.__class__.__name__
+    )
+    box.close()