Skip to content

Commit 840653e

Browse files
committed
improve webdriver detection/reuse
Signed-off-by: Andrei Gherghescu <[email protected]>
1 parent 6089357 commit 840653e

File tree

7 files changed

+318
-93
lines changed

7 files changed

+318
-93
lines changed

plotly/src/plot.rs

Lines changed: 85 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,7 @@ impl PartialEq for Plot {
803803
#[cfg(test)]
804804
mod tests {
805805
use std::path::PathBuf;
806+
use std::sync::atomic::{AtomicU32, Ordering};
806807

807808
#[cfg(feature = "kaleido")]
808809
use plotly_kaleido::ImageFormat;
@@ -815,6 +816,13 @@ mod tests {
815816
use super::*;
816817
use crate::Scatter;
817818

819+
// Helper to generate unique ports for parallel tests
820+
static PORT_COUNTER: AtomicU32 = AtomicU32::new(4444);
821+
822+
fn get_unique_port() -> u32 {
823+
PORT_COUNTER.fetch_add(1, Ordering::SeqCst)
824+
}
825+
818826
fn create_test_plot() -> Plot {
819827
let trace1 = Scatter::new(vec![0, 1, 2], vec![6, 10, 2]).name("trace1");
820828
let mut plot = Plot::new();
@@ -961,18 +969,25 @@ mod tests {
961969
let dst = PathBuf::from("example.html");
962970
plot.write_html(&dst);
963971
assert!(dst.exists());
964-
// assert!(std::fs::remove_file(&dst).is_ok());
965-
// assert!(!dst.exists());
972+
assert!(std::fs::remove_file(&dst).is_ok());
973+
assert!(!dst.exists());
966974
}
967975

968976
#[test]
969977
#[cfg(feature = "plotly_static")]
970978
fn save_to_png() {
971979
let plot = create_test_plot();
972980
let dst = PathBuf::from("example.png");
973-
plot.write_image(&dst, ImageFormat::PNG, 1024, 680, 1.0)
981+
let mut exporter = plotly_static::StaticExporterBuilder::default()
982+
.webdriver_port(get_unique_port())
983+
.build()
984+
.unwrap();
985+
plot.write_image_with_exporter(&mut exporter, &dst, ImageFormat::PNG, 1024, 680, 1.0)
974986
.unwrap();
975987
assert!(dst.exists());
988+
let metadata = std::fs::metadata(&dst).expect("Could not retrieve file metadata");
989+
let file_size = metadata.len();
990+
assert!(file_size > 0,);
976991
// assert!(std::fs::remove_file(&dst).is_ok());
977992
// assert!(!dst.exists());
978993
}
@@ -982,55 +997,89 @@ mod tests {
982997
fn save_to_jpeg() {
983998
let plot = create_test_plot();
984999
let dst = PathBuf::from("example.jpeg");
985-
plot.write_image(&dst, ImageFormat::JPEG, 1024, 680, 1.0)
1000+
let mut exporter = plotly_static::StaticExporterBuilder::default()
1001+
.webdriver_port(get_unique_port())
1002+
.build()
1003+
.unwrap();
1004+
plot.write_image_with_exporter(&mut exporter, &dst, ImageFormat::JPEG, 1024, 680, 1.0)
9861005
.unwrap();
9871006
assert!(dst.exists());
988-
// assert!(std::fs::remove_file(&dst).is_ok());
989-
// assert!(!dst.exists());
1007+
let metadata = std::fs::metadata(&dst).expect("Could not retrieve file metadata");
1008+
let file_size = metadata.len();
1009+
assert!(file_size > 0,);
1010+
assert!(std::fs::remove_file(&dst).is_ok());
1011+
assert!(!dst.exists());
9901012
}
9911013

9921014
#[test]
9931015
#[cfg(feature = "plotly_static")]
9941016
fn save_to_svg() {
9951017
let plot = create_test_plot();
9961018
let dst = PathBuf::from("example.svg");
997-
plot.write_image(&dst, ImageFormat::SVG, 1024, 680, 1.0)
1019+
let mut exporter = plotly_static::StaticExporterBuilder::default()
1020+
.webdriver_port(get_unique_port())
1021+
.build()
1022+
.unwrap();
1023+
plot.write_image_with_exporter(&mut exporter, &dst, ImageFormat::SVG, 1024, 680, 1.0)
9981024
.unwrap();
9991025
assert!(dst.exists());
1000-
// assert!(std::fs::remove_file(&dst).is_ok());
1001-
// assert!(!dst.exists());
1026+
let metadata = std::fs::metadata(&dst).expect("Could not retrieve file metadata");
1027+
let file_size = metadata.len();
1028+
assert!(file_size > 0,);
1029+
assert!(std::fs::remove_file(&dst).is_ok());
1030+
assert!(!dst.exists());
10021031
}
10031032

10041033
#[test]
10051034
#[cfg(feature = "plotly_static")]
10061035
fn save_to_pdf() {
10071036
let plot = create_test_plot();
10081037
let dst = PathBuf::from("example.pdf");
1009-
plot.write_image(&dst, ImageFormat::PDF, 1024, 680, 1.0)
1038+
let mut exporter = plotly_static::StaticExporterBuilder::default()
1039+
.webdriver_port(get_unique_port())
1040+
.build()
1041+
.unwrap();
1042+
plot.write_image_with_exporter(&mut exporter, &dst, ImageFormat::PDF, 1024, 680, 1.0)
10101043
.unwrap();
10111044
assert!(dst.exists());
1012-
// assert!(std::fs::remove_file(&dst).is_ok());
1013-
// assert!(!dst.exists());
1045+
let metadata = std::fs::metadata(&dst).expect("Could not retrieve file metadata");
1046+
let file_size = metadata.len();
1047+
assert!(file_size > 0,);
1048+
assert!(std::fs::remove_file(&dst).is_ok());
1049+
assert!(!dst.exists());
10141050
}
10151051

10161052
#[test]
10171053
#[cfg(feature = "plotly_static")]
10181054
fn save_to_webp() {
10191055
let plot = create_test_plot();
10201056
let dst = PathBuf::from("example.webp");
1021-
plot.write_image(&dst, ImageFormat::WEBP, 1024, 680, 1.0)
1057+
let mut exporter = plotly_static::StaticExporterBuilder::default()
1058+
.webdriver_port(get_unique_port())
1059+
.build()
1060+
.unwrap();
1061+
plot.write_image_with_exporter(&mut exporter, &dst, ImageFormat::WEBP, 1024, 680, 1.0)
10221062
.unwrap();
10231063
assert!(dst.exists());
1024-
// assert!(std::fs::remove_file(&dst).is_ok());
1025-
// assert!(!dst.exists());
1064+
let metadata = std::fs::metadata(&dst).expect("Could not retrieve file metadata");
1065+
let file_size = metadata.len();
1066+
assert!(file_size > 0,);
1067+
assert!(std::fs::remove_file(&dst).is_ok());
1068+
assert!(!dst.exists());
10261069
}
10271070

10281071
#[test]
10291072
#[cfg(feature = "plotly_static")]
10301073
fn image_to_base64() {
10311074
let plot = create_test_plot();
1075+
let mut exporter = plotly_static::StaticExporterBuilder::default()
1076+
.webdriver_port(get_unique_port())
1077+
.build()
1078+
.unwrap();
10321079

1033-
let image_base64 = plot.to_base64(ImageFormat::PNG, 200, 150, 1.0).unwrap();
1080+
let image_base64 = plot
1081+
.to_base64_with_exporter(&mut exporter, ImageFormat::PNG, 200, 150, 1.0)
1082+
.unwrap();
10341083

10351084
assert!(!image_base64.is_empty());
10361085

@@ -1048,7 +1097,13 @@ mod tests {
10481097
#[cfg(feature = "plotly_static")]
10491098
fn image_to_svg_string() {
10501099
let plot = create_test_plot();
1051-
let image_svg = plot.to_svg(200, 150, 1.0).unwrap();
1100+
let mut exporter = plotly_static::StaticExporterBuilder::default()
1101+
.webdriver_port(get_unique_port())
1102+
.build()
1103+
.unwrap();
1104+
let image_svg = plot
1105+
.to_svg_with_exporter(&mut exporter, 200, 150, 1.0)
1106+
.unwrap();
10521107

10531108
assert!(!image_svg.is_empty());
10541109

@@ -1078,13 +1133,22 @@ mod tests {
10781133

10791134
plot.add_trace(surface);
10801135
let dst = PathBuf::from("example.png");
1081-
plot.write_image(&dst, ImageFormat::PNG, 800, 600, 1.0)
1136+
let mut exporter = plotly_static::StaticExporterBuilder::default()
1137+
.webdriver_port(get_unique_port())
1138+
.build()
1139+
.unwrap();
1140+
plot.write_image_with_exporter(&mut exporter, &dst, ImageFormat::PNG, 800, 600, 1.0)
10821141
.unwrap();
10831142
assert!(dst.exists());
1084-
// assert!(std::fs::remove_file(&dst).is_ok());
1085-
// assert!(!dst.exists());
1143+
1144+
let metadata = std::fs::metadata(&dst).expect("Could not retrieve file metadata");
1145+
let file_size = metadata.len();
1146+
assert!(file_size > 0,);
1147+
assert!(std::fs::remove_file(&dst).is_ok());
1148+
assert!(!dst.exists());
1149+
10861150
assert!(!plot
1087-
.to_base64(ImageFormat::PNG, 1024, 680, 1.0)
1151+
.to_base64_with_exporter(&mut exporter, ImageFormat::PNG, 1024, 680, 1.0)
10881152
.unwrap()
10891153
.is_empty());
10901154
}

plotly_static/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ fantoccini = "0.21"
2727
tokio = { version = "1", features = ["full"] }
2828
anyhow = "1.0"
2929
urlencoding = "2"
30+
reqwest = { version = "0.11", features = ["blocking"] }
3031

3132
[dev-dependencies]
3233
plotly_static = { path = "." }

plotly_static/build.rs

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -65,23 +65,22 @@ fn is_webdriver_available(env_var: &str, executable_name: &str) -> bool {
6565
// First check environment variable path
6666
if let Ok(path) = env::var(env_var) {
6767
let exe_path = if cfg!(windows) && !path.to_lowercase().ends_with(".exe") {
68-
format!("{}{}", path, DRIVER_EXT)
68+
format!("{path}{DRIVER_EXT}")
6969
} else {
7070
path
7171
};
7272
let exe = Path::new(&exe_path);
7373
if exe.exists() && exe.is_file() {
7474
println!(
75-
"{} found at path specified in {}: {}",
76-
executable_name, env_var, exe_path
75+
"{executable_name} found at path specified in {env_var}: {exe_path}"
7776
);
7877
return true;
7978
}
8079
}
8180

8281
// Check if webdriver exists in user's bin directory
8382
let bin_dir = user_bin_dir();
84-
let bin_path = bin_dir.join(format!("{}{}", executable_name, DRIVER_EXT));
83+
let bin_path = bin_dir.join(format!("{executable_name}{DRIVER_EXT}"));
8584
if bin_path.exists() && bin_path.is_file() {
8685
println!(
8786
"{} found in user's bin directory: {}",
@@ -116,11 +115,9 @@ async fn download_with_retry(
116115
last_error = Some(e);
117116
attempts += 1;
118117
if attempts < MAX_DOWNLOAD_RETRIES {
119-
let delay =
120-
Duration::from_secs(INITIAL_RETRY_DELAY * 2u64.pow(attempts - 1));
118+
let delay = Duration::from_secs(INITIAL_RETRY_DELAY * 2u64.pow(attempts - 1));
121119
println!(
122-
"cargo:warning=Download attempt {} failed, retrying in {:?}...",
123-
attempts, delay
120+
"cargo:warning=Download attempt {attempts} failed, retrying in {delay:?}..."
124121
);
125122
sleep(delay).await;
126123
}
@@ -140,12 +137,10 @@ fn setup_driver(config: &WebdriverDownloadConfig) -> Result<()> {
140137
return Ok(());
141138
}
142139
println!(
143-
"cargo::warning=You can specify {} or {} to an existing installation to avoid downloads.",
144-
GECKODRIVER_PATH_ENV, CHROMEDRIVER_PATH_ENV
140+
"cargo::warning=You can specify {GECKODRIVER_PATH_ENV} or {CHROMEDRIVER_PATH_ENV} to an existing installation to avoid downloads."
145141
);
146142
println!(
147-
"cargo::warning=You can override browser detection using {} or {} environment variables.",
148-
CHROME_PATH_ENV, FIREFOX_PATH_ENV
143+
"cargo::warning=You can override browser detection using {CHROME_PATH_ENV} or {FIREFOX_PATH_ENV} environment variables."
149144
);
150145

151146
println!(
@@ -162,14 +157,12 @@ fn setup_driver(config: &WebdriverDownloadConfig) -> Result<()> {
162157
)
163158
})?;
164159
println!(
165-
"cargo::warning=Browser version detected: {}",
166-
browser_version
160+
"cargo::warning=Browser version detected: {browser_version}"
167161
);
168162

169163
let webdriver_bin_dir = user_bin_dir();
170164
println!(
171-
"cargo::warning=Driver will be installed in: {:?}",
172-
webdriver_bin_dir
165+
"cargo::warning=Driver will be installed in: {webdriver_bin_dir:?}"
173166
);
174167

175168
fs::create_dir_all(&webdriver_bin_dir).with_context(|| {
@@ -256,8 +249,7 @@ fn get_chrome_path() -> Result<PathBuf> {
256249
} else {
257250
Err(anyhow!("Chrome not found on path: {}", chrome_path)).with_context(|| {
258251
format!(
259-
"Please set {} to a valid Chrome installation",
260-
CHROME_PATH_ENV
252+
"Please set {CHROME_PATH_ENV} to a valid Chrome installation"
261253
)
262254
})
263255
}
@@ -271,8 +263,7 @@ fn get_chrome_path() -> Result<PathBuf> {
271263
} else {
272264
Err(anyhow!("Chrome browser not detected")).with_context(|| {
273265
format!(
274-
"Use {} to point to a valid Chrome installation",
275-
CHROME_PATH_ENV
266+
"Use {CHROME_PATH_ENV} to point to a valid Chrome installation"
276267
)
277268
})
278269
}
@@ -352,8 +343,7 @@ fn main() -> Result<()> {
352343
setup_driver(&config)?;
353344
}
354345
} else {
355-
let msg = format!("'webdriver_download' feature disabled. Please install a {} or {} version manually and make the environment variable 'WEBDRIVER_PATH' point to it.",
356-
GECKODRIVER_PATH_ENV, CHROMEDRIVER_PATH_ENV);
346+
let msg = format!("'webdriver_download' feature disabled. Please install a {GECKODRIVER_PATH_ENV} or {CHROMEDRIVER_PATH_ENV} version manually and make the environment variable 'WEBDRIVER_PATH' point to it.");
357347
println!("cargo::warning={msg}");
358348
}
359349
Ok(())

plotly_static/examples/main.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use std::io::{self, Read};
33
use std::path::PathBuf;
44

55
use clap::{Parser, ValueEnum};
6-
use env_logger;
76
use log::info;
87
use plotly_static::{ImageFormat, StaticExporterBuilder};
98
use serde_json::Value;

0 commit comments

Comments
 (0)