Skip to content

Commit 9a9c327

Browse files
committed
fix flakiness
Signed-off-by: Andrei Gherghescu <[email protected]>
1 parent 6724a78 commit 9a9c327

File tree

3 files changed

+89
-11
lines changed

3 files changed

+89
-11
lines changed

plotly/src/plot.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,12 +1065,10 @@ mod tests {
10651065
assert!(std::fs::remove_file(&dst).is_ok());
10661066
}
10671067

1068-
#[cfg(feature = "plotly_static")]
10691068
// Helper to generate unique ports for parallel tests
1070-
static PORT_COUNTER: AtomicU32 = AtomicU32::new(4444);
1071-
10721069
#[cfg(feature = "plotly_static")]
10731070
fn get_unique_port() -> u32 {
1071+
static PORT_COUNTER: AtomicU32 = AtomicU32::new(5544);
10741072
PORT_COUNTER.fetch_add(1, Ordering::SeqCst)
10751073
}
10761074

plotly_static/src/lib.rs

Lines changed: 87 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ impl StaticExporterBuilder {
599599
///
600600
/// # Examples
601601
///
602-
/// ```rust
602+
/// ```rust,no_run
603603
/// use plotly_static::StaticExporterBuilder;
604604
///
605605
/// let exporter = StaticExporterBuilder::default()
@@ -1047,8 +1047,12 @@ impl AsyncStaticExporter {
10471047

10481048
async fn extract(&mut self, html_content: &str, plot: &PlotData<'_>) -> Result<String> {
10491049
let caps = self.build_webdriver_caps()?;
1050-
debug!("Use WebDriver and headless browser to export static plot");
1050+
debug!(
1051+
"Use WebDriver and headless browser to export static plot (offline_mode={}, port={})",
1052+
self.offline_mode, self.webdriver_port
1053+
);
10511054
let webdriver_url = format!("{}:{}", self.webdriver_url, self.webdriver_port);
1055+
debug!("Connecting to WebDriver at {webdriver_url}");
10521056

10531057
// Reuse existing client or create new one
10541058
let client = if let Some(ref client) = self.webdriver_client {
@@ -1079,6 +1083,71 @@ impl AsyncStaticExporter {
10791083
// Open the HTML
10801084
client.goto(&url).await?;
10811085

1086+
// Ensure DOM is ready and required elements/scripts are available (Windows CI
1087+
// race)
1088+
{
1089+
let start = std::time::Instant::now();
1090+
let timeout = std::time::Duration::from_secs(10);
1091+
loop {
1092+
let state = client
1093+
.execute("return document.readyState;", vec![])
1094+
.await
1095+
.unwrap_or(serde_json::Value::Null);
1096+
if state.as_str().map(|s| s == "complete").unwrap_or(false) {
1097+
break;
1098+
}
1099+
if start.elapsed() > timeout {
1100+
return Err(anyhow!(
1101+
"Timeout waiting for document.readyState === 'complete'"
1102+
));
1103+
}
1104+
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
1105+
}
1106+
}
1107+
1108+
// Wait for Plotly container element
1109+
{
1110+
let start = std::time::Instant::now();
1111+
let timeout = std::time::Duration::from_secs(10);
1112+
loop {
1113+
let has_el = client
1114+
.execute(
1115+
"return !!document.getElementById('plotly-html-element');",
1116+
vec![],
1117+
)
1118+
.await
1119+
.unwrap_or(serde_json::Value::Bool(false));
1120+
if has_el.as_bool().unwrap_or(false) {
1121+
break;
1122+
}
1123+
if start.elapsed() > timeout {
1124+
return Err(anyhow!(
1125+
"Timeout waiting for #plotly-html-element to appear in DOM"
1126+
));
1127+
}
1128+
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
1129+
}
1130+
}
1131+
1132+
// In online mode, ensure Plotly is loaded
1133+
if !self.offline_mode {
1134+
let start = std::time::Instant::now();
1135+
let timeout = std::time::Duration::from_secs(15);
1136+
loop {
1137+
let has_plotly = client
1138+
.execute("return !!window.Plotly;", vec![])
1139+
.await
1140+
.unwrap_or(serde_json::Value::Bool(false));
1141+
if has_plotly.as_bool().unwrap_or(false) {
1142+
break;
1143+
}
1144+
if start.elapsed() > timeout {
1145+
return Err(anyhow!("Timeout waiting for Plotly library to load"));
1146+
}
1147+
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
1148+
}
1149+
}
1150+
10821151
let (js_script, args) = match plot.format {
10831152
ImageFormat::PDF => {
10841153
// Always use SVG for PDF export
@@ -1192,7 +1261,6 @@ impl AsyncStaticExporter {
11921261
#[cfg(test)]
11931262
mod tests {
11941263
use std::path::PathBuf;
1195-
use std::sync::atomic::{AtomicU32, Ordering};
11961264

11971265
use super::*;
11981266

@@ -1201,10 +1269,22 @@ mod tests {
12011269
}
12021270

12031271
// Helper to generate unique ports for parallel tests
1204-
static PORT_COUNTER: AtomicU32 = AtomicU32::new(4444);
1205-
12061272
fn get_unique_port() -> u32 {
1207-
PORT_COUNTER.fetch_add(1, Ordering::SeqCst)
1273+
use std::sync::atomic::{AtomicU32, Ordering};
1274+
static PORT_COUNTER: AtomicU32 = AtomicU32::new(4444);
1275+
1276+
// Before we used this counter to generate unique ports.
1277+
// >>> PORT_COUNTER.fetch_add(1, Ordering::SeqCst)
1278+
// However, sometimes the webdriver process is not stopped immediately
1279+
// and we get port conflicts.
1280+
// We try to give some time for other webdriver processes to stop so that we
1281+
// don't get port conflicts.
1282+
loop {
1283+
let p = PORT_COUNTER.fetch_add(1, Ordering::SeqCst);
1284+
if !webdriver::WebDriver::is_webdriver_running(p) {
1285+
return p;
1286+
}
1287+
}
12081288
}
12091289

12101290
fn create_test_plot() -> serde_json::Value {
@@ -1370,7 +1450,7 @@ mod tests {
13701450

13711451
let mut exporter = StaticExporterBuilder::default()
13721452
.spawn_webdriver(true)
1373-
.webdriver_port(get_unique_port())
1453+
.webdriver_port(5444)
13741454
.build_async()
13751455
.unwrap();
13761456

plotly_static/src/webdriver.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ const WEBDRIVER_BIN: &str = "chromedriver";
3232
/// Default WebDriver port
3333
pub(crate) const WEBDRIVER_PORT: u32 = 4444;
3434
/// Default WebDriver URL
35-
pub(crate) const WEBDRIVER_URL: &str = "http://localhost";
35+
pub(crate) const WEBDRIVER_URL: &str = "http://127.0.0.1";
3636

3737
#[cfg(all(feature = "chromedriver", not(target_os = "windows")))]
3838
pub(crate) fn chrome_default_caps() -> Vec<&'static str> {

0 commit comments

Comments
 (0)