-
Notifications
You must be signed in to change notification settings - Fork 81
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
Resolve webdriver api paths relative to server_url #240
Conversation
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.
when you use impl AsRef<_>
in a function it adds a silent generic, which makes it so the function will be monomorphized for every type of path, this is esspecially bad for async fn and has bigger impact on compile times, as every invocation now has its own complicated state machine
so I would suggest doing what the stdlib already does with things like std::fs::write:
fn foo(p: impl AsRef<Path>) {
fn inner(path: &Path) {
/*code*/
}
inner(p.as_ref())
}
hey I hope you aren't busy but I'm just checking to make sure you see this, so could you implement the changes or should I pull them in then change them my self? |
This reverts commit 27b01aa.
Hi sorry this fell between the cracks. I've reverted the AsRef commit, as it's not relevant to this PR |
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.
lgtm
@Vrtgs not sure I understand that coverage test failure. I don't see any failures in the log |
Sorry for the delay, but here is a fix for #229.
Thirtyfour uses Url::join to resolve url's with an argument that starts with '/'. Given webdriver url "http://localhost:4444/wd/hub" and "/session", Url::join will return http://localhost:4444/session, thus making is incompatible with systems such as selenoid, and selenium versions < 4 which use /wd/hub as a base path.
This PR simply removes the leading slash from the selenium api paths and this makes Url::join resolve the paths as expected.