Test/improve config loading tests (#5399)

Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>

openhands/core/config/utils.py

@@ -94,6 +94,10 @@ def load_from_toml(cfg: AppConfig, toml_file: str = 'config.toml'):
     Args:
         cfg: The AppConfig object to update attributes of.
         toml_file: The path to the toml file. Defaults to 'config.toml'.
+
+    See Also:
+    - `config.template.toml` for the full list of config options.
+    - `SandboxConfig` for the sandbox-specific config options.
     """
     # try to read the config.toml file into the config object
     try:
@@ -161,11 +165,11 @@ def load_from_toml(cfg: AppConfig, toml_file: str = 'config.toml'):
                     )
             except (TypeError, KeyError) as e:
                 logger.openhands_logger.warning(
-                    f'Cannot parse config from toml, toml values have not been applied.\n Error: {e}',
+                    f'Cannot parse [{key}] config from toml, values have not been applied.\nError: {e}',
                     exc_info=False,
                 )
         else:
-            logger.openhands_logger.warning(f'Unknown key in {toml_file}: "{key}')
+            logger.openhands_logger.warning(f'Unknown section [{key}] in {toml_file}')
 
     try:
         # set sandbox config from the toml file
@@ -179,7 +183,9 @@ def load_from_toml(cfg: AppConfig, toml_file: str = 'config.toml'):
                 # read the key in sandbox and remove it from core
                 setattr(sandbox_config, new_key, core_config.pop(key))
             else:
-                logger.openhands_logger.warning(f'Unknown sandbox config: {key}')
+                logger.openhands_logger.warning(
+                    f'Unknown config key "{key}" in [sandbox] section'
+                )
 
         # the new style values override the old style values
         if 'sandbox' in toml_config:
@@ -191,10 +197,12 @@ def load_from_toml(cfg: AppConfig, toml_file: str = 'config.toml'):
             if hasattr(cfg, key):
                 setattr(cfg, key, value)
             else:
-                logger.openhands_logger.warning(f'Unknown core config key: {key}')
+                logger.openhands_logger.warning(
+                    f'Unknown config key "{key}" in [core] section'
+                )
     except (TypeError, KeyError) as e:
         logger.openhands_logger.warning(
-            f'Cannot parse config from toml, toml values have not been applied.\nError: {e}',
+            f'Cannot parse [sandbox] config from toml, values have not been applied.\nError: {e}',
             exc_info=False,
         )
 

tests/unit/test_config.py

@@ -1,4 +1,6 @@
+import logging
 import os
+from io import StringIO
 
 import pytest
 
@@ -11,6 +13,7 @@ from openhands.core.config import (
     load_from_env,
     load_from_toml,
 )
+from openhands.core.logger import openhands_logger
 
 
 @pytest.fixture
@@ -377,6 +380,42 @@ user_id = 1001
     assert default_config.sandbox.user_id == 1001
 
 
+def test_security_config_from_toml(default_config, temp_toml_file):
+    """Test loading security specific configurations."""
+    with open(temp_toml_file, 'w', encoding='utf-8') as toml_file:
+        toml_file.write(
+            """
+[core]  # make sure core is loaded first
+workspace_base = "/opt/files/workspace"
+
+[llm]
+model = "test-model"
+
+[security]
+confirmation_mode = false
+security_analyzer = "semgrep"
+"""
+        )
+
+    load_from_toml(default_config, temp_toml_file)
+    assert default_config.security.confirmation_mode is False
+    assert default_config.security.security_analyzer == 'semgrep'
+
+
+def test_security_config_from_dict():
+    """Test creating SecurityConfig instance from dictionary."""
+    from openhands.core.config.security_config import SecurityConfig
+
+    # Test with all fields
+    config_dict = {'confirmation_mode': True, 'security_analyzer': 'some_analyzer'}
+
+    security_config = SecurityConfig.from_dict(config_dict)
+
+    # Verify all fields are correctly set
+    assert security_config.confirmation_mode is True
+    assert security_config.security_analyzer == 'some_analyzer'
+
+
 def test_defaults_dict_after_updates(default_config):
     # Test that `defaults_dict` retains initial values after updates.
     initial_defaults = default_config.defaults_dict
@@ -408,10 +447,11 @@ def test_invalid_toml_format(monkeypatch, temp_toml_file, default_config):
     monkeypatch.setenv('LLM_MODEL', 'gpt-5-turbo-1106')
     monkeypatch.setenv('WORKSPACE_MOUNT_PATH', '/home/user/project')
     monkeypatch.delenv('LLM_API_KEY', raising=False)
+
     with open(temp_toml_file, 'w', encoding='utf-8') as toml_file:
         toml_file.write('INVALID TOML CONTENT')
 
-    load_from_toml(default_config)
+    load_from_toml(default_config, temp_toml_file)
     load_from_env(default_config, os.environ)
     default_config.jwt_secret = None  # prevent leak
     for llm in default_config.llms.values():
@@ -421,6 +461,122 @@ def test_invalid_toml_format(monkeypatch, temp_toml_file, default_config):
     assert default_config.workspace_mount_path == '/home/user/project'
 
 
+def test_load_from_toml_file_not_found(default_config):
+    """Test loading configuration when the TOML file doesn't exist.
+
+    This ensures that:
+    1. The program doesn't crash when the config file is missing
+    2. The config object retains its default values
+    3. The application remains usable
+    """
+    # Try to load from a non-existent file
+    load_from_toml(default_config, 'nonexistent.toml')
+
+    # Verify that config object maintains default values
+    assert default_config.get_llm_config() is not None
+    assert default_config.get_agent_config() is not None
+    assert default_config.sandbox is not None
+
+
+def test_core_not_in_toml(default_config, temp_toml_file):
+    """Test loading configuration when the core section is not in the TOML file.
+
+    default values should be used for the missing sections.
+    """
+    with open(temp_toml_file, 'w', encoding='utf-8') as toml_file:
+        toml_file.write("""
+[llm]
+model = "test-model"
+
+[agent]
+memory_enabled = true
+
+[sandbox]
+timeout = 1
+base_container_image = "custom_image"
+user_id = 1001
+
+[security]
+security_analyzer = "semgrep"
+""")
+
+    load_from_toml(default_config, temp_toml_file)
+    assert default_config.get_llm_config().model == 'claude-3-5-sonnet-20241022'
+    assert default_config.get_agent_config().memory_enabled is False
+    assert (
+        default_config.sandbox.base_container_image
+        == 'nikolaik/python-nodejs:python3.12-nodejs22'
+    )
+    # assert default_config.sandbox.user_id == 1007
+    assert default_config.security.security_analyzer is None
+
+
+def test_load_from_toml_partial_invalid(default_config, temp_toml_file, caplog):
+    """Test loading configuration with partially invalid TOML content.
+
+    This ensures that:
+    1. Valid configuration sections are properly loaded
+    2. Invalid fields are ignored gracefully
+    3. The config object maintains correct values for valid fields
+    4. Appropriate warnings are logged for invalid fields
+
+    See `openhands/core/schema/config.py` for the list of valid fields.
+    """
+    with open(temp_toml_file, 'w', encoding='utf-8') as f:
+        f.write("""
+[core]
+debug = true
+
+[llm]
+# No set in `openhands/core/schema/config.py`
+invalid_field = "test"
+model = "gpt-4"
+
+[agent]
+memory_enabled = true
+
+[sandbox]
+invalid_field_in_sandbox = "test"
+""")
+
+    # Create a string buffer to capture log output
+    # Referenced from test_logging.py and `mock_logger`
+    log_output = StringIO()
+    handler = logging.StreamHandler(log_output)
+    handler.setLevel(logging.WARNING)
+    formatter = logging.Formatter('%(message)s')
+    handler.setFormatter(formatter)
+    openhands_logger.addHandler(handler)
+
+    try:
+        load_from_toml(default_config, temp_toml_file)
+        log_content = log_output.getvalue()
+
+        # invalid [llm] config
+        # Verify that the appropriate warning was logged
+        assert 'Cannot parse [llm] config from toml' in log_content
+        assert 'values have not been applied' in log_content
+        # Error: LLMConfig.__init__() got an unexpected keyword argume
+        assert (
+            'Error: LLMConfig.__init__() got an unexpected keyword argume'
+            in log_content
+        )
+        assert 'invalid_field' in log_content
+
+        # invalid [sandbox] config
+        assert 'Cannot parse [sandbox] config from toml' in log_content
+        assert 'values have not been applied' in log_content
+        assert 'invalid_field_in_sandbox' in log_content
+
+        # Verify valid configurations are loaded. Load from default instead of `config.toml`
+        # assert default_config.debug is True
+        assert default_config.debug is False
+        assert default_config.get_llm_config().model == 'claude-3-5-sonnet-20241022'
+        assert default_config.get_agent_config().memory_enabled is True
+    finally:
+        openhands_logger.removeHandler(handler)
+
+
 def test_finalize_config(default_config):
     # Test finalize config
     assert default_config.workspace_mount_path is None