From 439a32a431cae7aa186ff128b5b71d912c1c2755 Mon Sep 17 00:00:00 2001 From: Zdenek Kraus Date: Fri, 10 Apr 2026 14:30:19 +0200 Subject: [PATCH 1/8] FIX(config) Respect log_level from settings.yaml and suppress premature debug output Settings.yaml log_level was ignored because config was loaded after --log-level argument default was set. Additionally, debug messages from config loading appeared before the logger was properly configured. Changes: - Load config defaults before creating argparse arguments - Use config file log_level as default instead of hardcoded "INFO" - Remove loguru default handler before parsing args to suppress early debug output - Add log_config_status() to log config info AFTER logger is configured - Remove redundant --log-level from trigger/summary subcommands - Note: --log-level must now come BEFORE subcommand (e.g., rptool --log-level DEBUG write) Fixes: Kuadrant/testsuite-rptool#6 Co-Authored-By: Claude Sonnet 4.5 Signed-off-by: Zdenek Kraus --- src/reportportal/ap.py | 24 ++--- src/reportportal/config.py | 39 +++++++-- src/reportportal/rp_dispatcher.py | 11 ++- tests/unit/test_ap.py | 140 +++++++++++++++++++++++++++++- tests/unit/test_config.py | 110 ++++++++++++++++++----- 5 files changed, 273 insertions(+), 51 deletions(-) diff --git a/src/reportportal/ap.py b/src/reportportal/ap.py index 3d65980..9ce5648 100644 --- a/src/reportportal/ap.py +++ b/src/reportportal/ap.py @@ -45,6 +45,10 @@ def create_main_parser() -> argparse.ArgumentParser: except PackageNotFoundError: pkg_version = 'unknown (not installed)' + # Get configuration defaults (config file + env vars + built-in defaults) + # Must be loaded before creating arguments that use these defaults + defaults = _get_config_defaults() + parser = argparse.ArgumentParser( prog='rptool', description='Unified command-line interface for ReportPortal tools', @@ -61,8 +65,8 @@ def create_main_parser() -> argparse.ArgumentParser: parser.add_argument( "--log-level", choices=["DEBUG", "INFO", "WARNING", "ERROR"], - default="INFO", - help="Set the logging level (default: INFO)" + default=defaults["log_level"], + help="Set the logging level (default: from config or INFO)" ) # Create subparsers for each command @@ -74,9 +78,6 @@ def create_main_parser() -> argparse.ArgumentParser: required=True ) - # Get configuration defaults (config file + env vars + built-in defaults) - defaults = _get_config_defaults() - # Adding subparsers' arguments subparsers_hanlers = [ _add_write_arguments, @@ -235,13 +236,6 @@ def _add_trigger_arguments(subparsers: argparse.ArgumentParser, defaults: dict) ) _add_common_rp_args(parser, defaults) - parser.add_argument( - "--log-level", - choices=["DEBUG", "INFO", "WARNING", "ERROR"], - default=defaults.get("log_level", "INFO"), - help="Set the logging level (default: INFO)" - ) - def _add_summary_arguments(subparsers: argparse.ArgumentParser, defaults: dict) -> None: """Add arguments for summary command.""" @@ -255,12 +249,6 @@ def _add_summary_arguments(subparsers: argparse.ArgumentParser, defaults: dict) _add_common_rp_args(parser, defaults) - parser.add_argument( - "--log-level", - choices=["DEBUG", "INFO", "WARNING", "ERROR"], - default=defaults.get("log_level", "INFO"), - help="Set the logging level (default: INFO)" - ) parser.add_argument( "--attribute", action="append", diff --git a/src/reportportal/config.py b/src/reportportal/config.py index c6a35fa..0524aaa 100644 --- a/src/reportportal/config.py +++ b/src/reportportal/config.py @@ -35,6 +35,9 @@ def load_config_file() -> Dict[str, Any]: """ Load configuration from YAML file. + Note: Debug messages during loading are suppressed (logger not configured yet). + Use log_config_status() after logger is configured to see config loading status. + Returns: Dictionary with configuration values, empty dict if file doesn't exist or can't be loaded @@ -43,19 +46,20 @@ def load_config_file() -> Dict[str, Any]: config_file = get_config_file_path() if not config_file.exists(): - logger.debug(f"Config file not found: {config_file}") + # Debug message suppressed - will be logged by log_config_status() if needed return {} try: with open(config_file, 'r') as f: config = yaml.safe_load(f) if config is None: - logger.debug(f"Config file is empty: {config_file}") + # Debug message suppressed - will be logged by log_config_status() if needed return {} - logger.debug(f"Loaded config from: {config_file}") + # Debug message suppressed - will be logged by log_config_status() if needed return config except Exception as e: - logger.warning(f"Failed to load config file {config_file}: {e}") + # Warning should be shown, but logger may not be configured yet + # Will be logged by log_config_status() if needed return {} @@ -162,6 +166,31 @@ def get_effective_defaults() -> Dict[str, Any]: # Inject REQUESTS_CA_BUNDLE into environment if configured but not already set if merged.get("requests_ca_bundle") and not os.environ.get("REQUESTS_CA_BUNDLE"): os.environ["REQUESTS_CA_BUNDLE"] = merged["requests_ca_bundle"] - logger.debug(f"Set REQUESTS_CA_BUNDLE from config: {merged['requests_ca_bundle']}") + # Debug message suppressed - logger not configured yet return merged + + +def log_config_status() -> None: + """ + Log the configuration file loading status at DEBUG level. + + This should be called AFTER the logger is properly configured with the + desired log level. It will show users (when running with DEBUG) whether + their config file was found and loaded. + """ + config_file = get_config_file_path() + + if not config_file.exists(): + logger.debug(f"Config file not found: {config_file}") + return + + try: + with open(config_file, 'r') as f: + config = yaml.safe_load(f) + if config is None: + logger.debug(f"Config file exists but is empty: {config_file}") + else: + logger.debug(f"Loaded config from: {config_file} (keys: {list(config.keys())})") + except Exception as e: + logger.warning(f"Failed to load config file {config_file}: {e}") diff --git a/src/reportportal/rp_dispatcher.py b/src/reportportal/rp_dispatcher.py index 3ab9ebd..7ffc4f2 100644 --- a/src/reportportal/rp_dispatcher.py +++ b/src/reportportal/rp_dispatcher.py @@ -18,6 +18,7 @@ SHTAB_AVAILABLE = False from . import ap +from .config import log_config_status from .writer import RPWriter from .rp_query import run_query from .rp_trigger import run_auto_trigger @@ -164,6 +165,10 @@ def main(argv: Optional[List[str]] = None) -> int: Returns: Exit code (0 for success, 1 for error) """ + # Remove default loguru handler immediately to prevent premature debug messages + # (e.g., during config file loading before log level is determined) + logger.remove() + parser = ap.create_main_parser() # Parse arguments @@ -172,10 +177,12 @@ def main(argv: Optional[List[str]] = None) -> int: except SystemExit as e: return e.code if e.code is not None else 1 - # setup logging handlers - logger.remove() # remove default one + # Setup logging handler with configured log level logger.add(sink=sys.stderr, level=args.log_level) + # Log config file status now that logger is properly configured + log_config_status() + # Dispatch to appropriate command handler command_handlers = { 'write': run_write_command, diff --git a/tests/unit/test_ap.py b/tests/unit/test_ap.py index fa34aa9..cba9834 100644 --- a/tests/unit/test_ap.py +++ b/tests/unit/test_ap.py @@ -4,6 +4,7 @@ import pytest import os +from pathlib import Path from unittest.mock import patch from reportportal.ap import ( @@ -321,11 +322,11 @@ def test_release_parser_all_options(self): parser = create_main_parser() args = parser.parse_args([ + '--log-level', 'DEBUG', 'summary', '--rp-project', 'test_project', '--rp-url', 'https://test.reportportal.com', '--rp-token', 'test_key', - '--log-level', 'DEBUG', '--attribute', 'kuadrant:v1.3.1', '--attribute', 'platform:aws', '--days', '7', @@ -373,7 +374,7 @@ def test_release_parser_log_levels(self): parser = create_main_parser() for level in ['DEBUG', 'INFO', 'WARNING', 'ERROR']: - args = parser.parse_args(['summary', '--attribute', 'kuadrant:v1.3.1', '--log-level', level]) + args = parser.parse_args(['--log-level', level, 'summary', '--attribute', 'kuadrant:v1.3.1']) assert args.log_level == level def test_release_parser_days_parameter(self): @@ -535,4 +536,137 @@ def test_multi_criteria_filtering(self): assert 'platform:gcp' in args.attribute assert 'component:controller' in args.attribute assert 'env:staging' in args.attribute - assert args.days == 7 \ No newline at end of file + assert args.days == 7 + + +@pytest.mark.unit +class TestLogLevelConfig: + """Test log level configuration (Issue #6).""" + + def test_log_level_from_config_file(self): + """Test that log_level from config file is used as default.""" + with patch.dict(os.environ, {}, clear=True): + with patch('reportportal.config.load_config_file') as mock_load: + # Simulate config file with DEBUG log level + mock_load.return_value = {'log_level': 'DEBUG'} + parser = create_main_parser() + + # Parse without --log-level argument + args = parser.parse_args(['write', 'test.xml']) + + # Should use config file value + assert args.log_level == 'DEBUG' + + def test_log_level_cli_overrides_config(self): + """Test that CLI --log-level overrides config file.""" + with patch.dict(os.environ, {}, clear=True): + with patch('reportportal.config.load_config_file') as mock_load: + # Config has DEBUG + mock_load.return_value = {'log_level': 'DEBUG'} + parser = create_main_parser() + + # CLI specifies ERROR + args = parser.parse_args(['--log-level', 'ERROR', 'write', 'test.xml']) + + # Should use CLI value + assert args.log_level == 'ERROR' + + def test_log_level_default_when_no_config(self): + """Test that INFO is used when no config is provided.""" + with patch.dict(os.environ, {}, clear=True): + with patch('reportportal.config.load_config_file') as mock_load: + # No log_level in config + mock_load.return_value = {} + parser = create_main_parser() + + # Parse without --log-level argument + args = parser.parse_args(['write', 'test.xml']) + + # Should use built-in default (INFO) + assert args.log_level == 'INFO' + + def test_log_level_works_with_all_commands(self): + """Test that config log_level works for all commands.""" + with patch.dict(os.environ, {}, clear=True): + with patch('reportportal.config.load_config_file') as mock_load: + mock_load.return_value = {'log_level': 'WARNING'} + parser = create_main_parser() + + # Test write command + args = parser.parse_args(['write', 'test.xml']) + assert args.log_level == 'WARNING' + + # Test query command + args = parser.parse_args(['query']) + assert args.log_level == 'WARNING' + + # Test trigger command + args = parser.parse_args(['trigger']) + assert args.log_level == 'WARNING' + + # Test summary command + args = parser.parse_args(['summary', '--attribute', 'test:v1']) + assert args.log_level == 'WARNING' + + def test_no_premature_debug_messages_during_config_load(self): + """Test that debug messages during config loading are suppressed until log level is set.""" + import io + from unittest.mock import patch + from reportportal.rp_dispatcher import main + + # Capture stderr to check for debug messages + captured_stderr = io.StringIO() + + with patch.dict(os.environ, {}, clear=True): + with patch('reportportal.config.load_config_file') as mock_load: + # Simulate config file exists and has values + mock_load.return_value = {'log_level': 'INFO', 'rp_url': 'http://test.com'} + + # Redirect stderr + with patch('sys.stderr', captured_stderr): + try: + # Run with INFO level (default from config) + # This will fail because we don't have valid args, but we just want to check logging + main(['write', 'test.xml']) + except SystemExit: + pass + + # Check that no "Loaded config from" or "Config file" debug messages appear + stderr_output = captured_stderr.getvalue() + assert "Loaded config from" not in stderr_output, \ + "Debug message 'Loaded config from' should not appear with INFO log level" + assert "Config file not found" not in stderr_output, \ + "Debug message 'Config file not found' should not appear with INFO log level" + + def test_config_status_logged_with_debug_level(self): + """Test that config file status IS logged when using DEBUG level.""" + import io + from unittest.mock import patch, mock_open + from reportportal.rp_dispatcher import main + + captured_stderr = io.StringIO() + + with patch.dict(os.environ, {}, clear=True): + # Mock config file exists + with patch('reportportal.config.get_config_file_path') as mock_path: + mock_config_path = '/mock/.config/rptool/settings.yaml' + mock_path.return_value = Path(mock_config_path) + + # Mock Path.exists to return True + with patch('pathlib.Path.exists', return_value=True): + # Mock file open to return config content + mock_config_content = "log_level: DEBUG\nrp_url: http://test.com\n" + with patch('builtins.open', mock_open(read_data=mock_config_content)): + # Redirect stderr + with patch('sys.stderr', captured_stderr): + try: + # Run with explicit DEBUG level + main(['--log-level', 'DEBUG', 'write', 'test.xml']) + except SystemExit: + pass + + # Check that config status message appears with DEBUG level + stderr_output = captured_stderr.getvalue() + # Should contain config file path in debug output + assert "Config file" in stderr_output or "config" in stderr_output.lower(), \ + "Config file status should be logged with DEBUG log level" \ No newline at end of file diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 33be313..2e9195c 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -14,6 +14,7 @@ get_config_defaults, merge_with_env_vars, get_effective_defaults, + log_config_status, ) @@ -28,24 +29,25 @@ def test_get_config_file_path_with_platformdirs(self): assert path.name == "settings.yaml" assert "rptool" in str(path) + class TestLoadConfigFile: """Test YAML config file loading.""" def test_load_config_file_not_exists(self): """Test loading when config file doesn't exist.""" - with patch('reportportal.config.get_config_file_path') as mock_path: + with patch("reportportal.config.get_config_file_path") as mock_path: mock_path.return_value = Path("/nonexistent/settings.yaml") config = load_config_file() assert config == {} def test_load_config_file_empty(self): """Test loading empty config file.""" - with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml', delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: f.write("") temp_path = f.name try: - with patch('reportportal.config.get_config_file_path') as mock_path: + with patch("reportportal.config.get_config_file_path") as mock_path: mock_path.return_value = Path(temp_path) config = load_config_file() assert config == {} @@ -54,8 +56,9 @@ def test_load_config_file_empty(self): def test_load_config_file_valid(self): """Test loading valid config file.""" - with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml', delete=False) as f: - f.write(""" + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: + f.write( + """ rp_url: "https://test.com" rp_project: "test_project" rp_token: "test_token" @@ -63,11 +66,12 @@ def test_load_config_file_valid(self): launch_name: "Test Launch" launch_description: "Test Description" log_level: "DEBUG" -""") +""" + ) temp_path = f.name try: - with patch('reportportal.config.get_config_file_path') as mock_path: + with patch("reportportal.config.get_config_file_path") as mock_path: mock_path.return_value = Path(temp_path) config = load_config_file() assert config["rp_url"] == "https://test.com" @@ -82,12 +86,12 @@ def test_load_config_file_valid(self): def test_load_config_file_invalid_yaml(self): """Test loading invalid YAML file.""" - with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml', delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: f.write("invalid: yaml: content:") temp_path = f.name try: - with patch('reportportal.config.get_config_file_path') as mock_path: + with patch("reportportal.config.get_config_file_path") as mock_path: mock_path.return_value = Path(temp_path) config = load_config_file() # Should return empty dict on error @@ -101,7 +105,7 @@ class TestGetConfigDefaults: def test_get_config_defaults_builtin(self): """Test that built-in defaults are returned when no config file exists.""" - with patch('reportportal.config.load_config_file') as mock_load: + with patch("reportportal.config.load_config_file") as mock_load: mock_load.return_value = {} defaults = get_config_defaults() @@ -115,14 +119,14 @@ def test_get_config_defaults_builtin(self): def test_get_config_defaults_from_file(self): """Test that config file values override built-in defaults.""" - with patch('reportportal.config.load_config_file') as mock_load: + with patch("reportportal.config.load_config_file") as mock_load: mock_load.return_value = { "rp_url": "https://config-file.com", "rp_project": "file_project", "trigger_auto_analysis": True, "launch_name": "Test Launch", "launch_description": "Test Description", - "log_level": "DEBUG" + "log_level": "DEBUG", } defaults = get_config_defaults() @@ -147,7 +151,7 @@ def test_merge_with_env_vars_empty_env(self): "trigger_auto_analysis": False, "launch_name": None, "launch_description": "", - "log_level": "INFO" + "log_level": "INFO", } with patch.dict(os.environ, {}, clear=True): @@ -163,7 +167,7 @@ def test_merge_with_env_vars_override(self): "trigger_auto_analysis": False, "launch_name": "Config Launch", "launch_description": "Config Description", - "log_level": "INFO" + "log_level": "INFO", } env_vars = { @@ -172,7 +176,7 @@ def test_merge_with_env_vars_override(self): "TRIGGER_AUTO_ANALYSIS": "true", "RP_LAUNCH_NAME": "Env Launch", "RP_LAUNCH_DESCRIPTION": "Env Description", - "LOG_LEVEL": "DEBUG" + "LOG_LEVEL": "DEBUG", } with patch.dict(os.environ, env_vars, clear=True): @@ -198,7 +202,7 @@ def test_trigger_auto_analysis_true_values(self): "trigger_auto_analysis": False, "launch_name": None, "launch_description": "", - "log_level": "INFO" + "log_level": "INFO", } true_values = ["true", "True", "TRUE", "1", "yes", "YES", "on", "ON"] @@ -206,7 +210,9 @@ def test_trigger_auto_analysis_true_values(self): for val in true_values: with patch.dict(os.environ, {"TRIGGER_AUTO_ANALYSIS": val}, clear=True): merged = merge_with_env_vars(config) - assert merged["trigger_auto_analysis"] is True, f"Failed for value: {val}" + assert ( + merged["trigger_auto_analysis"] is True + ), f"Failed for value: {val}" def test_trigger_auto_analysis_false_values(self): """Test that various false string values are converted to boolean False.""" @@ -217,7 +223,7 @@ def test_trigger_auto_analysis_false_values(self): "trigger_auto_analysis": True, "launch_name": None, "launch_description": "", - "log_level": "INFO" + "log_level": "INFO", } false_values = ["false", "False", "FALSE", "0", "no", "NO", "off", "OFF"] @@ -225,7 +231,9 @@ def test_trigger_auto_analysis_false_values(self): for val in false_values: with patch.dict(os.environ, {"TRIGGER_AUTO_ANALYSIS": val}, clear=True): merged = merge_with_env_vars(config) - assert merged["trigger_auto_analysis"] is False, f"Failed for value: {val}" + assert ( + merged["trigger_auto_analysis"] is False + ), f"Failed for value: {val}" def test_trigger_auto_analysis_no_env(self): """Test that config value is preserved when env var is not set.""" @@ -236,7 +244,7 @@ def test_trigger_auto_analysis_no_env(self): "trigger_auto_analysis": True, "launch_name": None, "launch_description": "", - "log_level": "INFO" + "log_level": "INFO", } with patch.dict(os.environ, {}, clear=True): @@ -250,13 +258,13 @@ class TestGetEffectiveDefaults: def test_get_effective_defaults_priority(self): """Test that full priority chain works correctly.""" # Mock config file - with patch('reportportal.config.load_config_file') as mock_load: + with patch("reportportal.config.load_config_file") as mock_load: mock_load.return_value = { "rp_url": "https://config.com", "rp_project": "config_project", "trigger_auto_analysis": False, "launch_name": "Config Launch", - "log_level": "INFO" + "log_level": "INFO", } # Set env vars @@ -264,7 +272,7 @@ def test_get_effective_defaults_priority(self): "RP_URL": "https://env.com", "TRIGGER_AUTO_ANALYSIS": "1", "RP_LAUNCH_DESCRIPTION": "Env Description", - "LOG_LEVEL": "DEBUG" + "LOG_LEVEL": "DEBUG", } with patch.dict(os.environ, env_vars, clear=True): @@ -282,3 +290,59 @@ def test_get_effective_defaults_priority(self): # Built-in default when neither config nor ENV assert defaults["rp_token"] is None + + +class TestLogConfigStatus: + """Test log_config_status function.""" + + def test_log_config_status_file_not_found(self): + """Test log_config_status when config file doesn't exist.""" + from loguru import logger + import io + + # Capture loguru output + log_output = io.StringIO() + logger.remove() + logger.add(log_output, level="DEBUG", format="{message}") + + with patch("reportportal.config.get_config_file_path") as mock_path: + mock_path.return_value = Path("/nonexistent/config.yaml") + + with patch("pathlib.Path.exists", return_value=False): + log_config_status() + + # Check that "not found" message was logged + output = log_output.getvalue() + assert "not found" in output.lower() + assert "/nonexistent/config.yaml" in output + + def test_log_config_status_empty_file(self): + """Test log_config_status when config file is empty.""" + from loguru import logger + import io + + # Capture loguru output + log_output = io.StringIO() + logger.remove() + logger.add(log_output, level="DEBUG", format="{message}") + + with patch("reportportal.config.get_config_file_path") as mock_path: + mock_path.return_value = Path("/mock/.config/rptool/settings.yaml") + + with patch("pathlib.Path.exists", return_value=True): + with patch( + "builtins.open", + MagicMock( + return_value=MagicMock( + __enter__=MagicMock( + return_value=MagicMock(read=MagicMock(return_value="")) + ), + __exit__=MagicMock(), + ) + ), + ): + log_config_status() + + # Check that "empty" message was logged + output = log_output.getvalue() + assert "empty" in output.lower() From 6941978376ddc91bb069c0f58d9369c058c2f250 Mon Sep 17 00:00:00 2001 From: Zdenek Kraus Date: Thu, 16 Apr 2026 15:51:21 +0200 Subject: [PATCH 2/8] fixup! FIX(config) Respect log_level from settings.yaml and suppress premature debug output Signed-off-by: Zdenek Kraus --- src/reportportal/ap.py | 16 +++++--- src/reportportal/rp_dispatcher.py | 14 ++++--- tests/unit/test_ap.py | 67 +++++++++++++++---------------- 3 files changed, 53 insertions(+), 44 deletions(-) diff --git a/src/reportportal/ap.py b/src/reportportal/ap.py index 9ce5648..93b208d 100644 --- a/src/reportportal/ap.py +++ b/src/reportportal/ap.py @@ -62,10 +62,16 @@ def create_main_parser() -> argparse.ArgumentParser: version=f'rptool {pkg_version}' ) + # Validation of config file log_level + valid_log_levels = {"DEBUG", "INFO", "WARNING", "ERROR"} + configured_log_level = str(defaults.get("log_level", "INFO")).upper() + if configured_log_level not in valid_log_levels: + raise ValueError(f'Invalid log level in config: {configured_log_level}. Must be one of {valid_log_levels}') + parser.add_argument( "--log-level", choices=["DEBUG", "INFO", "WARNING", "ERROR"], - default=defaults["log_level"], + default=configured_log_level, help="Set the logging level (default: from config or INFO)" ) @@ -125,14 +131,14 @@ def _add_write_arguments(subparsers: argparse.ArgumentParser, defaults: dict) -> _add_common_rp_args(parser, defaults) parser.add_argument( - "--launch-name", + "--launch-name", help="Override Launch name that will be reported, otherwise filename will be used", default=defaults['rp_launch_name'] ) parser.add_argument( - "--launch-description", + "--launch-description", help="Custom head section to launch description, passthrough description will be added from the junit if available", - # The empty string from defaults is necessary to enable additional description to be added on .finish_launch() + # The empty string from defaults is necessary to enable additional description to be added on .finish_launch() default=defaults['rp_launch_description'], ) parser.add_argument( @@ -148,7 +154,7 @@ def _add_write_arguments(subparsers: argparse.ArgumentParser, defaults: dict) -> default=False ) parser.add_argument("junits", nargs='+', help="path to all junit results, multiple files will be reportes as one launch") - + def _add_query_arguments(subparsers: argparse.ArgumentParser, defaults: dict) -> None: """Add arguments for query command.""" diff --git a/src/reportportal/rp_dispatcher.py b/src/reportportal/rp_dispatcher.py index 7ffc4f2..82c105a 100644 --- a/src/reportportal/rp_dispatcher.py +++ b/src/reportportal/rp_dispatcher.py @@ -18,7 +18,6 @@ SHTAB_AVAILABLE = False from . import ap -from .config import log_config_status from .writer import RPWriter from .rp_query import run_query from .rp_trigger import run_auto_trigger @@ -168,8 +167,15 @@ def main(argv: Optional[List[str]] = None) -> int: # Remove default loguru handler immediately to prevent premature debug messages # (e.g., during config file loading before log level is determined) logger.remove() + # Setup intermittent WARNING logger for any configuration logs + logger.add(sink=sys.stderr, level='WARNING') + + try: + parser = ap.create_main_parser() + except ValueError as e: + logger.error('Improper configuration {}', str(e)) + return 1 - parser = ap.create_main_parser() # Parse arguments try: @@ -178,11 +184,9 @@ def main(argv: Optional[List[str]] = None) -> int: return e.code if e.code is not None else 1 # Setup logging handler with configured log level + logger.remove() logger.add(sink=sys.stderr, level=args.log_level) - # Log config file status now that logger is properly configured - log_config_status() - # Dispatch to appropriate command handler command_handlers = { 'write': run_write_command, diff --git a/tests/unit/test_ap.py b/tests/unit/test_ap.py index cba9834..99551c5 100644 --- a/tests/unit/test_ap.py +++ b/tests/unit/test_ap.py @@ -608,6 +608,38 @@ def test_log_level_works_with_all_commands(self): args = parser.parse_args(['summary', '--attribute', 'test:v1']) assert args.log_level == 'WARNING' + def test_log_level_invalid_in_config(self): + """Test that invalid log level in config raises ValueError.""" + with patch.dict(os.environ, {}, clear=True): + with patch('reportportal.config.load_config_file') as mock_load: + # Simulate config file with invalid log level + mock_load.return_value = {'log_level': 'VERBOSE'} + + # Creating parser should raise ValueError + with pytest.raises(ValueError) as exc_info: + create_main_parser() + + # Check error message contains the invalid value + error_msg = str(exc_info.value) + assert 'Invalid log level in config' in error_msg + assert 'VERBOSE' in error_msg + assert 'Must be one of' in error_msg + + def test_log_level_invalid_in_config_case_insensitive(self): + """Test that invalid log level works with case normalization.""" + with patch.dict(os.environ, {}, clear=True): + with patch('reportportal.config.load_config_file') as mock_load: + # Lowercase 'trace' should also be rejected + mock_load.return_value = {'log_level': 'trace'} + + with pytest.raises(ValueError) as exc_info: + create_main_parser() + + error_msg = str(exc_info.value) + assert 'Invalid log level in config' in error_msg + # Should show uppercase version in error + assert 'TRACE' in error_msg + def test_no_premature_debug_messages_during_config_load(self): """Test that debug messages during config loading are suppressed until log level is set.""" import io @@ -636,37 +668,4 @@ def test_no_premature_debug_messages_during_config_load(self): assert "Loaded config from" not in stderr_output, \ "Debug message 'Loaded config from' should not appear with INFO log level" assert "Config file not found" not in stderr_output, \ - "Debug message 'Config file not found' should not appear with INFO log level" - - def test_config_status_logged_with_debug_level(self): - """Test that config file status IS logged when using DEBUG level.""" - import io - from unittest.mock import patch, mock_open - from reportportal.rp_dispatcher import main - - captured_stderr = io.StringIO() - - with patch.dict(os.environ, {}, clear=True): - # Mock config file exists - with patch('reportportal.config.get_config_file_path') as mock_path: - mock_config_path = '/mock/.config/rptool/settings.yaml' - mock_path.return_value = Path(mock_config_path) - - # Mock Path.exists to return True - with patch('pathlib.Path.exists', return_value=True): - # Mock file open to return config content - mock_config_content = "log_level: DEBUG\nrp_url: http://test.com\n" - with patch('builtins.open', mock_open(read_data=mock_config_content)): - # Redirect stderr - with patch('sys.stderr', captured_stderr): - try: - # Run with explicit DEBUG level - main(['--log-level', 'DEBUG', 'write', 'test.xml']) - except SystemExit: - pass - - # Check that config status message appears with DEBUG level - stderr_output = captured_stderr.getvalue() - # Should contain config file path in debug output - assert "Config file" in stderr_output or "config" in stderr_output.lower(), \ - "Config file status should be logged with DEBUG log level" \ No newline at end of file + "Debug message 'Config file not found' should not appear with INFO log level" \ No newline at end of file From 1390a0c5bf85a8d102137dce0457bf9de53599e0 Mon Sep 17 00:00:00 2001 From: Zdenek Kraus Date: Thu, 16 Apr 2026 15:54:49 +0200 Subject: [PATCH 3/8] fixup! fixup! FIX(config) Respect log_level from settings.yaml and suppress premature debug output Signed-off-by: Zdenek Kraus --- src/reportportal/config.py | 24 ---------------- tests/unit/test_config.py | 56 -------------------------------------- 2 files changed, 80 deletions(-) diff --git a/src/reportportal/config.py b/src/reportportal/config.py index 0524aaa..20a606f 100644 --- a/src/reportportal/config.py +++ b/src/reportportal/config.py @@ -170,27 +170,3 @@ def get_effective_defaults() -> Dict[str, Any]: return merged - -def log_config_status() -> None: - """ - Log the configuration file loading status at DEBUG level. - - This should be called AFTER the logger is properly configured with the - desired log level. It will show users (when running with DEBUG) whether - their config file was found and loaded. - """ - config_file = get_config_file_path() - - if not config_file.exists(): - logger.debug(f"Config file not found: {config_file}") - return - - try: - with open(config_file, 'r') as f: - config = yaml.safe_load(f) - if config is None: - logger.debug(f"Config file exists but is empty: {config_file}") - else: - logger.debug(f"Loaded config from: {config_file} (keys: {list(config.keys())})") - except Exception as e: - logger.warning(f"Failed to load config file {config_file}: {e}") diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 2e9195c..2679fea 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -14,7 +14,6 @@ get_config_defaults, merge_with_env_vars, get_effective_defaults, - log_config_status, ) @@ -291,58 +290,3 @@ def test_get_effective_defaults_priority(self): # Built-in default when neither config nor ENV assert defaults["rp_token"] is None - -class TestLogConfigStatus: - """Test log_config_status function.""" - - def test_log_config_status_file_not_found(self): - """Test log_config_status when config file doesn't exist.""" - from loguru import logger - import io - - # Capture loguru output - log_output = io.StringIO() - logger.remove() - logger.add(log_output, level="DEBUG", format="{message}") - - with patch("reportportal.config.get_config_file_path") as mock_path: - mock_path.return_value = Path("/nonexistent/config.yaml") - - with patch("pathlib.Path.exists", return_value=False): - log_config_status() - - # Check that "not found" message was logged - output = log_output.getvalue() - assert "not found" in output.lower() - assert "/nonexistent/config.yaml" in output - - def test_log_config_status_empty_file(self): - """Test log_config_status when config file is empty.""" - from loguru import logger - import io - - # Capture loguru output - log_output = io.StringIO() - logger.remove() - logger.add(log_output, level="DEBUG", format="{message}") - - with patch("reportportal.config.get_config_file_path") as mock_path: - mock_path.return_value = Path("/mock/.config/rptool/settings.yaml") - - with patch("pathlib.Path.exists", return_value=True): - with patch( - "builtins.open", - MagicMock( - return_value=MagicMock( - __enter__=MagicMock( - return_value=MagicMock(read=MagicMock(return_value="")) - ), - __exit__=MagicMock(), - ) - ), - ): - log_config_status() - - # Check that "empty" message was logged - output = log_output.getvalue() - assert "empty" in output.lower() From 2e67b1a25a400b4e92c12147fdf6f53b0bc5f1db Mon Sep 17 00:00:00 2001 From: Zdenek Kraus Date: Thu, 16 Apr 2026 15:57:18 +0200 Subject: [PATCH 4/8] fixup! fixup! fixup! FIX(config) Respect log_level from settings.yaml and suppress premature debug output Signed-off-by: Zdenek Kraus --- tests/unit/test_ap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_ap.py b/tests/unit/test_ap.py index 99551c5..38d7ab2 100644 --- a/tests/unit/test_ap.py +++ b/tests/unit/test_ap.py @@ -668,4 +668,4 @@ def test_no_premature_debug_messages_during_config_load(self): assert "Loaded config from" not in stderr_output, \ "Debug message 'Loaded config from' should not appear with INFO log level" assert "Config file not found" not in stderr_output, \ - "Debug message 'Config file not found' should not appear with INFO log level" \ No newline at end of file + "Debug message 'Config file not found' should not appear with INFO log level" From f6d75af0e9ec3d83cac5318d8fa450a94f416040 Mon Sep 17 00:00:00 2001 From: Zdenek Kraus Date: Fri, 17 Apr 2026 13:14:30 +0200 Subject: [PATCH 5/8] fixup! fixup! fixup! fixup! FIX(config) Respect log_level from settings.yaml and suppress premature debug output Signed-off-by: Zdenek Kraus --- src/reportportal/config.py | 15 ++++++--------- tests/unit/test_config.py | 12 ++++++++---- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/reportportal/config.py b/src/reportportal/config.py index 20a606f..3a703f8 100644 --- a/src/reportportal/config.py +++ b/src/reportportal/config.py @@ -35,9 +35,6 @@ def load_config_file() -> Dict[str, Any]: """ Load configuration from YAML file. - Note: Debug messages during loading are suppressed (logger not configured yet). - Use log_config_status() after logger is configured to see config loading status. - Returns: Dictionary with configuration values, empty dict if file doesn't exist or can't be loaded @@ -46,21 +43,21 @@ def load_config_file() -> Dict[str, Any]: config_file = get_config_file_path() if not config_file.exists(): - # Debug message suppressed - will be logged by log_config_status() if needed + logger.debug("No config file present, using defaults") return {} try: with open(config_file, 'r') as f: config = yaml.safe_load(f) if config is None: - # Debug message suppressed - will be logged by log_config_status() if needed + logger.debug("Config file empty") return {} - # Debug message suppressed - will be logged by log_config_status() if needed + logger.debug("Config file loaded successfully") return config except Exception as e: - # Warning should be shown, but logger may not be configured yet - # Will be logged by log_config_status() if needed - return {} + logger.error("Error reading config file {} {}", config_file, e) + # need to raise ValueError to indicate critical problem + raise ValueError("Error reading config file {} {}", config_file, e) def get_config_defaults() -> Dict[str, Any]: diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 2679fea..ac1a1b2 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -84,7 +84,7 @@ def test_load_config_file_valid(self): os.unlink(temp_path) def test_load_config_file_invalid_yaml(self): - """Test loading invalid YAML file.""" + """Test loading invalid YAML file raises ValueError.""" with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: f.write("invalid: yaml: content:") temp_path = f.name @@ -92,9 +92,13 @@ def test_load_config_file_invalid_yaml(self): try: with patch("reportportal.config.get_config_file_path") as mock_path: mock_path.return_value = Path(temp_path) - config = load_config_file() - # Should return empty dict on error - assert config == {} + # Should raise ValueError on error + with pytest.raises(ValueError) as exc_info: + load_config_file() + + # Check error message contains file path + error_msg = str(exc_info.value) + assert "Error reading config file" in error_msg finally: os.unlink(temp_path) From 8a10fbf4ec76820749f5b18a9790c0316fcfcac7 Mon Sep 17 00:00:00 2001 From: Zdenek Kraus Date: Fri, 17 Apr 2026 13:28:33 +0200 Subject: [PATCH 6/8] fixup! fixup! fixup! fixup! fixup! FIX(config) Respect log_level from settings.yaml and suppress premature debug output Signed-off-by: Zdenek Kraus --- src/reportportal/config.py | 2 +- src/reportportal/rp_dispatcher.py | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/reportportal/config.py b/src/reportportal/config.py index 3a703f8..6fec967 100644 --- a/src/reportportal/config.py +++ b/src/reportportal/config.py @@ -52,7 +52,7 @@ def load_config_file() -> Dict[str, Any]: if config is None: logger.debug("Config file empty") return {} - logger.debug("Config file loaded successfully") + logger.info("Config file loaded successfully") return config except Exception as e: logger.error("Error reading config file {} {}", config_file, e) diff --git a/src/reportportal/rp_dispatcher.py b/src/reportportal/rp_dispatcher.py index 82c105a..d598b24 100644 --- a/src/reportportal/rp_dispatcher.py +++ b/src/reportportal/rp_dispatcher.py @@ -164,11 +164,13 @@ def main(argv: Optional[List[str]] = None) -> int: Returns: Exit code (0 for success, 1 for error) """ - # Remove default loguru handler immediately to prevent premature debug messages - # (e.g., during config file loading before log level is determined) - logger.remove() - # Setup intermittent WARNING logger for any configuration logs - logger.add(sink=sys.stderr, level='WARNING') + # only if not predefined LOG LEVEL as env variable + if not os.environ.get('LOG_LEVEL'): + # Remove default loguru handler immediately to prevent premature debug messages + # (e.g., during config file loading before log level is determined) + logger.remove() + # Setup intermittent WARNING logger for any configuration logs + logger.add(sink=sys.stderr, level='WARNING') try: parser = ap.create_main_parser() From 48d58a7f46fd1c681ce28a24840497b42bd09a90 Mon Sep 17 00:00:00 2001 From: Zdenek Kraus Date: Fri, 17 Apr 2026 13:36:21 +0200 Subject: [PATCH 7/8] fixup! fixup! fixup! fixup! fixup! fixup! FIX(config) Respect log_level from settings.yaml and suppress premature debug output Signed-off-by: Zdenek Kraus --- src/reportportal/config.py | 3 +-- tests/unit/test_rp_dispatcher_write.py | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/reportportal/config.py b/src/reportportal/config.py index 6fec967..dcc8ffe 100644 --- a/src/reportportal/config.py +++ b/src/reportportal/config.py @@ -55,9 +55,8 @@ def load_config_file() -> Dict[str, Any]: logger.info("Config file loaded successfully") return config except Exception as e: - logger.error("Error reading config file {} {}", config_file, e) # need to raise ValueError to indicate critical problem - raise ValueError("Error reading config file {} {}", config_file, e) + raise ValueError(f"Error reading config file {config_file} {e}") def get_config_defaults() -> Dict[str, Any]: diff --git a/tests/unit/test_rp_dispatcher_write.py b/tests/unit/test_rp_dispatcher_write.py index 7399aa6..cb75c45 100644 --- a/tests/unit/test_rp_dispatcher_write.py +++ b/tests/unit/test_rp_dispatcher_write.py @@ -197,8 +197,12 @@ def test_run_write_command_with_log_level(self, mock_rpwriter): class TestWriteCommandIntegration: """Test write command integration with main dispatcher.""" - def test_write_command_registered(self): + @patch('reportportal.config.load_config_file') + def test_write_command_registered(self, mock_load_config): """Test that write command is registered in dispatcher.""" + # Mock config file to isolate test from user's environment + mock_load_config.return_value = {} + parser = create_main_parser() # Parse write command help to verify it exists @@ -208,8 +212,12 @@ def test_write_command_registered(self): # --help should exit with code 0 assert exc_info.value.code == 0 - def test_write_command_arguments(self): + @patch('reportportal.config.load_config_file') + def test_write_command_arguments(self, mock_load_config): """Test parsing write command arguments.""" + # Mock config file to isolate test from user's environment + mock_load_config.return_value = {} + parser = create_main_parser() args = parser.parse_args([ '--log-level', 'DEBUG', @@ -231,8 +239,12 @@ def test_write_command_arguments(self): assert args.trigger_auto_analysis is True assert args.junits == ['test.xml'] - def test_write_command_missing_junit_file(self): + @patch('reportportal.config.load_config_file') + def test_write_command_missing_junit_file(self, mock_load_config): """Test that missing JUnit file raises error.""" + # Mock config file to isolate test from user's environment + mock_load_config.return_value = {} + parser = create_main_parser() with pytest.raises(SystemExit) as exc_info: From 9d39636b3317117053b60433d279d0b6fa249224 Mon Sep 17 00:00:00 2001 From: Zdenek Kraus Date: Fri, 17 Apr 2026 14:52:53 +0200 Subject: [PATCH 8/8] fixup! fixup! fixup! fixup! fixup! fixup! fixup! FIX(config) Respect log_level from settings.yaml and suppress premature debug output Signed-off-by: Zdenek Kraus --- src/reportportal/config.py | 7 +++++-- src/reportportal/rp_dispatcher.py | 13 ++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/reportportal/config.py b/src/reportportal/config.py index dcc8ffe..e116094 100644 --- a/src/reportportal/config.py +++ b/src/reportportal/config.py @@ -37,7 +37,10 @@ def load_config_file() -> Dict[str, Any]: Returns: Dictionary with configuration values, empty dict if file doesn't exist - or can't be loaded + or is empty. + + Raises: + ValueError: When config file cannot be parsed properly. """ config_file = get_config_file_path() @@ -162,7 +165,7 @@ def get_effective_defaults() -> Dict[str, Any]: # Inject REQUESTS_CA_BUNDLE into environment if configured but not already set if merged.get("requests_ca_bundle") and not os.environ.get("REQUESTS_CA_BUNDLE"): os.environ["REQUESTS_CA_BUNDLE"] = merged["requests_ca_bundle"] - # Debug message suppressed - logger not configured yet + logger.debug("Set REQUESTS_CA_BUNDLE from config: {}", merged['requests_ca_bundle']) return merged diff --git a/src/reportportal/rp_dispatcher.py b/src/reportportal/rp_dispatcher.py index d598b24..503d8f0 100644 --- a/src/reportportal/rp_dispatcher.py +++ b/src/reportportal/rp_dispatcher.py @@ -164,13 +164,12 @@ def main(argv: Optional[List[str]] = None) -> int: Returns: Exit code (0 for success, 1 for error) """ - # only if not predefined LOG LEVEL as env variable - if not os.environ.get('LOG_LEVEL'): - # Remove default loguru handler immediately to prevent premature debug messages - # (e.g., during config file loading before log level is determined) - logger.remove() - # Setup intermittent WARNING logger for any configuration logs - logger.add(sink=sys.stderr, level='WARNING') + # Remove default loguru handler immediately to prevent premature debug messages + # (e.g., during config file loading before log level is determined) + logger.remove() + # Setup intermittent WARNING logger for any configuration logs + # or set to environmnet variable LOG_LEVEL if defined + logger.add(sink=sys.stderr, level=os.environ.get('LOG_LEVEL', "").upper() or 'WARNING') try: parser = ap.create_main_parser()