-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
webdriver: Generalize implicit_wait
#40784
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
Conversation
yezhizhen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dear reviewers,
Please just check the first commit. The change is large because of fmt.
Best,
Euclid
components/webdriver_server/lib.rs
Outdated
| where | ||
| T: for<'de> Deserialize<'de> + Serialize, | ||
| callback: impl Fn() -> Result<T, WebDriverError>, | ||
| can_early_return: impl Fn(&T) -> bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm. Another option here to simplify things is to make it so that callback is callback: impl Fn() -> Result<Option<T>, WebDriverError> and ignore None results. That should give you more flexibility with how you implement the callback because it could also depend on some other kind of state in a more straight-forward way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried a while. It seems Option is hard in this case. But bool make things easy.
0d2706f to
168469b
Compare
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
168469b to
0e22d4b
Compare
| where | ||
| T: for<'de> Deserialize<'de> + Serialize, | ||
| { | ||
| callback: impl Fn() -> Result<(bool, T), WebDriverError>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting bool first to avoid potential cloning of T.
implicit_waitimplicit_wait
I wanted to implement implicit wait for other commands, such as send key/element clear/click. But the current trait bound, parameters, and logic are way too tight and specific.
Testing: This should not change existing behaviour.