From 247afa0fecbae944490aaaaf0a0f31a08dbed793 Mon Sep 17 00:00:00 2001 From: Jose Rodriguez Date: Sun, 8 Oct 2023 13:48:18 +0200 Subject: [PATCH] fix: prevents crash on config load/save errors --- src/api/config.py | 32 ++++++++++++++++--------- src/zxbc/args_config.py | 5 ++-- tests/api/test_config.py | 50 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 72 insertions(+), 15 deletions(-) diff --git a/src/api/config.py b/src/api/config.py index 2a3b38748..78614b04a 100644 --- a/src/api/config.py +++ b/src/api/config.py @@ -93,11 +93,16 @@ class OPTION(str, Enum): } -def load_config_from_file(filename: str, section: str, options_: options.Options = None, stop_on_error=True) -> bool: +def load_config_from_file( + filename: str, section: ConfigSections, options_: options.Options | None = None, *, stop_on_error: bool = True +) -> bool: """Opens file and read options from the given section. If stop_on_error is set, - the program stop if any error is found. Otherwise the result of the operation will be + the program stop if any error is found. Otherwise, the result of the operation will be returned (True on success, False on failure) """ + assert isinstance(section, ConfigSections) + section_ = section.value + if options_ is None: options_ = OPTIONS @@ -115,25 +120,30 @@ def load_config_from_file(filename: str, section: str, options_: options.Options sys.exit(1) return False - if section not in cfg.sections(): - errmsg.msg_output(f"Section '{section}' not found in config file '{filename}'") + if section_ not in cfg.sections(): + errmsg.msg_output(f"Section '{section_}' not found in config file '{filename}'") if stop_on_error: sys.exit(1) return False parsing: Dict[type, Callable] = {int: cfg.getint, float: cfg.getfloat, bool: cfg.getboolean} - for opt in cfg.options(section): + for opt in cfg.options(section_): options_[opt].value = parsing.get(options_[opt].type, cfg.get)(section=section, option=opt) return True -def save_config_into_file(filename: str, section: str, options_: options.Options = None, stop_on_error=True) -> bool: +def save_config_into_file( + filename: str, section: ConfigSections, options_: options.Options | None = None, *, stop_on_error: bool = True +) -> bool: """Save config into config ini file into the given section. If stop_on_error is set, - the program stop. Otherwise the result of the operation will be + the program stop. Otherwise, the result of the operation will be returned (True on success, False on failure) """ + assert isinstance(section, ConfigSections) + section_ = section.value + if options_ is None: options_ = OPTIONS @@ -147,16 +157,16 @@ def save_config_into_file(filename: str, section: str, options_: options.Options sys.exit(1) return False - cfg[section] = {} + cfg[section_] = {} for opt_name, opt in options_().items(): if opt_name.startswith("__") or opt.value is None or opt_name in OPTIONS_NOT_SAVED: continue if opt.type == bool: - cfg[section][opt_name] = str(opt.value).lower() + cfg[section_][opt_name] = str(opt.value).lower() continue - cfg[section][opt_name] = str(opt.value) + cfg[section_][opt_name] = str(opt.value) try: with open(filename, "wt", encoding="utf-8") as f: @@ -170,7 +180,7 @@ def save_config_into_file(filename: str, section: str, options_: options.Options return True -def init(): +def init() -> None: """Default Options and Compilation Flags""" OPTIONS(Action.CLEAR) diff --git a/src/zxbc/args_config.py b/src/zxbc/args_config.py index 028655591..ac6dbd922 100644 --- a/src/zxbc/args_config.py +++ b/src/zxbc/args_config.py @@ -7,7 +7,7 @@ import src.api.config import src.api.global_ as gl from src import arch -from src.api import debug, errmsg +from src.api import errmsg from src.api.config import OPTIONS from src.api.utils import open_file from src.zxbc import args_parser @@ -15,7 +15,7 @@ if TYPE_CHECKING: from argparse import Namespace -__all__ = ["FileType", "parse_options", "set_option_defines"] +__all__ = "FileType", "parse_options", "set_option_defines" class FileType: @@ -101,7 +101,6 @@ def parse_options(args: list[str] | None = None) -> Namespace: OPTIONS.case_insensitive = True OPTIONS.case_insensitive = options.ignore_case - debug.ENABLED = OPTIONS.debug_level > 0 if options.basic and not options.tzx and not options.tap: parser.error("Option --BASIC and --autorun requires --tzx or tap format") diff --git a/tests/api/test_config.py b/tests/api/test_config.py index 95c05ce5f..b85de00db 100644 --- a/tests/api/test_config.py +++ b/tests/api/test_config.py @@ -1,8 +1,10 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- - import sys import unittest +from unittest import mock + +import pytest from src.api import config, global_ @@ -100,3 +102,49 @@ def test_autorun_ignore_none(self): config.OPTIONS.autorun = True config.OPTIONS.autorun = None self.assertEqual(config.OPTIONS.autorun, True) + + @mock.patch("os.path.exists", lambda x: False) + def test_save_config_for_the_1st_time(self): + m = mock.mock_open() + with mock.patch("builtins.open", m): + config.save_config_into_file("dummy_filename", config.ConfigSections.ZXBC) + + m().write.assert_has_calls([mock.call("[zxbc]\n")], any_order=True) + + def test_save_config_does_not_accept_invalid_section(self): + with pytest.raises(AssertionError): + config.save_config_into_file("dummy_filename", "invalid_section") + + def test_load_config_from_file(self): + m = mock.mock_open(read_data="[zxbc]\nheap_size=1234\n") + config.OPTIONS(config.Action.ADD_IF_NOT_DEFINED, name="heap_size", type=int, default=4768, ignore_none=True) + + with mock.patch("builtins.open", m): + config.load_config_from_file("dummy_filename", config.ConfigSections.ZXBC) + + self.assertEqual(config.OPTIONS.heap_size, 1234) + + def test_load_config_from_file_fails_if_no_section(self): + m = mock.mock_open(read_data="[zxbasm]\norg=1234\n") + + with mock.patch("builtins.open", m): + result = config.load_config_from_file("dummy_filename", config.ConfigSections.ZXBC, stop_on_error=False) + + self.assertFalse(result) + + @mock.patch("os.path.exists", lambda x: False) + def test_load_config_from_file_fails_if_no_file(self): + result = config.load_config_from_file("dummy_filename", config.ConfigSections.ZXBC, stop_on_error=False) + self.assertFalse(result) + + def test_load_config_from_file_fails_if_duplicated_fields(self): + m = mock.mock_open(read_data="[zxbc]\nheap_size=1234\nheap_size=5678\n") + + with mock.patch("builtins.open", m): + result = config.load_config_from_file("dummy_filename", config.ConfigSections.ZXBC, stop_on_error=False) + + self.assertFalse(result) + + def test_load_config_does_not_accept_invalid_section(self): + with pytest.raises(AssertionError): + config.load_config_from_file("dummy_filename", "invalid_section", stop_on_error=False)