From 630fc6e88a77c0eab1bc8dd9d4f0c9df3a58508d Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Fri, 15 Dec 2017 14:18:11 +0000 Subject: [PATCH] No warn on old plugin conf format not in use Use a simple object to ensure matching against any default value is possible to distinguish been not being set by a user versus use of a negative default value passed. This prevents triggering an incorrect notification to the user when the code is querying for plugin options that could override the default behaviour and the compatibility code attempts to look up an non existing section. It would fail to distinguish between no section present and current value to return matching the default value suggested. Change-Id: I5597c2628ccb5a4282a97a4ce5d3bbe41bd9eebb --- jenkins_jobs/config.py | 14 +++++++++----- tests/cmd/fixtures/plugin_warning.ini | 5 +++++ tests/cmd/test_config.py | 27 +++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 tests/cmd/fixtures/plugin_warning.ini diff --git a/jenkins_jobs/config.py b/jenkins_jobs/config.py index 58016051f..5abadae63 100644 --- a/jenkins_jobs/config.py +++ b/jenkins_jobs/config.py @@ -51,6 +51,7 @@ query_plugins_info=True CONFIG_REQUIRED_MESSAGE = ("A valid configuration file is required. " "No configuration file passed.") +_NOTSET = object() class JJBConfig(object): @@ -354,10 +355,13 @@ class JJBConfig(object): # plugin configuration format in their config. This code should be # removed in future versions of JJB after 2.0. if value is default: - value = self.get_module_config(plugin, key, default) - logger.warning( - "Defining plugin configuration using [" + plugin + "] is" - " deprecated. The recommended way to define plugins now is by" - " configuring [plugin \"" + plugin + "\"]") + old_value = self.get_module_config(plugin, key, _NOTSET) + # only log warning if detected a plugin config setting. + if old_value is not _NOTSET: + value = old_value + logger.warning( + "Defining plugin configuration using [" + plugin + "] is " + "deprecated. The recommended way to define plugins now is " + "by configuring [plugin \"" + plugin + "\"]") return value diff --git a/tests/cmd/fixtures/plugin_warning.ini b/tests/cmd/fixtures/plugin_warning.ini new file mode 100644 index 000000000..58ae89091 --- /dev/null +++ b/tests/cmd/fixtures/plugin_warning.ini @@ -0,0 +1,5 @@ +[old_plugin] +setting = some value + +[plugin "new_plugin"] +setting = some value diff --git a/tests/cmd/test_config.py b/tests/cmd/test_config.py index fd6397d6f..d1cae5874 100644 --- a/tests/cmd/test_config.py +++ b/tests/cmd/test_config.py @@ -77,6 +77,33 @@ class TestConfigs(CmdTestsBase): jenkins_jobs = entry.JenkinsJobs(args) self.assertRaises(IOError, jenkins_jobs.execute) + def test_config_old_plugin_format_warning(self): + """ + Run test mode and check that old plugin settings result + in a warning, while ensuring that missing sections do not + trigger the same warning if a default value is provided. + """ + args = ['--conf', + os.path.join(self.fixtures_path, 'plugin_warning.ini'), + 'test', 'foo'] + jenkins_jobs = entry.JenkinsJobs(args) + jenkins_jobs.jjb_config.get_plugin_config( + 'old_plugin', 'setting', True) + jenkins_jobs.jjb_config.get_plugin_config( + 'old_plugin_no_conf', 'setting', True) + jenkins_jobs.jjb_config.get_plugin_config( + 'new_plugin', 'setting') + self.assertIn( + 'Defining plugin configuration using [old_plugin] is deprecated', + self.logger.output) + self.assertNotIn( + 'Defining plugin configuration using [old_plugin_no_conf] is ' + 'deprecated', + self.logger.output) + self.assertNotIn( + 'Defining plugin configuration using [new_plugin] is deprecated', + self.logger.output) + def test_config_options_not_replaced_by_cli_defaults(self): """ Run test mode and check config settings from conf file retained