Minor fine-tuning.

- Make `Keyword.args` and `assign` tuples. Earlier their type was not enforced. - Change `ModelObject.copy` and `deepcopy` to preserve tuples and disallow setting non-existing attributes.
robotframework · Jun 9, 2023 · a5a37e4 · a5a37e4
1 parent f574b1a
commit a5a37e4
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 30 deletions.
diff --git a/src/robot/model/keyword.py b/src/robot/model/keyword.py
@@ -40,8 +40,8 @@ def __init__(self, name: 'str|None' = '',
                  type: str = BodyItem.KEYWORD,
                  parent: BodyItemParent = None):
         self.name = name
-        self.args = args
-        self.assign = assign
+        self.args = tuple(args)
+        self.assign = tuple(assign)
         self.type = type
         self.parent = parent
 
@@ -74,9 +74,9 @@ def __str__(self) -> str:
     def to_dict(self) -> DataDict:
         data: DataDict = {'name': self.name}
         if self.args:
-            data['args'] = list(self.args)
+            data['args'] = self.args
         if self.assign:
-            data['assign'] = list(self.assign)
+            data['assign'] = self.assign
         return data
 
 

diff --git a/src/robot/model/modelobject.py b/src/robot/model/modelobject.py
@@ -150,10 +150,7 @@ def copy(self: T, **attributes) -> T:
 
         __ https://docs.python.org/3/library/copy.html
         """
-        copied = copy.copy(self)
-        for name in attributes:
-            setattr(copied, name, attributes[name])
-        return copied
+        return copy.copy(self).config(**attributes)
 
     def deepcopy(self: T, **attributes) -> T:
         """Return a deep copy of this object.
@@ -167,10 +164,7 @@ def deepcopy(self: T, **attributes) -> T:
 
         __ https://docs.python.org/3/library/copy.html
         """
-        copied = copy.deepcopy(self)
-        for name in attributes:
-            setattr(copied, name, attributes[name])
-        return copied
+        return copy.deepcopy(self).config(**attributes)
 
     def __repr__(self) -> str:
         arguments = [(name, getattr(self, name)) for name in self.repr_args]

diff --git a/utest/model/test_keyword.py b/utest/model/test_keyword.py
@@ -78,13 +78,13 @@ def test_string_reprs(self):
              "Keyword(name='Name', args=(), assign=('${x}', '${y}'))"),
             (Keyword('Name', assign=['${x}='], args=['x']),
              '${x}=    Name    x',
-             "Keyword(name='Name', args=['x'], assign=['${x}='])"),
+             "Keyword(name='Name', args=('x',), assign=('${x}=',))"),
             (Keyword('Name', args=(1, 2, 3)),
              'Name    1    2    3',
              "Keyword(name='Name', args=(1, 2, 3), assign=())"),
-            (Keyword(assign=['${\xe3}'], name='\xe4', args=['\xe5']),
-             '${\xe3}    \xe4    \xe5',
-             'Keyword(name=%r, args=[%r], assign=[%r])' % ('\xe4', '\xe5', '${\xe3}'))
+            (Keyword(assign=['${ã}'], name='ä', args=['å']),
+             '${ã}    ä    å',
+             "Keyword(name='ä', args=('å',), assign=('${ã}',))")
         ]:
             assert_equal(str(kw), exp_str)
             assert_equal(repr(kw), 'robot.model.' + exp_repr)
@@ -98,24 +98,28 @@ def test_copy(self):
         assert_equal(kw.name, copy.name)
         copy.name += ' copy'
         assert_not_equal(kw.name, copy.name)
-        assert_equal(id(kw.args), id(copy.args))
+        assert_equal(kw.args, copy.args)
 
     def test_copy_with_attributes(self):
-        kw = Keyword(name='Orig', args=['orig'])
+        kw = Keyword(name='Orig', args=('orig',))
         copy = kw.copy(name='New', args=['new'])
         assert_equal(copy.name, 'New')
-        assert_equal(copy.args, ['new'])
+        assert_equal(copy.args, ('new',))
 
     def test_deepcopy(self):
         kw = Keyword(name='Keyword', args=['a'])
         copy = kw.deepcopy()
         assert_equal(kw.name, copy.name)
-        assert_not_equal(id(kw.args), id(copy.args))
+        assert_equal(kw.args, copy.args)
 
     def test_deepcopy_with_attributes(self):
         copy = Keyword(name='Orig').deepcopy(name='New', args=['New'])
         assert_equal(copy.name, 'New')
-        assert_equal(copy.args, ['New'])
+        assert_equal(copy.args, ('New',))
+
+    def test_copy_and_deepcopy_with_non_existing_attributes(self):
+        assert_raises(AttributeError, Keyword().copy, bad='attr')
+        assert_raises(AttributeError, Keyword().deepcopy, bad='attr')
 
 
 class TestKeywords(unittest.TestCase):

diff --git a/utest/running/test_run_model.py b/utest/running/test_run_model.py
@@ -272,8 +272,8 @@ class TestToFromDictAndJson(unittest.TestCase):
     def test_keyword(self):
         self._verify(Keyword(), name='')
         self._verify(Keyword('Name'), name='Name')
-        self._verify(Keyword('N', tuple('args'), ('${result}',)),
-                     name='N', args=list('args'), assign=['${result}'])
+        self._verify(Keyword('N', 'args', ('${result}',)),
+                     name='N', args=tuple('args'), assign=('${result}',))
         self._verify(Keyword('Setup', type=Keyword.SETUP, lineno=1),
                      name='Setup', lineno=1)
 
@@ -312,7 +312,7 @@ def test_if_structure(self):
         self._verify(root,
                      type='IF/ELSE ROOT',
                      body=[{'type': 'IF', 'condition': '$c', 'body': [{'name': 'K1'}]},
-                           {'type': 'ELSE', 'body': [{'name': 'K2', 'args': ['a']}]}])
+                           {'type': 'ELSE', 'body': [{'name': 'K2', 'args': ('a',)}]}])
 
     def test_try(self):
         self._verify(Try(), type='TRY/EXCEPT ROOT', body=[])
@@ -365,15 +365,15 @@ def test_test_structure(self):
         test = TestCase('TC')
         test.setup.config(name='Setup')
         test.teardown.config(name='Teardown', args='a')
-        test.body.create_keyword('K1')
-        test.body.create_if().body.create_branch().body.create_keyword('K2')
+        test.body.create_keyword('K1', 'a')
+        test.body.create_if().body.create_branch('IF', '$c').body.create_keyword('K2')
         self._verify(test,
                      name='TC',
                      setup={'name': 'Setup'},
-                     teardown={'name': 'Teardown', 'args': ['a']},
-                     body=[{'name': 'K1'},
+                     teardown={'name': 'Teardown', 'args': ('a',)},
+                     body=[{'name': 'K1', 'args': ('a',)},
                            {'type': 'IF/ELSE ROOT',
-                            'body': [{'type': 'IF', 'condition': None,
+                            'body': [{'type': 'IF', 'condition': '$c',
                                       'body': [{'name': 'K2'}]}]}])
 
     def test_suite(self):
@@ -391,7 +391,7 @@ def test_suite_structure(self):
         self._verify(suite,
                      name='Root',
                      setup={'name': 'Setup'},
-                     teardown={'name': 'Teardown', 'args': ['a']},
+                     teardown={'name': 'Teardown', 'args': ('a',)},
                      tests=[{'name': 'T1', 'body': [{'name': 'K'}]}],
                      suites=[{'name': 'Child',
                               'tests': [{'name': 'T2', 'body': []}],