vscode: Smoke tests: revisit timeouts

But it has this comment to explain the reasoning

// ¯\_(ツ)_/¯

The search ones are interesting. They go before a click, and when I remove them, I think I see the issue that we see in #137195 - the button appears hovered but the click hasn’t taken effect. Like playwright can’t click a button unless it waits a bit. I checked that the click handler is added by the time the button is in the DOM

The problem with clicks is that they are not atomic: they are always a pair of mousedown and a mouseup events, on the same coordinate. But the DOM can always change between both events.

Imagine if the click happens during the time when the actionbar is adding its actions. The mousedown can go on the action, but the mouseup outside of it. That would explain why the action appears hovered. It also explains why waiting helps: it’s waiting for the action bar to finish adding all the actions. It also aligns with the fact that the ReplaceAllAction is inserted into the DOM first, and RemoveAction comes second, giving the UI automation time to click the first action. Finally, it also doesn’t help that actions are right-aligned, which means that if the mousedown event occurs between both actions go into the DOM, it will hit the ReplaceAllAction but the mouseup event will hit the RemoveAction instead. That’s my theory, tho it could be way off.

In other words: // ¯\_(ツ)_/¯ because at the time I had had enough with UI automation issues like these. I would appreciate fresh ideas on tackling stuff like this.

I was able to remove the double click usage as it was only used from the explorer and no one was calling that openFile method.

For the playwright driver we should be using page.mouse.dblclick: https://playwright.dev/docs/api/class-mouse#mouse-dblclick

One of the failures when checkWindowReady timeout is removed shows the search happening before the remote connection is finished:

Looks like the remote connection check doesn’t apply in web:

https://github.com/Microsoft/vscode/blob/27e38a8e2714b061c1e1647c553ab017f41b99a4/test/automation/src/application.ts#L115-L117

So the timeout was sometimes allowing that to succeed.

Removed the timeouts in the search smoketests. https://github.com/microsoft/vscode/commit/1341ca3d7f69e794478f172d876170ad91f5b268

Now I clear the previous search results before starting a new search, wait for the search to finish before trying to click in the tree, and will retry the click if it doesn’t work.

Trying to remove the checkWindowReady timeout in https://github.com/microsoft/vscode/pull/138516, will see if it passes.

Thanks @joaomoreno. Actually what I think is happening is that the search tree is rerendered multiple times quickly. We find the element once then try to click it as it’s being rerendered. Dunno how a smoke test can wait for it to settle without a timeout, but updating the tree to use diffIdentityProvider which I’ve been meaning to do might help.

But how can an editor be opened on startup unless the test is really for verifying that an editor restores after restart, which only really my data migration tests should do? Every smoke test otherwise should open in a fresh instance, with no prior state, no?