Skip to content

Commit 22e3d78

Browse files
committed
reuse session in webdriver
Signed-off-by: Andrei Gherghescu <[email protected]>
1 parent 03629c0 commit 22e3d78

File tree

5 files changed

+78
-39
lines changed

5 files changed

+78
-39
lines changed

.github/workflows/ci.yml

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,12 @@ jobs:
107107
-ChromeDriverPath "${{ steps.setup-chrome.outputs.chromedriver-path }}"
108108
109109
# Run tests on non-Windows platforms
110-
- name: Run tests (non-Windows)
110+
- name: Run tests (${{ matrix.os }})
111111
if: matrix.os != 'windows-latest'
112112
run: cargo test --verbose --workspace --features plotly_ndarray,plotly_image,static_export_default --exclude plotly_kaleido
113113

114114
# Run tests on Windows with Chrome WebDriver
115-
- name: Run tests (Windows)
115+
- name: Run tests (${{ matrix.os }})
116116
if: matrix.os == 'windows-latest'
117117
shell: pwsh
118118
run: |
@@ -127,12 +127,8 @@ jobs:
127127
Write-Host "WEBDRIVER_PATH: $env:WEBDRIVER_PATH"
128128
Write-Host "CHROME_PATH: $env:CHROME_PATH"
129129
130-
cargo test --verbose --workspace --features plotly_ndarray,plotly_image,chromedriver --exclude plotly_kaleido
130+
cargo test --verbose --workspace --features plotly_ndarray,plotly_image,static_export_chromedriver --exclude plotly_kaleido
131131
132-
# Windows-specific cleanup
133-
- if: matrix.os == 'windows-latest'
134-
run: gci -recurse -filter "*example*"
135-
136132
code-coverage:
137133
name: Code Coverage
138134
runs-on: ubuntu-latest

plotly/src/configuration.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ pub enum ImageButtonFormats {
1010
Webp,
1111
}
1212

13-
// TODO: should this be behind the plotly-kaleido feature?
1413
#[serde_with::skip_serializing_none]
1514
#[derive(Serialize, Debug, Default, Clone)]
1615
pub struct ToImageButtonOptions {

plotly/src/plot.rs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -831,13 +831,6 @@ mod tests {
831831
use super::*;
832832
use crate::Scatter;
833833

834-
// Helper to generate unique ports for parallel tests
835-
static PORT_COUNTER: AtomicU32 = AtomicU32::new(4444);
836-
837-
fn get_unique_port() -> u32 {
838-
PORT_COUNTER.fetch_add(1, Ordering::SeqCst)
839-
}
840-
841834
fn create_test_plot() -> Plot {
842835
let trace1 = Scatter::new(vec![0, 1, 2], vec![6, 10, 2]).name("trace1");
843836
let mut plot = Plot::new();
@@ -988,6 +981,15 @@ mod tests {
988981
assert!(!dst.exists());
989982
}
990983

984+
#[cfg(feature = "plotly_static")]
985+
// Helper to generate unique ports for parallel tests
986+
static PORT_COUNTER: AtomicU32 = AtomicU32::new(4444);
987+
988+
#[cfg(feature = "plotly_static")]
989+
fn get_unique_port() -> u32 {
990+
PORT_COUNTER.fetch_add(1, Ordering::SeqCst)
991+
}
992+
991993
#[test]
992994
#[cfg(feature = "plotly_static")]
993995
fn save_to_png() {
@@ -1003,8 +1005,8 @@ mod tests {
10031005
let metadata = std::fs::metadata(&dst).expect("Could not retrieve file metadata");
10041006
let file_size = metadata.len();
10051007
assert!(file_size > 0,);
1006-
// assert!(std::fs::remove_file(&dst).is_ok());
1007-
// assert!(!dst.exists());
1008+
assert!(std::fs::remove_file(&dst).is_ok());
1009+
assert!(!dst.exists());
10081010
}
10091011

10101012
#[test]

plotly_static/build.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,13 +336,12 @@ fn main() -> Result<()> {
336336
} else {
337337
#[cfg(feature = "chromedriver")]
338338
{
339-
let msg = format!("'webdriver_download' feature disabled. Please install a {CHROMEDRIVER_PATH_ENV} version manually and make the environment variable 'WEBDRIVER_PATH' point to it.");
339+
let msg = "'webdriver_download' feature disabled. Please install a 'chromedriver' version manually and make the environment variable 'WEBDRIVER_PATH' point to it.".to_string();
340340
println!("cargo::warning={msg}");
341341
}
342342
#[cfg(feature = "geckodriver")]
343343
{
344-
let msg = format!("'webdriver_download' feature disabled. Please install a {} version manually and make the environment variable 'WEBDRIVER_PATH' point to it.",
345-
GECKODRIVER_PATH_ENV);
344+
let msg = format!("'webdriver_download' feature disabled. Please install a 'geckodriver' version manually and make the environment variable 'WEBDRIVER_PATH' point to it.");
346345
println!("cargo::warning={msg}");
347346
}
348347
#[cfg(not(any(feature = "chromedriver", feature = "geckodriver")))]

plotly_static/src/lib.rs

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ use std::{println as error, println as warn, println as debug};
266266

267267
use anyhow::{anyhow, Context, Result};
268268
use base64::{engine::general_purpose, Engine as _};
269-
use fantoccini::{wd::Capabilities, ClientBuilder};
269+
use fantoccini::{wd::Capabilities, Client, ClientBuilder};
270270
#[cfg(not(test))]
271271
use log::{debug, error, warn};
272272
use serde::Serialize;
@@ -426,9 +426,8 @@ impl std::fmt::Display for ImageFormat {
426426
}
427427
}
428428

429-
// TODO: how to avoid cyclic dependency on the ImageFormat and the Plot data
430-
// ideally ImageFormat will be defined in a single place and the `data` field
431-
// would be just a Plot object which is later serialized to JSON
429+
/// TODO: ideally data would be a Plot object which is later serialized to JSON
430+
/// but with the current workspace set up, that would be a cyclic dependency.
432431
#[derive(Serialize)]
433432
struct PlotData<'a> {
434433
format: ImageFormat,
@@ -469,10 +468,15 @@ struct PlotData<'a> {
469468
/// - Browser capabilities: Default Chrome/Firefox headless options
470469
/// - Automatic WebDriver detection and connection reuse
471470
pub struct StaticExporterBuilder {
471+
/// WebDriver server port (default: 4444)
472472
webdriver_port: u32,
473+
/// WebDriver server base URL (default: "http://localhost")
473474
webdriver_url: String,
475+
/// Auto-spawn WebDriver if not running (default: true)
474476
spawn_webdriver: bool,
477+
/// Use bundled JS libraries instead of CDN (default: false)
475478
offline_mode: bool,
479+
/// Browser command-line flags (e.g., "--headless", "--no-sandbox")
476480
webdriver_browser_caps: Vec<String>,
477481
}
478482

@@ -675,6 +679,7 @@ impl StaticExporterBuilder {
675679
offline_mode: self.offline_mode,
676680
webdriver_browser_caps: self.webdriver_browser_caps.clone(),
677681
runtime,
682+
webdriver_client: None,
678683
})
679684
}
680685

@@ -737,12 +742,26 @@ impl StaticExporterBuilder {
737742
/// - Offline mode support
738743
/// - Automatic WebDriver management
739744
pub struct StaticExporter {
745+
/// WebDriver server port (default: 4444)
740746
webdriver_port: u32,
747+
748+
/// WebDriver server base URL (default: "http://localhost")
741749
webdriver_url: String,
750+
751+
/// WebDriver process manager for spawning and cleanup
742752
webdriver: WebDriver,
753+
754+
/// Use bundled JS libraries instead of CDN
743755
offline_mode: bool,
756+
757+
/// Browser command-line flags (e.g., "--headless", "--no-sandbox")
744758
webdriver_browser_caps: Vec<String>,
759+
760+
/// Tokio runtime for async operations
745761
runtime: std::sync::Arc<tokio::runtime::Runtime>,
762+
763+
/// Cached WebDriver client for session reuse
764+
webdriver_client: Option<Client>,
746765
}
747766

748767
impl Drop for StaticExporter {
@@ -757,6 +776,16 @@ impl Drop for StaticExporter {
757776
/// - Leaves externally managed WebDriver sessions running
758777
/// - Logs errors but doesn't panic if cleanup fails
759778
fn drop(&mut self) {
779+
// Close the WebDriver client if it exists
780+
if let Some(client) = self.webdriver_client.take() {
781+
let runtime = self.runtime.clone();
782+
runtime.block_on(async {
783+
if let Err(e) = client.close().await {
784+
error!("Failed to close WebDriver client: {e}");
785+
}
786+
});
787+
}
788+
760789
// Stop the WebDriver process
761790
if let Err(e) = self.webdriver.stop() {
762791
error!("Failed to stop WebDriver: {e}");
@@ -941,11 +970,20 @@ impl StaticExporter {
941970
debug!("Use WebDriver and headless browser to export static plot");
942971
let webdriver_url = format!("{}:{}", self.webdriver_url, self.webdriver_port,);
943972

944-
let client = ClientBuilder::native()
945-
.capabilities(caps)
946-
.connect(&webdriver_url)
947-
.await
948-
.with_context(|| "WebDriver session errror")?;
973+
// Reuse existing client or create new one
974+
let client = if let Some(ref client) = self.webdriver_client {
975+
debug!("Reusing existing WebDriver session");
976+
client.clone()
977+
} else {
978+
debug!("Creating new WebDriver session");
979+
let new_client = ClientBuilder::native()
980+
.capabilities(caps)
981+
.connect(&webdriver_url)
982+
.await
983+
.with_context(|| "WebDriver session error")?;
984+
self.webdriver_client = Some(new_client.clone());
985+
new_client
986+
};
949987

950988
// URL-encode the HTML
951989
let data_uri = format!("data:text/html,{}", encode(data_uri));
@@ -978,7 +1016,8 @@ impl StaticExporter {
9781016

9791017
let data = client.execute_async(js_script, args).await?;
9801018

981-
client.close().await?;
1019+
// Don't close the client - keep it for reuse
1020+
// client.close().await?;
9821021

9831022
let src = data.as_str().ok_or(anyhow!(
9841023
"Failed to execute Plotly.toImage in browser session"
@@ -1282,46 +1321,50 @@ mod tests {
12821321
init();
12831322
let test_plot = create_test_plot();
12841323

1324+
// Use a unique port to test actual WebDriver process reuse
1325+
let test_port = get_unique_port();
1326+
12851327
// Create first exporter - this should spawn a new WebDriver
12861328
let mut export1 = StaticExporterBuilder::default()
12871329
.spawn_webdriver(true)
1288-
.webdriver_port(get_unique_port())
1330+
.webdriver_port(test_port)
12891331
.build()
12901332
.unwrap();
12911333

12921334
// Export first image
1293-
let dst1 = PathBuf::from("session_reuse_1.png");
1335+
let dst1 = PathBuf::from("process_reuse_1.png");
12941336
export1
12951337
.write_fig(dst1.as_path(), &test_plot, ImageFormat::PNG, 800, 600, 1.0)
12961338
.unwrap();
12971339
assert!(dst1.exists());
12981340
assert!(std::fs::remove_file(dst1.as_path()).is_ok());
12991341

13001342
// Create second exporter on the same port - this should connect to existing
1301-
// WebDriver
1343+
// WebDriver process (but create a new session)
13021344
let mut export2 = StaticExporterBuilder::default()
13031345
.spawn_webdriver(true)
1304-
.webdriver_port(get_unique_port())
1346+
.webdriver_port(test_port)
13051347
.build()
13061348
.unwrap();
13071349

1308-
// Export second image using the same WebDriver session
1309-
let dst2 = PathBuf::from("session_reuse_2.png");
1350+
// Export second image using a new session on the same WebDriver process
1351+
let dst2 = PathBuf::from("process_reuse_2.png");
13101352
export2
13111353
.write_fig(dst2.as_path(), &test_plot, ImageFormat::PNG, 800, 600, 1.0)
13121354
.unwrap();
13131355
assert!(dst2.exists());
13141356
assert!(std::fs::remove_file(dst2.as_path()).is_ok());
13151357

13161358
// Create third exporter on the same port - should also connect to existing
1359+
// WebDriver process
13171360
let mut export3 = StaticExporterBuilder::default()
13181361
.spawn_webdriver(true)
1319-
.webdriver_port(get_unique_port())
1362+
.webdriver_port(test_port)
13201363
.build()
13211364
.unwrap();
13221365

1323-
// Export third image
1324-
let dst3 = PathBuf::from("session_reuse_3.png");
1366+
// Export third image using another new session on the same WebDriver process
1367+
let dst3 = PathBuf::from("process_reuse_3.png");
13251368
export3
13261369
.write_fig(dst3.as_path(), &test_plot, ImageFormat::PNG, 800, 600, 1.0)
13271370
.unwrap();

0 commit comments

Comments
 (0)