Fixes #76 broken selector in TitleBar.getItem()

[vaclavHala]

Closes #76

[christian-bromann]

Amazing, thank you 👍

[christian-bromann]

It seems like the locator is not working for VS Code Ubuntu, can you verify? I would recommend to use https://github.com/stateful/vscode-server-action to inspect the pipelines.

[vaclavHala]

Hmm this may be trickier than I originally thought.

Apparently the title bar looks different in each environment:

• In my locally installed vscode (where all the tests pass) the titlebar is regular native menu bar with all items (File, Edit etc.) listed as top level menu entries
• In stable, insiders and 1.75 the native bar is also used, but all the items are collapsed under single top-level item More (displayed as three lines icon) - I've no idea how vscode decides how to show the titlebar, it seems to always show the collapsed one when run in xvfb, even when I run the exact same version I have installed locally
• In web the native bar does not exist at all so the titlebar is embedded in the left side bar, and all items are also hidden under More

So I'm not sure how you want the API for this to look - should the API expose these differences, forcing users to write the test a bit differently in each environment, or should it hide these differences (i.e. bypass the More item if present and look for the whole widget in different place when in web) to make writing tests simpler (but there may be some cases where this automagic would cause trouble...)?

[christian-bromann]

should the API expose these differences, forcing users to write the test a bit differently in each environment

The idea of the page object API is to allow to just use a single interface for all environments. So ideally it would be great if we could be flexible here, e.g. check if the menu bar is collapsed and if so click on it first to get all items. If it is not possible at it should throw an error that the titlebar is not available for that environment.

Do you think we can do something like this?

[vaclavHala]

Sure, that's how I'd like it to behave too.

As for the implementation, I think it would be best to determine if we need to expand the overflow first by looking at .menubar under elem$ of TitleBar which will have class .overflow-menu-only when titlebar is too narrow to show all items on the top level. Trouble is none of this is available in any of the locators/*.ts files which I suppose means these classes are off-limits?

[christian-bromann]

Trouble is none of this is available in any of the locators/*.ts files which I suppose means these classes are off-limits?

Feel free to add locators as you need, just because they are not in there doesn't mean they don't exist in VS Code.

[vaclavHala]

So I finally made some progress on this, a few points that came up while working on it:

I had to fix (or was this intentional? doesn't seem like it) ContextMenuItem.select to return the nested menu like TitleBarItem.select does.

I also had to add same sleeps as are present in the ContextMenu into TitleBar to wait for menu to open.
I'm not happy about this solution and would prefer some waitUntil(menu open) in both cases, but that would be too many changes for this PR.

Lastly I'm not sure if it is intentional that ContextMenuItem.isNesting spins for quite a long time if the menu does not actually have submenu,
because clicking that menu item will close the whole menu and the selector never finds it.
Maybe the isNesting check should be evaluated and stored into variable for later before the item is clicked
and rewriten in such a way it just looks at classes of the item (which is already open and so guaranteed to be found quick) to see if it has the .submenu.indicator.
In this PR I just moved the isNesting check before clicking the menu because the long delay it imposes was causing effect of the click (e.g. when it opens notification) to disappear before the isNesting check ends

Also, I'm not sure how to handle testing in CI of all the various states the titlebar can be in as this relies on width of the vscode window.
In adition to the ones I already showed in the screenshot above there is a third one

• if at least 5 items fit but not all items fix, the first 5 are shown top level and rest is hidden under last More overflow item
  I tested the change in this PR works in all 3 possible titlebar states in the custom mode by manually resizing the window before running the tests, but that is tedious and not automated. In the native mode the toolbar items can't be acessed at all so there is clear error message for that case

[christian-bromann]

I had to fix (or was this intentional? doesn't seem like it) ContextMenuItem.select to return the nested menu like TitleBarItem.select does.

VS Code might have changed some selectors which caused this to break and it seems we never had thorough tests for that Page Object.

I also had to add same sleeps as are present in the ContextMenu into TitleBar to wait for menu to open.
I'm not happy about this solution and would prefer some waitUntil(menu open) in both cases, but that would be too many changes for this PR.

Happy to review bigger PRs too or additional PRs at a later point. Let's make sure the code we add uses waitUntil though if possible.

In adition to the ones I already showed in the screenshot above there is a third one

I am happy to have certain context menu types to be "not supported". I understand that trying to provide solid interfaces for all environments and use case to be impossible to maintain. Therefor let's go with the approach of whatever helps to unblock you in your project we can implement.

[christian-bromann]

Thank you for this fix. I will go ahead and merge. 👍