From d5b2ce18cb1508fff6b44d72b394a2ce6ec3996d Mon Sep 17 00:00:00 2001 From: Cheng Yang <93481273+young010101@users.noreply.github.com> Date: Fri, 3 Jan 2025 05:32:23 +0800 Subject: [PATCH] Test/improve config loading tests (#5399) Co-authored-by: Engel Nyst --- openhands/core/config/utils.py | 18 ++-- tests/unit/test_config.py | 158 ++++++++++++++++++++++++++++++++- 2 files changed, 170 insertions(+), 6 deletions(-) diff --git a/openhands/core/config/utils.py b/openhands/core/config/utils.py index e9bd80fbcb..c375d2d553 100644 --- a/openhands/core/config/utils.py +++ b/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, ) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index b3eb278b4f..d1306ca631 100644 --- a/tests/unit/test_config.py +++ b/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