Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A means to test the API via Web Driver #146

Open
marcoscaceres opened this issue Apr 8, 2024 · 9 comments · May be fixed by #170
Open

A means to test the API via Web Driver #146

marcoscaceres opened this issue Apr 8, 2024 · 9 comments · May be fixed by #170
Assignees

Comments

@marcoscaceres
Copy link
Member

marcoscaceres commented Apr 8, 2024

It would neat if the Geolocation API a web driver extension for emitting a GeolocationPosition. A developer could provide an object representing a GeolocationCoordinates (i.e., basically #145), and then the browser would echo that via the API itself.

This would be useful for sites that that need to mock location information and use the actual Geolocation API at the same time. This would work in concert with the ability to request permission via web driver.

@reillyeon
Copy link
Member

Big +1. Consider this support from Chromium. We already have support for this in the Chrome DevTools protocol so it would just need to be hooked up through ChromeDriver: https://pptr.dev/api/puppeteer.page.setgeolocation/

@marcoscaceres
Copy link
Member Author

marcoscaceres commented Apr 9, 2024

Ok, cool... so the question is: does the API "echo" or does it "set" a geolocation position (or both).

I was originally thinking it would "echo", in that one would (not actually suggesting this be called "echoLocation"... just as an example):

await new Promise((resolve, reject) => 
  navigator.geolocation.getCurrentPosition(resolve, reject);
  await test_driver.echoLocation(object);
});

Similarly, with watchPosition(), it would allow you to echo positions as needed:

navigator.geolocation.watchPosition(updateMap);
await test_driver.echoLocation(somePosition);
doSomething();
await test_driver.echoLocation(anotherPosition);

And so on...

Just for completeness, it doesn't look like we need to deal with errors in testing API... I think there are (somewhat convoluted ways) to test for TIMEOUT and POSITION_UNAVAILABLE.

And I guess we would need to figure out what to do when passing bogus positions to the testing API.

@reillyeon
Copy link
Member

When building automation for sensors we found that having it "set" the sensor value makes it easier to write tests because the "echo" behavior is prone to race conditions.

The "set" semantics are also easier to define in terms of the existing specification algorithms as we can hook into the acquire a position steps with "Has an override value been provided, if so return it" rather than introducing language which has to check "are we trying to acquire position? If so, here's a value to return. Otherwise, throw it away?"

@marcoscaceres
Copy link
Member Author

marcoscaceres commented Apr 10, 2024

When building automation for sensors we found that having it "set" the sensor value makes it easier to write tests because the "echo" behavior is prone to race conditions.

Wouldn't WebDriver BiDi account for the race conditions though? (honest question, I assume that's what "BiDi" was supposed to address)

The "set" semantics are also easier to define in terms of the existing specification algorithms as we can hook into the acquire a position steps with "Has an override value been provided, if so return it" rather than introducing language which has to check "are we trying to acquire position? If so, here's a value to return. Otherwise, throw it away?"

I guess "set" works in both cases, so long as this works:

navigator.geolocation.getCurrentPosition(console.log);
await test_driver.setGeolocation(position);

My worry was that the above would be racy (or the .getCurrentPosition call back wouldn't be called because no position is initially set for the page). If that's not the case, then "set" is totally fine and would work the same as echo.

We should confirm that with Puppeteer just in case. Any chance you can do that? Otherwise I can try to find some time to quickly slap something together.

@marcoscaceres
Copy link
Member Author

marcoscaceres commented Apr 10, 2024

Ok, actually... thinking about it... not having the position initially set might be a cute way to force POSITION_ERROR, which I kinda like. We could also then support clearing the current position.

I just tested in Puppeteer, and indeed what I suggested above doesn't work (but it reports the wrong error... it shows a "user denied permission" dialog, which is not right - and the modal blocks the page, so kinda breaks puppeteer too):

If I first set the position for the page, it works fine though.

@marcoscaceres
Copy link
Member Author

Here is the code for test, in case you want to take a look:
https://github.com/marcoscaceres/playground/tree/gh-pages/geo-pup

And here is the test page:
https://marcoscaceres.github.io/playground/geo-pup/

@reillyeon
Copy link
Member

There isn't a Puppeteer API for setting "location unavailable" even though it is actually part of the underlying Chrome DevTools Protocol. Not setting a location at all before the test doesn't work because without being instructed to override the location the browser will try to request it from the OS.

The modal is displayed by your own test page because the browser doesn't have OS-level location permission. There is a Puppeteer API for dismissing alert() dialogs but at that point the test has already gone wrong.

@marcoscaceres
Copy link
Member Author

Does the restrictions you mention affect the requirements of what we are putting together for a Web Driver solution? (it doesn't sound like it, but just want to make sure I didn't miss anything)

@reillyeon
Copy link
Member

It shouldn't cause trouble because tests must always provide an override of either "pretend the device is at the following coordinates" or "pretend the device cannot determine its location".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants