Add more end2end tests for tree tabs feature

[pylbrecht]

I simply translated parts of @toofar's superb documentation into end2end tests.

Covering

• :tab-close without --recursive
• :tab-next --sibling
• :tab-prev --sibling
• :tab-focus parent
• :tab-move <index>

---

Fixes part of #8072

[toofar]

Thanks for more tests! And sorry for the long delay for such a straightforward PR, I'll try to be more prompt in the future.

I can see that the new tests all cover new functionality and they all pass. But might I suggest that some of them are too trusting? Specifically:

• the tab-next and tab-prev tests have the same setup and assertion. If I can swap one command out for the other, how do I know that the two commands actually have different functionality, they must just be aliases!
• the tab-focus parent test would pass with tab-focus prev as well right? Oh we don't support that. Anyhow, if it actually skipped a sibling to go up to the parent I think it would demonstrate what it is doing better.

I don't think the :tab-focus parent one is working correctly. I think it should only focus the immediate parent, not the grandparent or root like the test says. I think the problem here is that since tab 3 is already focused tab-focus 3 ends up switching away from the active tab to the previous tab, tab 2. Who's parent then (correctly) gets focused by tab-focus parent.

[pylbrecht]

@toofar, thanks for your thorough review! ..and apologies for letting it just sit there for the past couple of days.
I will work on your suggestions in the next days.

[pylbrecht]

@toofar, I updated the :tab-focus parent test. That one is now failing:

FAILED tests/end2end/features/test_treetabs_bdd.py::test_tabfocus_parent - AssertionError: assert '  http://localhost:64851/data/numbers/2.txt' == '  http://localhost:64851/data/numbers/3.txt'

After inspecting the session, it seems that the order of tabs is off. The children of data/numbers/1.txt are reversed.

Full session data

Current session data:
windows:
- active: true
  geometry: !!binary |
    AdnQywADAAAAAANjAAAAJgAABr8AAARcAAADYwAAAEIAAAa/AAAEXAAAAAAAAAAABsAAAANjAAAA
    QgAABr8AAARc
  tabs:
  - active: true
    history:
    - last_visited: '2024-02-20T20:39:49'
      pinned: false
      title: about:blank
      url: about:blank
    - active: true
      last_visited: '2024-02-20T20:39:49'
      pinned: false
      scroll-pos:
        x: 0
        y: 0
      title: localhost:64851/data/numbers/1.txt
      url: http://localhost:64851/data/numbers/1.txt
      zoom: 1.0
    treetab_node_data:
      children:
      - 24
      - 23
      collapsed: false
      node: 22
      parent: 20
      uid: 22
  - history:
    - active: true
      last_visited: '2024-02-20T20:39:49'
      pinned: false
      scroll-pos:
        x: 0
        y: 0
      title: localhost:64851/data/numbers/3.txt
      url: http://localhost:64851/data/numbers/3.txt
      zoom: 1.0
    treetab_node_data:
      children: []
      collapsed: false
      node: 24
      parent: 22
      uid: 24
  - history:
    - active: true
      last_visited: '2024-02-20T20:39:49'
      pinned: false
      scroll-pos:
        x: 0
        y: 0
      title: localhost:64851/data/numbers/2.txt
      url: http://localhost:64851/data/numbers/2.txt
      zoom: 1.0
    treetab_node_data:
      children: []
      collapsed: false
      node: 23
      parent: 22
      uid: 23
  treetab_root:
    children:
    - 22
    uid: 20

I would expect the session to reflect the same order in which tabs are opened in the end2end test:

When I open data/numbers/1.txt
And I open data/numbers/2.txt in a new related tab
And I open data/numbers/3.txt in a new sibling tab

               |   |
               |   |
              \     /
               \   /
                \ /

    - data/numbers/1.txt
      - data/numbers/2.txt
      - data/numbers/3.txt

With this change the test passes:

diff --git a/tests/end2end/features/treetabs.feature b/tests/end2end/features/treetabs.feature
index 56f744fc4..da6c8b118 100644
--- a/tests/end2end/features/treetabs.feature
+++ b/tests/end2end/features/treetabs.feature
@@ -54,8 +54,8 @@ Feature: Tree tab management
         And I run :tab-focus parent
         Then the following tabs should be open:
             - data/numbers/1.txt (active)
-              - data/numbers/2.txt
               - data/numbers/3.txt
+              - data/numbers/2.txt
 
     Scenario: :tab-close --recursive
         When I open data/numbers/1.txt

Any idea why that is?

---

P.S.
My best idea is to dig into the comparison logic in tests/end2end/features/conftest.py::check_open_tabs(), but I am too tired to wrap my head around that right now. I am hoping that I simply misunderstand some of the tree tab commands here. 🤞

[toofar]

My best idea is to dig into the comparison logic

gasp how could you! My code is always perfect!

What I find useful as the first step in these situations is to run through the steps manually and to see if the test results are accurate, or if I need to add a bunch more debugging. Eg:

python3 -m qutebrowser -T -s tabs.tree_tabs true -s tabs.position left -s tabs.width 50%
:open about:blank?1
:open -t -r about:blank?2
:open -t -S about:blank?3

And I'm seeing:

There is the setting tabs.new_position.tree.new_sibling that controls where new sibling tabs are placed and that indeed defaults to first. So it seems tab 3 is being placed right where it is supposed to be. I respect your surprise though, I don't have mine set to that, but I haven't been using tree tabs for very long. I'll leave it up to you to decide how you want to handle that, either way though this might be some valuable feedback for #8075

[pylbrecht]

gasp how dare you! My code is always perfect!

Forgive me, master. Never shall I place your code in doubt again. 🙏

What I find useful as the first step in these situations is to run through the steps manually and to see if the test results are accurate, or if I need to add a bunch more debugging. Eg:

python3 -m qutebrowser -T -s tabs.tree_tabs true -s tabs.position left -s tabs.width 50%
:open about:blank?1
:open -t -r about:blank?2
:open -t -S about:blank?3

And I'm seeing:

Thanks for that insight. This confirms my findings from checking the raw session data.

There is the setting tabs.new_position.tree.new_sibling that controls where new sibling tabs are placed and that indeed defaults to first. So it seems tab 3 is being placed right where it is supposed to be. I respect your surprise though, I don't have mine set to that, but I haven't been using tree tabs for very long. I'll leave it up to you to decide how you want to handle that, either way though this might be some valuable feedback for #8075

I don't want to add behavioral changes to the mix in this PR, as it's about adding characterization tests; and since it's not a bug, I would move the discussion about ergonomics to a separate issue (e.g. #8075).

[pylbrecht]

the tab-next and tab-prev tests have the same setup and assertion. If I can swap one command out for the other, how do I know that the two commands actually have different functionality, they must just be aliases!

It still amazes me how I could have missed that symmetry.
Anyhow, I changed these two tests a bit. Now they have slightly different setups and assertions.

[pylbrecht]

Now the :tab-prev --sibling test is failing for some reason. I will look into it tomorrow.

[pylbrecht]

Debugging was easier than expected.

I think the problem here is that since tab 3 is already focused tab-focus 3 ends up switching away from the active tab to the previous tab

This exact thing bit me again!

---

Moving forward with adding tests for:

• :tab-move
• :tree-tab-promote and :tree-tab-demote
• :tab-give

[pylbrecht]

I played around with :tab-move.

:tab-move start and :tab-move <index> were pretty intuitive. However using the special arguments (+, -, end) tripped me up quite a bit. I still haven't figured out what to expect in which scenario (e.g. calling :tab-move end does not move a tab to the absolute end).

I will need to sit down and document my findings with a fresh mind.

[pylbrecht]

@toofar, whenever you get a chance, this is ready to be reviewed. As usual your feedback is very much appreciated and valued!

Even though I wanted to add more tests, I would like to get this merged now as I find myself lacking motivation to keep digging for more tests. Better to get at least these tests in than to let this PR go stale.

[toofar]

@pylbrecht sorry I managed to miss your message somehow! Will have a look tomorrow.

[toofar]

Thanks, we've got twice as many tests now and they all look good to me!

Would have been nice to have more coverage for tab-move but we can add more later.. I opened #8167 for the issue you found with :tab-move end.

I'll try merging the main branch into the tree-tabs-integration branch too to improve flaky tests.

[pylbrecht]

Would have been nice to have more coverage for tab-move but we can add more later.. I opened #8167 for the issue you found with :tab-move end.

Thanks for opening the issue.
I agree that more tests would have been nice. After taking a motivational refresher with #7407, I might get back to tree tabs. I would like to support your effort to get it out the door!