From 17c621373645906ab7054493481530d322556941 Mon Sep 17 00:00:00 2001 From: Charles Whittington Date: Mon, 4 Nov 2024 23:24:13 -0500 Subject: [PATCH 1/5] Assign node to applicator before reapply can happen --- src/travertino/node.py | 6 +++--- tests/test_node.py | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/travertino/node.py b/src/travertino/node.py index bed40fb..3a392ba 100644 --- a/src/travertino/node.py +++ b/src/travertino/node.py @@ -44,12 +44,12 @@ def applicator(self): @applicator.setter def applicator(self, applicator): - self._applicator = applicator - self.style._applicator = applicator - if applicator: applicator.node = self + self._applicator = applicator + self.style._applicator = applicator + @property def root(self): """The root of the tree containing this node. diff --git a/tests/test_node.py b/tests/test_node.py index c281eb1..acc7ee9 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -1,4 +1,5 @@ from unittest.mock import Mock +from warnings import catch_warnings import pytest @@ -43,6 +44,17 @@ def layout(self, root, viewport): root.layout.content_height = viewport.height * 2 +class AttributeTestStyle(BaseStyle): + class IntrinsicSize(BaseIntrinsicSize): + pass + + class Box(BaseBox): + pass + + def reapply(self): + assert self._applicator.node.style is self + + def test_create_leaf(): """A leaf can be created""" style = Style() @@ -371,3 +383,11 @@ def test_apply_before_node_is_ready(): with pytest.warns(RuntimeWarning): Node(style=style, applicator=applicator) + + +def test_applicator_has_node_reference(): + # At the point that the style tries to apply itself, the applicator should already + # have a reference to its node. + + with catch_warnings(action="error", category=RuntimeWarning): + Node(style=AttributeTestStyle(), applicator=Mock()) From 6b6f27cbee4218465e723be30cf3451e187412ea Mon Sep 17 00:00:00 2001 From: Charles Whittington Date: Mon, 4 Nov 2024 23:28:47 -0500 Subject: [PATCH 2/5] changenote --- changes/232.misc.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/232.misc.rst diff --git a/changes/232.misc.rst b/changes/232.misc.rst new file mode 100644 index 0000000..688215f --- /dev/null +++ b/changes/232.misc.rst @@ -0,0 +1 @@ +Applicator gets its reference to its node *before* the applicator is assigned to that node's style. From ddab78eb2062665feee9bbe898d9f3f53e3ff86d Mon Sep 17 00:00:00 2001 From: Charles Whittington Date: Mon, 4 Nov 2024 23:32:14 -0500 Subject: [PATCH 3/5] Python version compatibility in test --- tests/test_node.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_node.py b/tests/test_node.py index acc7ee9..103808e 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -1,5 +1,5 @@ from unittest.mock import Mock -from warnings import catch_warnings +from warnings import catch_warnings, filterwarnings import pytest @@ -389,5 +389,6 @@ def test_applicator_has_node_reference(): # At the point that the style tries to apply itself, the applicator should already # have a reference to its node. - with catch_warnings(action="error", category=RuntimeWarning): + with catch_warnings(): + filterwarnings("error", category=RuntimeWarning) Node(style=AttributeTestStyle(), applicator=Mock()) From 28ddb5085c0ad6d7d83a9ec942b3874400903530 Mon Sep 17 00:00:00 2001 From: Charles Whittington Date: Tue, 5 Nov 2024 19:26:23 -0500 Subject: [PATCH 4/5] Added clarifying comments; clear node reference on removed applicator --- src/travertino/node.py | 9 +++++++++ tests/test_node.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/travertino/node.py b/src/travertino/node.py index 3a392ba..b0d50d5 100644 --- a/src/travertino/node.py +++ b/src/travertino/node.py @@ -44,10 +44,19 @@ def applicator(self): @applicator.setter def applicator(self, applicator): + if self.applicator: + # If an existing applicator is present, clear its reference to this node. + self.applicator.node = None + if applicator: + # This needs to happen *before* assigning the applicator to the style, + # below, because as part of receiving the applicator, the style will + # reapply itself. How this happens will vary with applicator + # implementation, but will probably need access to the node. applicator.node = self self._applicator = applicator + # This triggers style.reapply(): self.style._applicator = applicator @property diff --git a/tests/test_node.py b/tests/test_node.py index 103808e..92f27f4 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -332,6 +332,32 @@ def test_assign_applicator_none(node): node.style.reapply.assert_not_called() +def assign_new_applicator(): + # Assigning a new applicator clears the reference to node on the old applicator. + applicator_1 = Mock() + node = Node(style=Style(), applicator=applicator_1) + + assert applicator_1.node is node + + applicator_2 = Mock() + node.applicator = applicator_2 + + assert applicator_1.node is None + assert applicator_2.node is node + + +def assign_new_applicator_none(): + # Assigning None to applicator clears the reference to node on the old applicator. + applicator = Mock() + node = Node(style=Style(), applicator=applicator) + + assert applicator.node is node + + node.applicator = None + + assert applicator.node is None + + def test_assign_style_with_applicator(): style_1 = Style(int_prop=5) node = Node(style=style_1, applicator=Mock()) @@ -389,6 +415,11 @@ def test_applicator_has_node_reference(): # At the point that the style tries to apply itself, the applicator should already # have a reference to its node. + # We can't just check it after creating the widget, because at that point the + # reapply will have already happened. AttributeTestStyle has a reapply() method + # that asserts the reference trail of style -> applicator -> node -> style is + # already intact at the point that reapply is called. + with catch_warnings(): filterwarnings("error", category=RuntimeWarning) Node(style=AttributeTestStyle(), applicator=Mock()) From 3cc9760437c7cc0a9ab649cfebb43216ae80bbb5 Mon Sep 17 00:00:00 2001 From: Charles Whittington Date: Tue, 5 Nov 2024 19:53:28 -0500 Subject: [PATCH 5/5] Added doc strings to tests --- tests/test_node.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/test_node.py b/tests/test_node.py index 92f27f4..29374d5 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -258,6 +258,7 @@ def test_clear(): def test_create_with_no_applicator(): + """A node can be created without an applicator.""" style = Style(int_prop=5) node = Node(style=style) @@ -271,6 +272,7 @@ def test_create_with_no_applicator(): def test_create_with_applicator(): + """A node can be created with an applicator.""" style = Style(int_prop=5) applicator = Mock() node = Node(style=style, applicator=applicator) @@ -298,6 +300,7 @@ def test_create_with_applicator(): ], ) def test_assign_applicator(node): + """A node can be assigned an applicator after creation.""" node.style.reapply.reset_mock() applicator = Mock() @@ -321,6 +324,7 @@ def test_assign_applicator(node): ], ) def test_assign_applicator_none(node): + """A node can have its applicator set to None.""" node.style.reapply.reset_mock() node.applicator = None @@ -333,7 +337,7 @@ def test_assign_applicator_none(node): def assign_new_applicator(): - # Assigning a new applicator clears the reference to node on the old applicator. + """Assigning a new applicator clears reference to node on the old applicator.""" applicator_1 = Mock() node = Node(style=Style(), applicator=applicator_1) @@ -347,7 +351,7 @@ def assign_new_applicator(): def assign_new_applicator_none(): - # Assigning None to applicator clears the reference to node on the old applicator. + """Assigning None to applicator clears reference to node on the old applicator.""" applicator = Mock() node = Node(style=Style(), applicator=applicator) @@ -359,6 +363,7 @@ def assign_new_applicator_none(): def test_assign_style_with_applicator(): + """Assigning a new style triggers a reapply if an applicator is already present.""" style_1 = Style(int_prop=5) node = Node(style=style_1, applicator=Mock()) @@ -378,6 +383,7 @@ def test_assign_style_with_applicator(): def test_assign_style_with_no_applicator(): + """Assigning a new style doesn't trigger a reapply if an applicator isn't present.""" style_1 = Style(int_prop=5) node = Node(style=style_1) @@ -397,6 +403,7 @@ def test_assign_style_with_no_applicator(): def test_apply_before_node_is_ready(): + """Triggering a reapply raises a warning if the node is not ready to apply style.""" style = BrokenStyle() applicator = Mock() @@ -412,8 +419,7 @@ def test_apply_before_node_is_ready(): def test_applicator_has_node_reference(): - # At the point that the style tries to apply itself, the applicator should already - # have a reference to its node. + """Applicator should have a reference to its node before style is first applied.""" # We can't just check it after creating the widget, because at that point the # reapply will have already happened. AttributeTestStyle has a reapply() method