From 2021e69bf11f5406682e37fc7f9901aaea027522 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Tue, 25 Jun 2024 12:43:09 -0400 Subject: [PATCH 1/4] Runnable: only set configuration used by runners This reverts commit 94584fc60 ("Runnable: do not ignore the configuration passed from recipes"). That change, because of the fact that every runnable uses "runner.identifier_format", erroneously assumed that every configuration should be passed to the runnables. In fact, it was decided long ago, and implemented after that, that only the configuration that is used by runnables should be set on them. Now, because "runner.indentifier_format" is used by all, this needs to be taken into account. In the near future, we may revisit the idea of "runner.indentifier_format" being handled by the runnable as a configuration item, and be transformed into a primary parameter of the runnables. Reference: https://github.com/avocado-framework/avocado/issues/5964 Signed-off-by: Cleber Rosa --- avocado/core/nrunner/runnable.py | 48 +++++++++++++++++-------------- avocado/plugins/runner_nrunner.py | 2 +- selftests/unit/nrunner.py | 22 ++++++++++++-- 3 files changed, 47 insertions(+), 25 deletions(-) diff --git a/avocado/core/nrunner/runnable.py b/avocado/core/nrunner/runnable.py index a818515721..7f2964d66d 100644 --- a/avocado/core/nrunner/runnable.py +++ b/avocado/core/nrunner/runnable.py @@ -32,6 +32,11 @@ #: Location used for schemas when packaged (as in RPMs) SYSTEM_WIDE_SCHEMA_PATH = "/usr/share/avocado/schemas" +#: Configuration used by all runnables, no matter what its kind. The +#: configuration that a kind uses in addition to this is set in their +#: own class attribute "CONFIGURATION_USED" +CONFIGURATION_USED = ["runner.identifier_format"] + class RunnableRecipeInvalidError(Exception): """Signals that a runnable recipe is not well formed, contains @@ -97,7 +102,7 @@ def __init__(self, kind, uri, *args, config=None, **kwargs): #: attr:`avocado.core.nrunner.runner.BaseRunner.CONFIGURATION_USED` self._config = {} if config is None: - config = self.add_configuration_used(kind, {}) + config = self.filter_runnable_config(kind, {}) self.config = config or {} self.args = args self.tags = kwargs.pop("tags", None) @@ -193,10 +198,10 @@ def config(self, config): configuration_used = Runnable.get_configuration_used_by_kind(self.kind) if not set(configuration_used).issubset(set(config.keys())): LOG.warning( - "The runnable config should have values essential for its runner. " - "In this case, it's missing some of the used configuration. In a " - "future avocado version this will raise a ValueError. Please " - "use avocado.core.nrunner.runnable.Runnable.add_configuration_used " + "The runnable config should have only values " + "essential for its runner. In the next version of " + "avocado, this will raise a Value Error. Please " + "use avocado.core.nrunner.runnable.Runnable.filter_runnable_config " "or avocado.core.nrunner.runnable.Runnable.from_avocado_config" ) self._config = config @@ -297,7 +302,7 @@ def from_avocado_config(cls, kind, uri, *args, config=None, **kwargs): """Creates runnable with only essential config for runner of specific kind.""" if not config: config = {} - config = cls.add_configuration_used(kind, config) + config = cls.filter_runnable_config(kind, config) return cls(kind, uri, *args, config=config, **kwargs) @classmethod @@ -321,30 +326,31 @@ def get_configuration_used_by_kind(cls, kind): return configuration_used @classmethod - def add_configuration_used(cls, kind, config): + def filter_runnable_config(cls, kind, config): """ - Adds essential configuration values for specific runner. + Returns only essential values for specific runner. - It will add missing configuration in the given config, - complementing it with values from config file and avocado default - configuration. + It will use configuration from argument completed by values from + config file and avocado default configuration. :param kind: Kind of runner which should use the configuration. :type kind: str - :param config: Configuration values for runner. If any used configuration - values are missing, the default ones and from config file - will be used. + :param config: Configuration values for runner. If some values will be + missing the default ones and from config file will be + used. :type config: dict - :returns: Config dict, which has existing entries plus values - essential for runner based on - STANDALONE_EXECUTABLE_CONFIG_USED + :returns: Config dict, which has only values essential for runner + based on STANDALONE_EXECUTABLE_CONFIG_USED :rtype: dict """ whole_config = settings.as_dict() - for config_item in cls.get_configuration_used_by_kind(kind): - if config_item not in config: - config[config_item] = whole_config.get(config_item) - return config + filtered_config = {} + config_items = cls.get_configuration_used_by_kind(kind) + CONFIGURATION_USED + for config_item in config_items: + filtered_config[config_item] = config.get( + config_item, whole_config.get(config_item) + ) + return filtered_config def read_dependencies(self, dependencies_dict): """ diff --git a/avocado/plugins/runner_nrunner.py b/avocado/plugins/runner_nrunner.py index 6e4675d405..8ad1ba455f 100644 --- a/avocado/plugins/runner_nrunner.py +++ b/avocado/plugins/runner_nrunner.py @@ -222,7 +222,7 @@ def _update_avocado_configuration_used_on_runnables(runnables, config): :type config: dict """ for runnable in runnables: - runnable.config = Runnable.add_configuration_used(runnable.kind, config) + runnable.config = Runnable.filter_runnable_config(runnable.kind, config) def _determine_status_server(self, test_suite, config_key): if test_suite.config.get("run.status_server_auto"): diff --git a/selftests/unit/nrunner.py b/selftests/unit/nrunner.py index 2c3133cb67..b633b4785d 100644 --- a/selftests/unit/nrunner.py +++ b/selftests/unit/nrunner.py @@ -102,14 +102,30 @@ def test_identifier_args(self): def test_runnable_command_args(self): runnable = Runnable("noop", "uri", "arg1", "arg2") actual_args = runnable.get_command_args() - exp_args = ["-k", "noop", "-u", "uri", "-a", "arg1", "-a", "arg2"] + exp_args = [ + "-k", + "noop", + "-u", + "uri", + "-c", + '{"runner.identifier_format": "{uri}"}', + "-a", + "arg1", + "-a", + "arg2", + ] self.assertEqual(actual_args, exp_args) def test_get_dict(self): runnable = Runnable("noop", "_uri_", "arg1", "arg2") self.assertEqual( runnable.get_dict(), - {"kind": "noop", "uri": "_uri_", "args": ("arg1", "arg2"), "config": {}}, + { + "kind": "noop", + "uri": "_uri_", + "args": ("arg1", "arg2"), + "config": {"runner.identifier_format": "{uri}"}, + }, ) def test_get_json(self): @@ -117,7 +133,7 @@ def test_get_json(self): expected = ( '{"kind": "noop", ' '"uri": "_uri_", ' - '"config": {}, ' + '"config": {"runner.identifier_format": "{uri}"}, ' '"args": ["arg1", "arg2"]}' ) self.assertEqual(runnable.get_json(), expected) From 2f55d12babf1a597b2f2b51a150a25cec123561a Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Tue, 25 Jun 2024 11:03:54 -0400 Subject: [PATCH 2/4] Fix use of job/suite configuration on runnables The setting of configuration into runnables is currently being done at the time where a runnable will be run (at the "nrunner" suite runner plugin). But, there are situations, such as when using the Job API, that this is too late or not effective. This change moves the setting of the configuration to the suite, which is based on the job configuration, and thus already has all the information needed to set the final version of the configuration. Fixes: https://github.com/avocado-framework/avocado/issues/5963 Signed-off-by: Cleber Rosa --- avocado/core/suite.py | 1 + avocado/plugins/runner_nrunner.py | 17 ----------------- selftests/check.py | 2 +- selftests/unit/suite.py | 11 +++++++++++ 4 files changed, 13 insertions(+), 18 deletions(-) diff --git a/avocado/core/suite.py b/avocado/core/suite.py index 6530a7176a..88949f526d 100644 --- a/avocado/core/suite.py +++ b/avocado/core/suite.py @@ -76,6 +76,7 @@ def resolutions_to_runnables(resolutions, config): if resolution.result != ReferenceResolutionResult.SUCCESS: continue for runnable in resolution.resolutions: + runnable.config = runnable.filter_runnable_config(runnable.kind, config) result.append(runnable) return result diff --git a/avocado/plugins/runner_nrunner.py b/avocado/plugins/runner_nrunner.py index 8ad1ba455f..0d06483dea 100644 --- a/avocado/plugins/runner_nrunner.py +++ b/avocado/plugins/runner_nrunner.py @@ -26,7 +26,6 @@ from avocado.core.dispatcher import SpawnerDispatcher from avocado.core.exceptions import JobError, JobFailFast from avocado.core.messages import MessageHandler -from avocado.core.nrunner.runnable import Runnable from avocado.core.nrunner.runner import check_runnables_runner_requirements from avocado.core.output import LOG_JOB from avocado.core.plugin_interfaces import CLI, Init, SuiteRunner @@ -212,18 +211,6 @@ def __init__(self): super().__init__() self.status_server_dir = None - @staticmethod - def _update_avocado_configuration_used_on_runnables(runnables, config): - """Updates the config used on runnables with this suite's config values - - :param runnables: the tasks whose runner requirements will be checked - :type runnables: list of :class:`Runnable` - :param config: A config dict to be used on the desired test suite. - :type config: dict - """ - for runnable in runnables: - runnable.config = Runnable.filter_runnable_config(runnable.kind, config) - def _determine_status_server(self, test_suite, config_key): if test_suite.config.get("run.status_server_auto"): # no UNIX domain sockets on Windows @@ -303,10 +290,6 @@ def run_suite(self, job, test_suite): test_suite.tests ) - self._update_avocado_configuration_used_on_runnables( - test_suite.tests, test_suite.config - ) - self._abort_if_missing_runners(missing_requirements) job.result.tests_total = len(test_suite.tests) diff --git a/selftests/check.py b/selftests/check.py index 5575681a80..2eccc9e621 100755 --- a/selftests/check.py +++ b/selftests/check.py @@ -27,7 +27,7 @@ "job-api-7": 1, "nrunner-interface": 70, "nrunner-requirement": 28, - "unit": 669, + "unit": 670, "jobs": 11, "functional-parallel": 307, "functional-serial": 7, diff --git a/selftests/unit/suite.py b/selftests/unit/suite.py index b3110a258d..51c967e8b9 100644 --- a/selftests/unit/suite.py +++ b/selftests/unit/suite.py @@ -74,6 +74,17 @@ def test_config_extend_automatic(self): self.suite = TestSuite.from_config(config=suite_config, job_config=job_config) self.assertEqual(self.suite.config.get("core.show"), ["none"]) + def test_config_runnable(self): + config = { + "resolver.references": [ + "examples/nrunner/recipes/runnable/noop.json", + ], + "runner.identifier_format": "NOT FOO", + } + suite = TestSuite.from_config(config) + runnable = suite.tests[0] + self.assertEqual(runnable.config.get("runner.identifier_format"), "NOT FOO") + def tearDown(self): self.tmpdir.cleanup() From 3d45f3799d5eed17a3d7b72535f38fd0b954d1f3 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Tue, 25 Jun 2024 21:04:28 -0400 Subject: [PATCH 3/4] Runnable identifier: use the actual configuration A runnable will have its configuration updated by the suite (and job) configuration at suite creation time. This configuration should be used, and not the one at the time the Runnable was crated, to set the runnable identifier. Signed-off-by: Cleber Rosa --- avocado/core/nrunner/runnable.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/avocado/core/nrunner/runnable.py b/avocado/core/nrunner/runnable.py index 7f2964d66d..18fb29a8a5 100644 --- a/avocado/core/nrunner/runnable.py +++ b/avocado/core/nrunner/runnable.py @@ -113,7 +113,6 @@ def __init__(self, kind, uri, *args, config=None, **kwargs): #: expressing assets that the test will require in order to run. self.assets = kwargs.pop("assets", None) self.kwargs = kwargs - self._identifier_format = config.get("runner.identifier_format", "{uri}") def __repr__(self): fmt = ( @@ -157,7 +156,7 @@ def identifier(self): Since this is formatter, combined values can be used. Example: "{uri}-{args}". """ - fmt = self._identifier_format + fmt = self.config.get("runner.identifier_format", "{uri}") # For the cases where there is no config (when calling the Runnable # directly From 3a73b3072b26891b75bbd4a76986e51e2ad077ec Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Tue, 25 Jun 2024 21:05:16 -0400 Subject: [PATCH 4/4] Runnable identifier: optmize for the most common scenario The most common scenario is the runnable having its identifier format after its URI. When that's the case, there's no need to build the arguments to the formatting string and performing the string format itself. Signed-off-by: Cleber Rosa --- avocado/core/nrunner/runnable.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/avocado/core/nrunner/runnable.py b/avocado/core/nrunner/runnable.py index 18fb29a8a5..78bff90f8b 100644 --- a/avocado/core/nrunner/runnable.py +++ b/avocado/core/nrunner/runnable.py @@ -158,9 +158,8 @@ def identifier(self): """ fmt = self.config.get("runner.identifier_format", "{uri}") - # For the cases where there is no config (when calling the Runnable - # directly - if not fmt: + # Optimize for the most common scenario + if fmt == "{uri}": return self.uri # For args we can use the entire list of arguments or with a specific