From 07a72b9682d96b42e09fef722001f2c37e67d537 Mon Sep 17 00:00:00 2001 From: surajk-m Date: Sun, 22 Sep 2024 03:32:34 +0530 Subject: [PATCH 1/5] fix: Encapsulate `Wcmd` within a private wrapper --- src/session.rs | 26 +++++++++++++++----------- tests/local.rs | 2 +- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/session.rs b/src/session.rs index 4af1b7e..2edeb2b 100644 --- a/src/session.rs +++ b/src/session.rs @@ -26,6 +26,10 @@ type Ack = oneshot::Sender>; type Wcmd = WebDriverCommand; +// Wrapper to hide Wcmd from the public API. +#[derive(Debug)] +struct WcmdWrapper(Wcmd); + #[allow(clippy::large_enum_variant)] #[derive(Debug)] pub(crate) enum Cmd { @@ -43,7 +47,7 @@ pub(crate) enum Cmd { WebDriver(Box), } -impl WebDriverCompatibleCommand for Wcmd { +impl WebDriverCompatibleCommand for WcmdWrapper { /// Helper for determining what URL endpoint to use for various requests. /// /// This mapping is essentially that of . @@ -52,16 +56,16 @@ impl WebDriverCompatibleCommand for Wcmd { base_url: &url::Url, session_id: Option<&str>, ) -> Result { - if let WebDriverCommand::NewSession(..) = self { + if let WebDriverCommand::NewSession(..) = self.0 { return base_url.join("session"); } - if let WebDriverCommand::Status = self { + if let WebDriverCommand::Status = self.0 { return base_url.join("status"); } let base = { base_url.join(&format!("session/{}/", session_id.as_ref().unwrap()))? }; - match self { + match self.0 { WebDriverCommand::NewSession(..) => unreachable!(), WebDriverCommand::DeleteSession => unreachable!(), WebDriverCommand::Get(..) | WebDriverCommand::GetCurrentUrl => base.join("url"), @@ -159,7 +163,7 @@ impl WebDriverCompatibleCommand for Wcmd { let mut body = None; // but some have a request body - match self { + match self.0 { WebDriverCommand::NewSession(command::NewSessionParameters::Spec(ref conf)) => { // TODO: awful hacks let mut also = String::new(); @@ -280,12 +284,12 @@ impl WebDriverCompatibleCommand for Wcmd { } fn is_new_session(&self) -> bool { - matches!(self, WebDriverCommand::NewSession(..)) + matches!(self.0, WebDriverCommand::NewSession(..)) } fn is_legacy(&self) -> bool { matches!( - self, + self.0, WebDriverCommand::NewSession(webdriver::command::NewSessionParameters::Legacy(..)), ) } @@ -293,7 +297,7 @@ impl WebDriverCompatibleCommand for Wcmd { impl From for Cmd { fn from(o: Wcmd) -> Self { - Cmd::WebDriver(Box::new(o)) + Cmd::WebDriver(Box::new(WcmdWrapper(o))) } } @@ -333,10 +337,10 @@ impl Client { /// Issue the specified [`WebDriverCompatibleCommand`] to the WebDriver instance. pub async fn issue_cmd( - &self, - cmd: impl WebDriverCompatibleCommand + Send + 'static, + &self, + cmd: Wcmd ) -> Result { - self.issue(Cmd::WebDriver(Box::new(cmd))).await + self.issue(Cmd::WebDriver(Box::new(WcmdWrapper(cmd)))).await } pub(crate) fn is_legacy(&self) -> bool { diff --git a/tests/local.rs b/tests/local.rs index 3611cb7..82c39d9 100644 --- a/tests/local.rs +++ b/tests/local.rs @@ -425,7 +425,7 @@ async fn dynamic_commands(c: Client, port: u16) -> Result<(), error::CmdError> { c.goto(&sample_url).await?; let title = c.issue_cmd(WebDriverCommand::GetTitle).await?; assert_eq!(title.as_str(), Some("Sample Page")); - let title = c.issue_cmd(Box::new(WebDriverCommand::GetTitle)).await?; + let title = c.issue_cmd(*Box::new(WebDriverCommand::GetTitle)).await?; assert_eq!(title.as_str(), Some("Sample Page")); Ok(()) } From 6c19c8016957b66f52bda9c70f05bfbe1f121dee Mon Sep 17 00:00:00 2001 From: surajk-m Date: Sun, 22 Sep 2024 03:46:28 +0530 Subject: [PATCH 2/5] cargo fmt --- src/session.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/session.rs b/src/session.rs index 2edeb2b..ece29db 100644 --- a/src/session.rs +++ b/src/session.rs @@ -336,10 +336,7 @@ impl Client { } /// Issue the specified [`WebDriverCompatibleCommand`] to the WebDriver instance. - pub async fn issue_cmd( - &self, - cmd: Wcmd - ) -> Result { + pub async fn issue_cmd(&self, cmd: Wcmd) -> Result { self.issue(Cmd::WebDriver(Box::new(WcmdWrapper(cmd)))).await } From f3d43da0df305e416f207f0172c28a2d21d3df28 Mon Sep 17 00:00:00 2001 From: surajk-m Date: Sat, 19 Oct 2024 20:39:43 +0530 Subject: [PATCH 3/5] Add `test_wrap_command` to wrap `WebDriverCommand` in tests --- Cargo.lock | 1 + Cargo.toml | 2 ++ src/lib.rs | 3 +++ src/session.rs | 19 +++++++++++++++++-- tests/local.rs | 10 +++++++--- 5 files changed, 30 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bd051b0..829cf08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -241,6 +241,7 @@ version = "0.21.2" dependencies = [ "base64 0.22.1", "cookie 0.18.1", + "fantoccini", "futures-core", "futures-util", "http 1.1.0", diff --git a/Cargo.toml b/Cargo.toml index 803f2e0..4d3f379 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ license = "MIT OR Apache-2.0" default = ["native-tls"] native-tls = ["hyper-tls", "openssl"] rustls-tls = ["hyper-rustls"] +test_helpers = [] [dependencies] webdriver = { version = "0.50", default-features = false } @@ -47,6 +48,7 @@ tokio = { version = "1", features = ["full"] } hyper = { version = "1.1.0", features = ["server"] } hyper-util = { version = "0.1.3", features = ["server", "http1"] } serial_test = "3.0" +fantoccini = { path = "../fantoccini", features = ["test_helpers"] } # for minimal-versions [target.'cfg(any())'.dependencies] diff --git a/src/lib.rs b/src/lib.rs index 9cfd098..b2d125b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -266,3 +266,6 @@ pub mod wait; pub mod wd; #[doc(inline)] pub use wd::Locator; + +#[cfg(any(test, feature = "test_helpers"))] +pub use crate::session::test_wrap_command; diff --git a/src/session.rs b/src/session.rs index ece29db..10ec7a5 100644 --- a/src/session.rs +++ b/src/session.rs @@ -30,6 +30,18 @@ type Wcmd = WebDriverCommand; #[derive(Debug)] struct WcmdWrapper(Wcmd); +/// Wraps a WebDriverCommand inside a WcmdWrapper and returns it as a +/// Box. +/// +/// This helper function is intended for use in tests where internal +/// WebDriver commands need to be wrapped in the private `WcmdWrapper` +#[cfg(any(test, feature = "test_helpers"))] +pub fn test_wrap_command( + cmd: WebDriverCommand, +) -> Box { + Box::new(WcmdWrapper(cmd)) +} + #[allow(clippy::large_enum_variant)] #[derive(Debug)] pub(crate) enum Cmd { @@ -336,8 +348,11 @@ impl Client { } /// Issue the specified [`WebDriverCompatibleCommand`] to the WebDriver instance. - pub async fn issue_cmd(&self, cmd: Wcmd) -> Result { - self.issue(Cmd::WebDriver(Box::new(WcmdWrapper(cmd)))).await + pub async fn issue_cmd( + &self, + cmd: Box, + ) -> Result { + self.issue(Cmd::WebDriver(cmd)).await } pub(crate) fn is_legacy(&self) -> bool { diff --git a/tests/local.rs b/tests/local.rs index 82c39d9..0d1bfe2 100644 --- a/tests/local.rs +++ b/tests/local.rs @@ -1,7 +1,7 @@ //! Tests that don't make use of external websites. use crate::common::{other_page_url, sample_page_url}; use fantoccini::wd::TimeoutConfiguration; -use fantoccini::{error, Client, Locator}; +use fantoccini::{error, test_wrap_command, Client, Locator}; use http_body_util::BodyExt; use hyper::Method; use serial_test::serial; @@ -423,9 +423,13 @@ async fn timeouts(c: Client, _: u16) -> Result<(), error::CmdError> { async fn dynamic_commands(c: Client, port: u16) -> Result<(), error::CmdError> { let sample_url = sample_page_url(port); c.goto(&sample_url).await?; - let title = c.issue_cmd(WebDriverCommand::GetTitle).await?; + let title = c + .issue_cmd(test_wrap_command(WebDriverCommand::GetTitle)) + .await?; assert_eq!(title.as_str(), Some("Sample Page")); - let title = c.issue_cmd(*Box::new(WebDriverCommand::GetTitle)).await?; + let title = c + .issue_cmd(test_wrap_command(WebDriverCommand::GetTitle)) + .await?; assert_eq!(title.as_str(), Some("Sample Page")); Ok(()) } From e26d58e98951f23699ff2740f875c5d09c1fb4b2 Mon Sep 17 00:00:00 2001 From: surajk-m Date: Mon, 28 Oct 2024 12:39:43 +0530 Subject: [PATCH 4/5] Update `issue_cmd` for boxed and non-boxed command types - Remove `test_wrap_command` and `test_helpers` feature - Refactor `issue_cmd` to accept impl `Into>` - Introduce a `GetTitle` struct that implements `WebDriverCompatibleCommand` --- Cargo.lock | 1 - Cargo.toml | 2 -- src/lib.rs | 3 --- src/session.rs | 16 ++-------------- tests/local.rs | 47 ++++++++++++++++++++++++++++++++++++++--------- 5 files changed, 40 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 829cf08..bd051b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -241,7 +241,6 @@ version = "0.21.2" dependencies = [ "base64 0.22.1", "cookie 0.18.1", - "fantoccini", "futures-core", "futures-util", "http 1.1.0", diff --git a/Cargo.toml b/Cargo.toml index 4d3f379..803f2e0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,7 +22,6 @@ license = "MIT OR Apache-2.0" default = ["native-tls"] native-tls = ["hyper-tls", "openssl"] rustls-tls = ["hyper-rustls"] -test_helpers = [] [dependencies] webdriver = { version = "0.50", default-features = false } @@ -48,7 +47,6 @@ tokio = { version = "1", features = ["full"] } hyper = { version = "1.1.0", features = ["server"] } hyper-util = { version = "0.1.3", features = ["server", "http1"] } serial_test = "3.0" -fantoccini = { path = "../fantoccini", features = ["test_helpers"] } # for minimal-versions [target.'cfg(any())'.dependencies] diff --git a/src/lib.rs b/src/lib.rs index b2d125b..9cfd098 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -266,6 +266,3 @@ pub mod wait; pub mod wd; #[doc(inline)] pub use wd::Locator; - -#[cfg(any(test, feature = "test_helpers"))] -pub use crate::session::test_wrap_command; diff --git a/src/session.rs b/src/session.rs index 10ec7a5..d24d10f 100644 --- a/src/session.rs +++ b/src/session.rs @@ -30,18 +30,6 @@ type Wcmd = WebDriverCommand; #[derive(Debug)] struct WcmdWrapper(Wcmd); -/// Wraps a WebDriverCommand inside a WcmdWrapper and returns it as a -/// Box. -/// -/// This helper function is intended for use in tests where internal -/// WebDriver commands need to be wrapped in the private `WcmdWrapper` -#[cfg(any(test, feature = "test_helpers"))] -pub fn test_wrap_command( - cmd: WebDriverCommand, -) -> Box { - Box::new(WcmdWrapper(cmd)) -} - #[allow(clippy::large_enum_variant)] #[derive(Debug)] pub(crate) enum Cmd { @@ -350,9 +338,9 @@ impl Client { /// Issue the specified [`WebDriverCompatibleCommand`] to the WebDriver instance. pub async fn issue_cmd( &self, - cmd: Box, + cmd: impl Into>, ) -> Result { - self.issue(Cmd::WebDriver(cmd)).await + self.issue(Cmd::WebDriver(cmd.into())).await } pub(crate) fn is_legacy(&self) -> bool { diff --git a/tests/local.rs b/tests/local.rs index 0d1bfe2..c34a74f 100644 --- a/tests/local.rs +++ b/tests/local.rs @@ -1,16 +1,49 @@ //! Tests that don't make use of external websites. use crate::common::{other_page_url, sample_page_url}; -use fantoccini::wd::TimeoutConfiguration; -use fantoccini::{error, test_wrap_command, Client, Locator}; +use fantoccini::wd::{TimeoutConfiguration, WebDriverCompatibleCommand}; +use fantoccini::{error, Client, Locator}; use http_body_util::BodyExt; use hyper::Method; use serial_test::serial; use std::time::Duration; use url::Url; -use webdriver::command::WebDriverCommand; mod common; +#[derive(Debug)] +struct GetTitle; + +// Implement `WebDriverCompatibleCommand` for `GetTitle` +impl WebDriverCompatibleCommand for GetTitle { + fn endpoint( + &self, + base_url: &url::Url, + session_id: Option<&str>, + ) -> Result { + let base = base_url.join(&format!("session/{}/", session_id.unwrap()))?; + base.join("title") + } + + fn method_and_body(&self, _: &url::Url) -> (http::Method, Option) { + (http::Method::GET, None) + } + + fn is_new_session(&self) -> bool { + false + } + + fn is_legacy(&self) -> bool { + false + } +} + +// Implement `Into>` for `GetTitle` +impl Into> for GetTitle { + fn into(self) -> Box { + Box::new(self) + } +} + async fn goto(c: Client, port: u16) -> Result<(), error::CmdError> { let url = sample_page_url(port); c.goto(&url).await?; @@ -423,13 +456,9 @@ async fn timeouts(c: Client, _: u16) -> Result<(), error::CmdError> { async fn dynamic_commands(c: Client, port: u16) -> Result<(), error::CmdError> { let sample_url = sample_page_url(port); c.goto(&sample_url).await?; - let title = c - .issue_cmd(test_wrap_command(WebDriverCommand::GetTitle)) - .await?; + let title = c.issue_cmd(GetTitle).await?; assert_eq!(title.as_str(), Some("Sample Page")); - let title = c - .issue_cmd(test_wrap_command(WebDriverCommand::GetTitle)) - .await?; + let title = c.issue_cmd(GetTitle).await?; assert_eq!(title.as_str(), Some("Sample Page")); Ok(()) } From 0dd402db4a0fbdac178b7b701319e2e520f266b5 Mon Sep 17 00:00:00 2001 From: surajk-m Date: Tue, 29 Oct 2024 13:17:09 +0530 Subject: [PATCH 5/5] Use `` generics in `issue_cmd` with `C: Into>` --- src/session.rs | 9 +++++---- tests/local.rs | 15 --------------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/src/session.rs b/src/session.rs index d24d10f..cbb8faa 100644 --- a/src/session.rs +++ b/src/session.rs @@ -336,10 +336,11 @@ impl Client { } /// Issue the specified [`WebDriverCompatibleCommand`] to the WebDriver instance. - pub async fn issue_cmd( - &self, - cmd: impl Into>, - ) -> Result { + pub async fn issue_cmd(&self, cmd: C) -> Result + where + C: Into>, + CC: WebDriverCompatibleCommand + Send + 'static, + { self.issue(Cmd::WebDriver(cmd.into())).await } diff --git a/tests/local.rs b/tests/local.rs index c34a74f..1fe26af 100644 --- a/tests/local.rs +++ b/tests/local.rs @@ -27,21 +27,6 @@ impl WebDriverCompatibleCommand for GetTitle { fn method_and_body(&self, _: &url::Url) -> (http::Method, Option) { (http::Method::GET, None) } - - fn is_new_session(&self) -> bool { - false - } - - fn is_legacy(&self) -> bool { - false - } -} - -// Implement `Into>` for `GetTitle` -impl Into> for GetTitle { - fn into(self) -> Box { - Box::new(self) - } } async fn goto(c: Client, port: u16) -> Result<(), error::CmdError> {