Vrtgs/thirtyfour

Expose default timeouts to public

Closed this issue ยท 7 comments

Hi @stevepryde what do you thing to make a public constant or exposed method for a default TimeoutConfiguration?

It would be useful to be able to obtain a default one to be able to switch back after a usage of thirtyfour_query and set_implicit_wait_timeout

https://github.com/stevepryde/thirtyfour/blob/83c91ce6106f4b3150e0ec83cac3c4c548606123/src/webdrivercommands.rs#L78-L83

By the way it would be also helpful to have some clean method after usage of thirtyfour_query to not have a boilerplate code with a remove of ElementPoller from config and setting default timeouts.

What do you think?

I think TimeoutConfiguration::default() could be used as the default (both internally and also people can use it to infer the defaults later if needed).

As I've said before, the plan with thirtyfour_query was to merge it into thirtyfour and make it a standard feature. It's probably time to make that happen. I'm not sure how to deal with timeouts though. The easiest way is to just set the default implicit wait to 0 and mention it in the docs. This is a breaking change but in my opinion it is a better default because implicit waits are not as convenient (i.e. when verifying that an element doesn't exist). There are also some other changes in the current main branch that are also breaking changes so now is as good a time as any. It probably needs a warning label in the documentation.

I think I may have misunderstood your later comment. Were you thinking that WebDriverConfig should have a clean() method or similar? or perhaps that method could exist on WebDriver itself?

I've implemented Default for TimeoutConfiguration. I also added a way to clear the WebDriverConfig, but it requires mutable access to WebDriver which may pose a challenge. I don't see this as an urgent thing since in the majority of cases it's better to end the session and create a new browser instance rather than try to keep one session open and reuse it.

However if we want to allow the configuration to be updated at any time, then I would suggest putting the entire WebDriverSession inside a mutex i.e. Arc<Mutex<WebDriverSession>> . That should work ok I think. Actually a RwLock may be better since config writes will be quite rare.

I think I may have misunderstood your later comment.

I was just thinking that things we need to set up to use a thirtyfour_query and to switch to default behavior could be hidden by some functions.

But never mind ๐Ÿ˜‰ don't worry to much about it

Regarding removing boilerplate from the thirtyfour_query setup, I've now merged thirtyfour_query into the main branch, so it's available out-of-the-box without any setup. The default implicit wait time is now 0 and the default poller is ElementPoller::TimeoutWithInterval(Duration::from_secs(20), Duration::from_millis(500)) (using ElementPoller::default()). You can set a different poller via WebDriver::set_query_poller()

This means you can now use WebDriver::query(), WebElement::query() and WebElement::wait_until() without any boilerplate setup and it will just work. The traits are also part of the prelude so you don't need to use them specifically.

I suspect the existence of find_element() etc alongside query() may confuse some people though. I'm not really sure how to address that. I may need to make a distinction between lower vs higher level abstractions within WebDriver/WebElement. My preference is to steer people towards the higher-level abstractions but still allow access to the lower-level ones if needed and for those who know what they're doing.

I haven't published v0.25 yet.

looking forward 0.25 :)