From 673c83ca1cb35256690677c1cddc88a4a2025586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Zachar?= <1548503+lukaszachy@users.noreply.github.com> Date: Tue, 4 Jun 2024 17:10:49 +0200 Subject: [PATCH] Accept additional rules in the `adjust()` call (#230) Use case: Explicit set of the rules to be applied, regardless what is in the Tree. --- fmf/base.py | 121 +++++++++++++++++++++----------------- tests/unit/test_adjust.py | 26 ++++++++ 2 files changed, 94 insertions(+), 53 deletions(-) diff --git a/fmf/base.py b/fmf/base.py index e7529608..4fdec044 100644 --- a/fmf/base.py +++ b/fmf/base.py @@ -343,7 +343,8 @@ def update(self, data): log.data(pretty(self.data)) def adjust(self, context, key='adjust', undecided='skip', - case_sensitive=True, decision_callback=None): + case_sensitive=True, decision_callback=None, + additional_rules=None): """ Adjust tree data based on provided context and rules @@ -365,6 +366,11 @@ class describing the environment context. By default, the key Optional 'decision_callback' callback would be called for every adjust rule inspected, with three arguments: current fmf node, current adjust rule, and whether it was applied or not. + + Optional 'additional_rules' parameter can be used to specify rules + that should be applied after those from the node itself. + These additional rules are processed even when an applied + rule defined in the node has ``continue: false`` set. """ # Check context sanity @@ -387,66 +393,75 @@ class describing the environment context. By default, the key except KeyError: rules = [] - context.case_sensitive = case_sensitive - - # Check and apply each rule - for rule in rules: - - # Rule must be a dictionary - if not isinstance(rule, dict): - raise utils.FormatError("Adjust rule should be a dictionary.") + # Accept same type as rules from data + if additional_rules is None: + additional_rules = [] + elif isinstance(additional_rules, dict): + additional_rules = [additional_rules] - # Missing 'when' means always enabled rule - try: - condition = rule['when'] - except KeyError: - condition = True - - # The optional 'continue' key should be a bool - continue_ = rule.get('continue', True) - if not isinstance(continue_, bool): - raise utils.FormatError( - "The 'continue' value should be bool, " - "got '{}'.".format(continue_)) + context.case_sensitive = case_sensitive - # Apply remaining rule attributes if context matches - try: - if context.matches(condition): - if decision_callback: - decision_callback(self, rule, True) - - # Remove special keys (when, because...) from the rule - apply_rule = { - key: value - for key, value in rule.items() - if key not in ADJUST_CONTROL_KEYS - } - self._merge_special(self.data, apply_rule) - - # First matching rule wins, skip the rest unless continue - if not continue_: - break - else: + # 'continue' has to affect only its rule_set + for rule_set in rules, additional_rules: + # Check and apply each rule + for rule in rule_set: + # Rule must be a dictionary + if not isinstance(rule, dict): + raise utils.FormatError("Adjust rule should be a dictionary.") + + # Missing 'when' means always enabled rule + try: + condition = rule['when'] + except KeyError: + condition = True + + # The optional 'continue' key should be a bool + continue_ = rule.get('continue', True) + if not isinstance(continue_, bool): + raise utils.FormatError( + "The 'continue' value should be bool, " + "got '{}'.".format(continue_)) + + # Apply remaining rule attributes if context matches + try: + if context.matches(condition): + if decision_callback: + decision_callback(self, rule, True) + + # Remove special keys (when, because...) from the rule + apply_rule = { + key: value + for key, value in rule.items() + if key not in ADJUST_CONTROL_KEYS + } + self._merge_special(self.data, apply_rule) + + # First matching rule wins, skip the rest of this set unless continue + if not continue_: + break + else: + if decision_callback: + decision_callback(self, rule, False) + # Handle undecided rules as requested + except fmf.context.CannotDecide: if decision_callback: - decision_callback(self, rule, False) - # Handle undecided rules as requested - except fmf.context.CannotDecide: - if decision_callback: - decision_callback(self, rule, None) + decision_callback(self, rule, None) - if undecided == 'skip': - continue - elif undecided == 'raise': - raise - else: - raise utils.GeneralError( - "Invalid value for the 'undecided' parameter. Should " - "be 'skip' or 'raise', got '{}'.".format(undecided)) + if undecided == 'skip': + continue + elif undecided == 'raise': + raise + else: + raise utils.GeneralError( + "Invalid value for the 'undecided' parameter. Should " + "be 'skip' or 'raise', got '{}'.".format(undecided)) # Adjust all child nodes as well for child in self.children.values(): child.adjust(context, key, undecided, - case_sensitive=case_sensitive, decision_callback=decision_callback) + case_sensitive=case_sensitive, + decision_callback=decision_callback, + additional_rules=additional_rules) def get(self, name=None, default=None): """ diff --git a/tests/unit/test_adjust.py b/tests/unit/test_adjust.py index 4230a39c..0e1fca6e 100644 --- a/tests/unit/test_adjust.py +++ b/tests/unit/test_adjust.py @@ -127,6 +127,24 @@ def test_adjusted(self, mini, centos): mini.adjust(centos) assert mini.get('enabled') is False + def test_adjusted_additional(self, mini, centos): + """ Additional rule is evaluated even if 'main' rule matched """ + mini.adjust(centos, additional_rules={'enabled': True}) + assert mini.get('enabled') is True + + def test_adjusted_additional_after_continue(self, full, centos): + """ Additional rule is evaluated even if 'node' rule has continue:false """ + full.adjust(centos, + additional_rules=[{'tag': 'foo'}, + {'require': 'bar', + 'continue': False, + 'when': 'distro == centos'}, + {'recommend': 'baz'}]) + assert full.get('enabled') is False + assert full.get('tag') == 'foo' + assert full.get('require') == 'bar' + assert full.get('recommend', []) == [] + def test_keep_original_adjust_rules(self, mini, centos): original_adjust = copy.deepcopy(mini.get('adjust')) mini.adjust(centos) @@ -210,6 +228,8 @@ def test_adjust_callback(self, mini, fedora, centos): # From the mini tree rule = mini.data['adjust'] + add_rule = {"when": "distro == centos", "foo": "bar"} + mock_callback = MagicMock(name='callback') mini.adjust(fmf.Context(), decision_callback=mock_callback) mock_callback.assert_called_once_with(mini, rule, None) @@ -222,6 +242,12 @@ def test_adjust_callback(self, mini, fedora, centos): mini.adjust(centos, decision_callback=mock_callback) mock_callback.assert_called_once_with(mini, rule, True) + mock_callback = MagicMock(name='callback') + mini.adjust(centos, decision_callback=mock_callback, additional_rules=[add_rule]) + mock_callback.assert_any_call(mini, rule, True) + mock_callback.assert_any_call(mini, add_rule, True) + assert mock_callback.call_count == 2 + def test_case_sensitive(self, mini, centos): """ Make sure the adjust rules are case-sensitive by default """ mini.data['adjust'] = dict(when='distro = CentOS', enabled=False)