refactor incremental tests #434

fscherf · 2023-07-06T12:18:51Z

This PR refactors the incremental tests and fixes #433

m-reichle · 2023-07-10T09:39:24Z

Thank you for taking care of this!
This PR fixes my issues described in #433

Only noteworthy change I noticed is in a custom widget of mine (see below), where I just pass a single Node as self.nodes, this now results in an empty <NodeList([]))> whereas before the NodeList would contain my icon.

class CollapseIcon(Widget):

    def __init__(self, view):
        self.view = view
        self.html = view.html
        self.icon = I(
            _class="fas fa-minus-square",
            style={'cursor': 'pointer', 'color': '#3b8b5a', 'padding-right': '5px'},
            events=[CLICK]
        )
        self.nodes = self.icon

It is easily fixed doing a self.nodes = [self.icon] and I'm not sure if it was supposed to work before the change, but I thought I'd mention it since it's a change in behaviour.

sobolevn · 2023-07-10T09:46:53Z

lona/html/abstract_node.py

@@ -58,6 +58,11 @@ def __eq__(self, other):
        if self.NODE_TYPE == NODE_TYPE.TEXT_NODE:
            return self._string == other._string

+        # widgets
+        # TODO: remove in 2.0


Maybe issue a deprecation warning?

Until now we did not issue deprecation warnings but added wrote them to the documentation. It's a good idea to deprecate the Legacy Widget API now, but I think it's a little bit much for this PR because the old API gets used in multiple examples in the documentation. I opened #437 as a base for a follow-up PR.

sobolevn · 2023-07-10T09:49:01Z

tests/test_0107_html_node_compare.py

+    assert TestWidget(Div()) == TestWidget(Div())
+    assert TestWidget(Div()) != TestWidget(Span())
+
+    assert TestWidget(foo=1) == TestWidget(foo=1)


I think that this test can be split into multiple ones.

Good idea. Done!

Previously, the tests were designed to be incremental so failing tests in low-level components would cancel higher level tests. This way, a failing low-level test would not result in an all-red test-suite where the actual issue is hard to find. This behavior can be achieved much simpler, by ordering all tests from low-level to high-level and stopping at the first error. This patch removes all old infrastructure which implemented the incremental tests, and orders the tests from low-level to high-level, by using numeric file prefixes. After this patch the test-suite has the following stages: 00xx Testsuite tests 01xx HTML Datastructure Tests 02xx HTML API Tests 03xx Basic IO and Rendering Tests 04xx HTML API Browser Tests xxxx Misc Signed-off-by: Florian Scherf <[email protected]>

Signed-off-by: Florian Scherf <[email protected]>

client2 does not support `lona.html.Widget` and previously it would crash if it attempted to render widget nodes. Signed-off-by: Florian Scherf <[email protected]>

Signed-off-by: Florian Scherf <[email protected]>

Previously, `lona.html.Node.nodes` would only accept nodes and lists of nodes. This is unhandy when creating custom node classes and using `*args` and `**kwargs` since `*args` are tuples by default. This patch changes `lona.html.node_list.NodeList._reset()` to accept any kind of iterable. Signed-off-by: Florian Scherf <[email protected]>

Signed-off-by: Florian Scherf <[email protected]>

Previously, `lona.html.abstract_node.AbstractNode.__eq__()` would crash when one or both compared nodes where a widget. This patch fixes `lona.html.abstract_node.AbstractNode.__eq__()`, by adding proper compare logic for `lona.html.Widget`. Signed-off-by: Florian Scherf <[email protected]>

Signed-off-by: Florian Scherf <[email protected]>

codecov-commenter · 2023-07-10T11:38:09Z

Codecov Report

Patch coverage: 90.00% and project coverage change: +0.23 🎉

Comparison is base (47bf11b) 71.30% compared to head (b1b30f3) 71.53%.

❗ Current head b1b30f3 differs from pull request most recent head 5f79f2a. Consider uploading reports for the commit 5f79f2a to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #434      +/-   ##
==========================================
+ Coverage   71.30%   71.53%   +0.23%     
==========================================
  Files          83       83              
  Lines        5697     5702       +5     
  Branches     1226     1072     -154     
==========================================
+ Hits         4062     4079      +17     
- Misses       1345     1349       +4     
+ Partials      290      274      -16

Impacted Files	Coverage Δ
lona/html/node_list.py	`87.31% <50.00%> (+1.59%)`	⬆️
lona/html/abstract_node.py	`90.78% <100.00%> (+10.78%)`	⬆️
lona/html/node.py	`78.73% <100.00%> (+0.45%)`	⬆️
lona/html/widget_data.py	`56.82% <100.00%> (+0.44%)`	⬆️
lona/server.py	`70.14% <100.00%> (-1.05%)`	⬇️

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

fscherf · 2023-07-10T11:41:03Z

@m-reichle:

Only noteworthy change I noticed is in a custom widget of mine (see below), where I just pass a single Node as self.nodes, this now results in an empty <NodeList([]))> whereas before the NodeList would contain my icon.

Oh! Thanks for reporting, that is not supposed to happen. I updated the patch and added two tests so this may not happen again.

m-reichle · 2023-07-10T13:31:55Z

@m-reichle:

Only noteworthy change I noticed is in a custom widget of mine (see below), where I just pass a single Node as self.nodes, this now results in an empty <NodeList([]))> whereas before the NodeList would contain my icon.

Oh! Thanks for reporting, that is not supposed to happen. I updated the patch and added two tests so this may not happen again.

Jup, runs without issues now!

fscherf · 2023-07-13T10:32:05Z

@m-reichle: Great! Thanks for testing and reporting #433 in the first place!

fscherf requested a review from sobolevn July 6, 2023 12:18

fscherf mentioned this pull request Jul 6, 2023

AttributeError when comparing nodes #433

Closed

sobolevn reviewed Jul 10, 2023

View reviewed changes

fscherf added 8 commits July 10, 2023 13:16

tests: rendering: refactor test parameter

3a77a76

Signed-off-by: Florian Scherf <[email protected]>

client2: rendering: add check for supported nodes

d8c7904

client2 does not support `lona.html.Widget` and previously it would crash if it attempted to render widget nodes. Signed-off-by: Florian Scherf <[email protected]>

server: use most abstract class of lona.responses for all checks

241453c

Signed-off-by: Florian Scherf <[email protected]>

html: add support for initializing widget data via node constructors

43abe6a

Signed-off-by: Florian Scherf <[email protected]>

tests: html: add tests for node comparisons

5f79f2a

Signed-off-by: Florian Scherf <[email protected]>

fscherf force-pushed the fscherf/refactor-incremental-tests branch from b1b30f3 to 5f79f2a Compare July 10, 2023 11:37

fscherf requested a review from sobolevn July 10, 2023 11:48

fscherf merged commit 413569c into master Jul 13, 2023

fscherf deleted the fscherf/refactor-incremental-tests branch July 13, 2023 10:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor incremental tests #434

refactor incremental tests #434

fscherf commented Jul 6, 2023

m-reichle commented Jul 10, 2023

sobolevn Jul 10, 2023

fscherf Jul 10, 2023

sobolevn Jul 10, 2023

fscherf Jul 10, 2023

codecov-commenter commented Jul 10, 2023

fscherf commented Jul 10, 2023

m-reichle commented Jul 10, 2023

fscherf commented Jul 13, 2023

refactor incremental tests #434

refactor incremental tests #434

Conversation

fscherf commented Jul 6, 2023

m-reichle commented Jul 10, 2023

sobolevn Jul 10, 2023

Choose a reason for hiding this comment

fscherf Jul 10, 2023

Choose a reason for hiding this comment

sobolevn Jul 10, 2023

Choose a reason for hiding this comment

fscherf Jul 10, 2023

Choose a reason for hiding this comment

codecov-commenter commented Jul 10, 2023

Codecov Report

fscherf commented Jul 10, 2023

m-reichle commented Jul 10, 2023

fscherf commented Jul 13, 2023