webdriverio-community / wdio-vscode-service

A service to test VSCode extensions from end to end using WebdriverIO
https://webdriverio-community.github.io/wdio-vscode-service/
MIT License
33 stars 28 forks source link

Fixes #76 broken selector in TitleBar.getItem() #77

Closed vaclavHala closed 1 year ago

vaclavHala commented 1 year ago

Closes #76

christian-bromann commented 1 year ago

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 commented 1 year ago

Hmm this may be trickier than I originally thought.

Apparently the title bar looks different in each environment: wdio_titlebar

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

christian-bromann commented 1 year ago

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.