search

Vrtgs / thirtyfour

Selenium WebDriver client for Rust, for automated testing of websites
Other
1.06k stars 78 forks source link

Adjustable durations with ActionChain actions #242

Closed 0xlunar closed 3 weeks ago

0xlunar commented 1 month ago

Adds implementation for adjustable action_chain durations mentioned in issue #241

Vrtgs commented 3 weeks ago

LGTM

Vrtgs commented 3 weeks ago

or actually, can you make the public API use std::time::Duration, and convert internally using https://doc.rust-lang.org/std/time/struct.Duration.html#method.as_millis and then u64::try_from(milis).ok()

Vrtgs commented 3 weeks ago

Sorry to do this yet again but I think it would make more sense that you use u64::try_from(milis).ok().unwrap_or(u64::MAX) (or just copy the stdlib but with u64, whichever you prefer) as having such a long timeout might as well be like waiting an eternity (I mean I think nothing will be around in 5849424 centuries (u64::MAX milis in centuries))

https://github.com/rust-lang/rust/blob/86d69c705a552236a622eee3fdea94bf13c5f102/library/std/src/sys/pal/windows/mod.rs#L312

I think the note should be updated or omitted

0xlunar commented 3 weeks ago

Sorry to do this yet again but I think it would make more sense that you use u64::try_from(milis).ok().unwrap_or(u64::MAX) (or just copy the stdlib but with u64, whichever you prefer) as having such a long timeout might as well be like waiting an eternity (I mean I think nothing will be around in 5849424 centuries (u64::MAX milis in centuries))

https://github.com/rust-lang/rust/blob/86d69c705a552236a622eee3fdea94bf13c5f102/library/std/src/sys/pal/windows/mod.rs#L312

I think the note should be updated or omitted

It's all goods, happy to contribute. I went with the unwrap_or(u64::MAX) as was simplest to change and seems reasonably clear.