fix(browser): passing options to hover/unhover

[mneveroff]

Description

This PR resolves the issue with context.userEvent not sending the options for hover and unhover BrowserCommands.

It also tidies up the context.provider usage in commands (click, tab) to match the rest of the commands.

1. Calls to context userEvent
2. Console.log:
   • options received in command (node_modules) - you can see that click gets the options, but hover doesn't
   • coordinates received by component (mousemove handler) - you can see it defaulting to middle of the screen on hover
3. Change to context.js (node_modules)
4. Now hover correctly passes the options to the browser command from userEvent.

P.S. When running tests I got × test/file-watching.test.ts > adding a new test file triggers re-run 20539ms failing, all the other tests pass, lmk how I can work on troubleshooting that.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• [] It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
• [] Ideally, include a test that fails without this PR but passes with it.
• Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

• Run the tests with pnpm test:ci.

Documentation

• [] If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

• Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

[userquin]

why are you changing click and tab modules?

[mneveroff]

why are you changing click and tab modules?

Consistency with other commands, only those two seem to assign provider as a variable to be used, all others rely on context.provider. If it's intentional and/or you feel it should stay that way I'm happy to revert that aspect of the change.

[sheremet-va]

Would also be nice to have a test covering this to catch the regression in the future

[mneveroff]

Yeah, I agree, but atm I'm not in a position to contribute that bit as well, still very new to the vitest's codebase. If that's a blocker then I'd appreciate some assistance on getting this covered.

[sheremet-va]

There is a test/browser/test/userEvent.test.ts file. You just need to add a test that makes sure the position is correct when hovering the element (using addEventListener most likely)

[sheremet-va]

There is already test with hover/unhover - I think just checking the cursor position in the even should be enough

[mneveroff]

So what was expected to be a simple test turned out to be not so. After a few hours and experimenting with repo's settings around ui: false, headless a.s.o, as well as running the suite via regex pattern I arrived at no conclusive result.

Case in point:

1. Whilst running the whole file and ui: true (default), the results are rather nonsensical, and seem to be static. I think it might have something to do with pointer not being reset correctly between events, and moving into the target element from unexpected angle, and/or screenX including some of the Vitest's UI in it's coordinates.
2. Second is more silly, as it's only off by one, and with no browser interface to interfere my only guess is that iframe introduces fractional css height / offset.
3. The last one passes, as it runs only that suite in isolation.

Overall, whilst I appreciate wanting to have this change covered, the bug seems to fall under strict fix, so unless there's something obvious that I'm missing to fix the test easily, I'm not convinced that lack thereof should be blocking this bug fix?

P.S. We could create a suite that would spy on the invoked functions, but it'd be rather close to testing the implementation, rather then behaviour, IMO.
P.P.S. No idea why test started failing in that way in CI, locally they're all green.

[sheremet-va]

I think this just proves that we need to spend more time working on how coordinates are reported. Maybe we need to hide the UI while the test is running for example

We can merge this PR if there is a test that checks that at least one option is working correctly except positions - maybe modifiers?

[mneveroff]

Yeah I agree regarding the reporting bit and covering / understanding it better, but neither my bandwidth nor understanding of vitest's UI / browser suite is anywhere near enough sufficient to accomplish that at the moment.

I have pushed a change that asserts shift and control modifier keys.

All tests pass locally via pnpm run test:ci but I'm not sure why the browser ones in CI are failing. I don't think that I introduced new suites, so not certain I understand why the number of tests ran is off by one against the expected. It seems like many recent PRs are failing those.

[sheremet-va]

test:ci doesn't run browser tests. test:browser:webdriverio and test:browser:playwright do

In your case, it seems like you need to limit the test to playwright because webdriverio doesn't support modifiers. Your test also seems weird to me because you override shiftModifier and controlModifier

If you don't have time, I am fine with merging this PR without tests for now, but with a // todo to test options

[mneveroff]

Yeah fair, must've been too late, found the suites in test/browser.

Kinda see what you mean regarding redeclaring variables, just based it off the existing test for hover/unhover. I separated it into a standalone, playwright-only test (preview also doesn't pass options IIRC, but I also see a few more tests failing with preview, presuming it's intended) - good catch!

Not sure why Firefox is unhappy on Windows in CI, lmk if there's something I'm missing. Playwright doesn't seem to have any notes about options support in Firefox.

[sheremet-va]

The click test is flaky for some reason, I don't think the fail is related to your test

[netlify]

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	c1f4769
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/66a2d693551f080008a4a518
😎 Deploy Preview	https://deploy-preview-6175--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.