From 1c51e6019529a9d452ad4647b485ab5a2401c6a4 Mon Sep 17 00:00:00 2001 From: toofar Date: Sun, 27 Apr 2025 12:28:51 +1200 Subject: [PATCH] Move tree tab moving and dragging tests to test_notree This commit moves the tests around moving tree tabs into a unit test targeting the notree library direction. The tests haven't changed but they run much faster without having to spin up real tabs and windows. I've left one complex test for each method in the end2end test to make sure everything is still working there. I've moved the existing `create_tree()` method into `test_notree.py` and added a symmetric `tree_to_str()` method. I hope this makes the test setup more readable than constructing Node objects directly or having one fixed tree that all the tests operate on. These tests run in about 150ms on my machine, vs about 20s when they were using real tabs. I've used a fancy typed NamedTuple to provide parameters for pytest because it was quite difficult to discern the different tuple entries when there was just three strings stacked on top each other. The keyword arguments make it much more readable than the default pytest setup of string parameter names and then a big list of tuples. Inspired by these two posts: https://til.simonwillison.net/pytest/namedtuple-parameterized-tests https://mathspp.com/blog/til/better-test-parametrisation-in-pytest --- tests/end2end/features/treetabs.feature | 270 +----------- .../unit/mainwindow/test_treetabbedbrowser.py | 33 +- tests/unit/misc/test_notree.py | 387 ++++++++++++++++++ 3 files changed, 400 insertions(+), 290 deletions(-) diff --git a/tests/end2end/features/treetabs.feature b/tests/end2end/features/treetabs.feature index 0d83eb071..4f08984bf 100644 --- a/tests/end2end/features/treetabs.feature +++ b/tests/end2end/features/treetabs.feature @@ -417,67 +417,7 @@ Feature: Tree tab management - about:blank?three (active) """ - ## :tab-move, but with trees - Scenario: Move tab out of a group - When I open about:blank?one - And I open about:blank?two in a new related tab - And I open about:blank?three in a new related tab - And I open about:blank?four in a new tab - And I run :tab-select ?three - And I run :tab-move 4 - Then the following tabs should be open: - # one - # two - # three (active) - # four - """ - - about:blank?one - - about:blank?two - - about:blank?four - - about:blank?three (active) - """ - - Scenario: Move multiple tabs out of a group - When I open about:blank?one - And I open about:blank?two in a new related tab - And I open about:blank?three in a new related tab - And I open about:blank?four in a new tab - And I open about:blank?five in a new related tab - And I run :tab-select ?two - And I run :tab-move 4 - Then the following tabs should be open: - # one - # two (active) - # three - # four - # five - """ - - about:blank?one - - about:blank?four - - about:blank?five - - about:blank?two (active) - - about:blank?three - """ - - Scenario: Move two sibling groups - When I open about:blank?one - And I open about:blank?two in a new related tab - And I open about:blank?three in a new tab - And I open about:blank?four in a new related tab - And I run :tab-select ?three - And I run :tab-move 1 - Then the following tabs should be open: - # one - # two - # three (active) - # four - """ - - about:blank?three (active) - - about:blank?four - - about:blank?one - - about:blank?two - """ - + # Test a complex move case, most coverage is in test_notree.py Scenario: Move multiple tabs into another group When I open about:blank?one And I open about:blank?two in a new related tab @@ -500,207 +440,8 @@ Feature: Tree tab management - about:blank?three """ - Scenario: Move a tab a single step over a group - When I open about:blank?one - And I open about:blank?two in a new tab - And I open about:blank?three in a new related tab - And I run :tab-select ?one - And I run :tab-move 2 - Then the following tabs should be open: - # one (active) - # two - # three - """ - - about:blank?two - - about:blank?three - - about:blank?one - """ - - ## Move tabs via mouse drags - Scenario: Drag a tab down between siblings - When I open about:blank?one - And I open about:blank?two in a new tab - And I run :tab-select ?one - And I run :debug-mouse-move + - Then the following tabs should be open: - # one (active) - # two - """ - - about:blank?two - - about:blank?one (active) - """ - - Scenario: Drag a tab down out of a group - When I open about:blank?one - And I open about:blank?two in a new related tab - And I open about:blank?three in a new related tab - And I open about:blank?four in a new tab - And I run :tab-select ?three - And I run :debug-mouse-move + - Then the following tabs should be open: - # one - # two - # three (active) - # four - """ - - about:blank?one - - about:blank?two - - about:blank?four - - about:blank?three (active) - """ - - Scenario: Drag a tab down out of a group into another - When I open about:blank?one - And I open about:blank?two in a new related tab - And I open about:blank?three in a new related tab - And I open about:blank?four in a new tab - And I open about:blank?five in a new related tab - And I run :tab-select ?three - And I run :debug-mouse-move + - Then the following tabs should be open: - # one - # two - # three (active) - # four - # five - """ - - about:blank?one - - about:blank?two - - about:blank?four - - about:blank?three (active) - - about:blank?five - """ - - Scenario: Drag a tab down a group - When I open about:blank?one - And I open about:blank?two in a new related tab - And I open about:blank?three in a new related tab - And I open about:blank?four in a new tab - And I run :tab-select ?two - And I run :debug-mouse-move + - Then the following tabs should be open: - # one - # two (active) - # three - # four - """ - - about:blank?one - - about:blank?three - - about:blank?two (active) - - about:blank?four - """ - - Scenario: Drag a tab with children down a group - When I open about:blank?one - And I open about:blank?two in a new related tab - And I open about:blank?five in a new related tab - And I open about:blank?three in a new sibling tab - And I open about:blank?four in a new related tab - And I open about:blank?six in a new tab - And I run :tab-select ?two - And I run :debug-mouse-move + - Then the following tabs should be open: - # one - # two (active) - # three - # four - # five - # six - """ - - about:blank?one - - about:blank?three - - about:blank?two (active) - - about:blank?four - - about:blank?five - - about:blank?six - """ - - ## And dragging upwards - Scenario: Drag a tab up between siblings - When I open about:blank?one - And I open about:blank?two in a new tab - And I open about:blank?three in a new related tab - And I run :tab-select ?two - And I run :debug-mouse-move - - Then the following tabs should be open: - # one - # two (active) - # three - """ - - about:blank?two (active) - - about:blank?one - - about:blank?three - """ - - Scenario: Drag a tab up out of a group - When I open about:blank?one - And I open about:blank?two in a new tab - And I open about:blank?three in a new related tab - And I open about:blank?four in a new related tab - And I run :tab-select ?three - And I run :debug-mouse-move - - Then the following tabs should be open: - # one - # two - # three (active) - # four - """ - - about:blank?one - - about:blank?three (active) - - about:blank?two - - about:blank?four - """ - - Scenario: Drag a tab with children up into a group - When I open about:blank?one - And I open about:blank?foo in a new related tab - And I open about:blank?two in a new sibling tab - And I open about:blank?three in a new tab - And I open about:blank?four in a new related tab - And I open about:blank?five in a new tab - And I run :tab-select ?three - And I run :debug-mouse-move - - Then the following tabs should be open: - # one - # two - # foo - # three (active) - # four - # five - """ - - about:blank?one - - about:blank?two - - about:blank?three (active) - - about:blank?foo - - about:blank?four - - about:blank?five - """ - - Scenario: Drag a tab up a group - When I open about:blank?one - And I open about:blank?two in a new related tab - And I open about:blank?five in a new related tab - And I open about:blank?three in a new sibling tab - And I open about:blank?six in a new related tab - And I open about:blank?four in a new tab - And I run :tab-select ?three - And I run :debug-mouse-move - - Then the following tabs should be open: - # one - # two - # three (active) - # six - # five - # four - """ - - about:blank?one - - about:blank?three (active) - - about:blank?two - - about:blank?six - - about:blank?five - - about:blank?four - """ - + # Test dragging upwards a couple of steps, most coverage is in + # test_notree.py Scenario: Drag a tab with children up a group When I open about:blank?one And I open about:blank?two in a new related tab @@ -710,6 +451,7 @@ Feature: Tree tab management And I open about:blank?six in a new tab And I run :tab-select ?three And I run :debug-mouse-move - + And I run :debug-mouse-move - Then the following tabs should be open: # one # two @@ -718,8 +460,8 @@ Feature: Tree tab management # five # six """ - - about:blank?one - - about:blank?three (active) + - about:blank?three (active) + - about:blank?one - about:blank?two - about:blank?four - about:blank?five diff --git a/tests/unit/mainwindow/test_treetabbedbrowser.py b/tests/unit/mainwindow/test_treetabbedbrowser.py index 3ae2b3b00..de8fa8e5c 100644 --- a/tests/unit/mainwindow/test_treetabbedbrowser.py +++ b/tests/unit/mainwindow/test_treetabbedbrowser.py @@ -8,6 +8,8 @@ from qutebrowser.config.configtypes import NewTabPosition, NewChildPosition from qutebrowser.misc.notree import Node from qutebrowser.mainwindow import treetabbedbrowser, treetabwidget +from tests.unit.misc.test_notree import str_to_tree + @pytest.fixture def mock_browser(mocker): @@ -75,7 +77,7 @@ class TestPositionTab: """Test tree tab positioning. How to use the parameters above: - * refer to the tree structure being passed to create_tree() below, that's + * refer to the tree structure being passed to str_to_tree() below, that's our starting state * specify how the new node should be related to the current one * specify cur_node by value, which is the tab currently focused when the @@ -94,7 +96,7 @@ class TestPositionTab: situations apart. But I went this route to avoid having to specify multiple trees in the parameters. """ - root = self.create_tree( + root = str_to_tree( """ - one - two @@ -104,7 +106,7 @@ class TestPositionTab: - six - seven """, - ) + )[0] new_node = Node("new", parent=root) config_stub.val.tabs.new_position.stacking = False @@ -178,27 +180,6 @@ class TestPositionTab: background=background, ) - def create_tree(self, tree_str): - # Construct a notree.Node tree from the test string. - root = Node("root") - previous_indent = '' - previous_node = root - for line in tree_str.splitlines(): - if not line.strip(): - continue - indent, value = line.split("-") - node = Node(value.strip()) - if len(indent) > len(previous_indent): - node.parent = previous_node - elif len(indent) == len(previous_indent): - node.parent = previous_node.parent - else: - # TODO: handle going up in jumps of more than one rank - node.parent = previous_node.parent.parent - previous_indent = indent - previous_node = node - return root - @pytest.mark.parametrize( " test_tree, relation, pos, expected", [ ("tree_one", "sibling", "next", "one,two,new1,new2,new3",), @@ -229,12 +210,12 @@ class TestPositionTab: """ # Simpler tree here to make the assert string a bit simpler. # Tab "two" is hardcoded as cur_tab. - root = self.create_tree( + root = str_to_tree( """ - one - two """, - ) + )[0] config_stub.val.tabs.new_position.stacking = True for val in ["new1", "new2", "new3"]: diff --git a/tests/unit/misc/test_notree.py b/tests/unit/misc/test_notree.py index 056daf463..0fb8ecb07 100644 --- a/tests/unit/misc/test_notree.py +++ b/tests/unit/misc/test_notree.py @@ -2,6 +2,10 @@ # # SPDX-License-Identifier: GPL-3.0-or-later """Tests for misc.notree library.""" +import itertools +import textwrap +from typing import NamedTuple, Optional + import pytest from qutebrowser.misc.notree import TreeError, Node, TraverseOrder @@ -294,3 +298,386 @@ def test_memoization(node): assert node.children[0]._Node__modified is True node.render() assert node._Node__modified is False + + +def str_to_tree(tree_str: str) -> tuple["Node", "Node"]: + """Construct a notree.Node tree from a string. + + Input strings can look like: + one (active) + two + + Or + - one (active) + - two + + You can use any indent to separate levels of the tree but it needs to be + consistent. + + Returns a tuple of (tree_root, active_tab). + """ + root = Node("root") + previous_indent = "" + previous_node = root + indent_increment = None + active = None + for line in textwrap.dedent(tree_str).splitlines(): + if not line.strip(): + continue + + indent = "".join(list(itertools.takewhile(lambda c: c.isspace(), line))) + if indent and not indent_increment: + indent_increment = indent + + line = line.removeprefix(indent) + line = line.removeprefix("- ") # Strip optional dash prefix thing + parts = line.split() + value = parts[0] + node = Node(value) + + if len(parts) > 1: + if parts[1] == "(active)": + active = node + + if previous_node == root: + node.parent = root + elif len(indent) > len(previous_indent): + node.parent = previous_node + elif len(indent) == len(previous_indent): + node.parent = previous_node.parent + else: + if not indent: + level = 0 + else: + level = int(len(indent) / len(indent_increment)) + node.parent = previous_node.path[level] + + previous_indent = indent + previous_node = node + return root, active + + +def tree_to_str( + node: Node, + active: Optional[bool] = None, + indent: str = "", + indent_increment: str = " ", + include_root: bool = False, +) -> None: + """Serialize Node tree into a string. + + Output string will look like: + + one + two + three (active) + + With two spaces of indentation marking levels of the tree. The reverse of + `str_to_tree()`. + """ + lines = [] + if not node.parent and include_root is False: + next_indent = indent + else: + next_indent = indent + indent_increment + lines.append(f"{indent}{node.value}{' (active)' if node == active else ''}") + for child in node.children: + lines.append( + tree_to_str( + child, active, indent=next_indent, indent_increment=indent_increment + ) + ) + return "\n".join(lines) + + +class MoveTestArgs(NamedTuple): + description: str + before: str + to: str + after: str + + +@pytest.mark.parametrize( + MoveTestArgs._fields, + [ + MoveTestArgs( + description="Move a tab out of a group", + before=""" + one + two + three (active) + four + """, + to="four", + after=""" + one + two + four + three (active) + """, + ), + MoveTestArgs( + description="Move multiple tabs out of a group", + before=""" + one + two (active) + three + four + five + """, + to="four", + after=""" + one + four + five + two (active) + three + """, + ), + MoveTestArgs( + description="Move two sibling groups", + before=""" + one + two + three (active) + four + """, + to="one", + after=""" + three (active) + four + one + two + """, + ), + MoveTestArgs( + description="Move multiple tabs into another group", + before=""" + one + two (active) + three + four + five + """, + to="five", + after=""" + one + four + five + two (active) + three + """, + ), + MoveTestArgs( + description="Move a tab a single step over a group", + before=""" + one (active) + two + three + """, + to="two", + after=""" + two + three + one (active) + """, + ), + ], +) +def test_move_recursive(description, before, to, after): + before, after = textwrap.dedent(before), textwrap.dedent(after).strip() + tree_root, active = str_to_tree(before) + nodes = list(tree_root.traverse(render_collapsed=False)) + to_node = [node for node in nodes if node.value == to][0] + + active.move_recursive(to_node) + + assert tree_to_str(tree_root, active) == after + + +@pytest.mark.parametrize( + MoveTestArgs._fields, + [ + # And now the drag downwards test cases + MoveTestArgs( + description="Drag a tab down between siblings", + before=""" + one (active) + two + """, + to="+", + after=""" + two + one (active) + """, + ), + MoveTestArgs( + description="Drag a tab down out of a group", + before=""" + one + two + three (active) + four + """, + to="+", + after=""" + one + two + four + three (active) + """, + ), + MoveTestArgs( + description="Drag a tab down out of a group into another", + before=""" + one + two + three (active) + four + five + """, + to="+", + after=""" + one + two + four + three (active) + five + """, + ), + MoveTestArgs( + description="Drag a tab down a group", + before=""" + one + two (active) + three + four + """, + to="+", + after=""" + one + three + two (active) + four + """, + ), + MoveTestArgs( + description="Drag a tab with children down a group", + before=""" + one + two (active) + three + four + five + six + """, + to="+", + after=""" + one + three + two (active) + four + five + six + """, + ), + # And now the drag upwards test cases + MoveTestArgs( + description="Drag a tab up between siblings", + before=""" + one + two (active) + three + """, + to="-", + after=""" + two (active) + one + three + """, + ), + MoveTestArgs( + description="Drag a tab up out of a group", + before=""" + one + two + three (active) + four + """, + to="-", + after=""" + one + three (active) + two + four + """, + ), + MoveTestArgs( + description="Drag a tab with children up into a group", + before=""" + one + two + foo + three (active) + four + five + """, + to="-", + after=""" + one + two + three (active) + foo + four + five + """, + ), + MoveTestArgs( + description="Drag a tab up a group", + before=""" + one + two + three (active) + six + five + four + """, + to="-", + after=""" + one + three (active) + two + six + five + four + """, + ), + MoveTestArgs( + description="Drag a tab with children up a group", + before=""" + one + two + three (active) + four + five + six + """, + to="-", + after=""" + one + three (active) + two + four + five + six + """, + ), + ], +) +def test_drag(description, before, to, after): + before, after = textwrap.dedent(before), textwrap.dedent(after).strip() + tree_root, active = str_to_tree(before) + + active.drag(to) + + assert tree_to_str(tree_root, active) == after