Skip to content

Commit 21ab88f

Browse files
sanityclaude
andauthored
fix(test): use port retry instead of serial execution for blocked_peers tests (#2254)
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 051af68 commit 21ab88f

File tree

5 files changed

+162
-14
lines changed

5 files changed

+162
-14
lines changed

Cargo.lock

Lines changed: 41 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

apps/freenet-ping/Cargo.lock

Lines changed: 41 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

apps/freenet-ping/app/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ humantime = "2.2.0"
2626

2727
[dev-dependencies]
2828
freenet = { path = "../../../crates/core" }
29+
serial_test = "3"
2930
test-log = "0.2"
3031
testresult = { workspace = true }
3132

apps/freenet-ping/app/tests/common/mod.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,14 +330,16 @@ fn compile_contract(contract_path: &PathBuf) -> anyhow::Result<Vec<u8>> {
330330
&BuildToolConfig {
331331
features: None,
332332
package_type: PackageType::Contract,
333-
debug: true,
333+
// Use release builds - debug WASM is ~12MB vs ~186KB for release,
334+
// which exceeds WebSocket message size limits when serialized
335+
debug: false,
334336
},
335337
contract_path,
336338
)?;
337339

338340
let output_file = Path::new(&target)
339341
.join(WASM_TARGET)
340-
.join("debug")
342+
.join("release")
341343
.join(WASM_FILE_NAME.replace('-', "_"))
342344
.with_extension("wasm");
343345
println!("output file: {output_file:?}");

apps/freenet-ping/app/tests/run_app_blocked_peers.rs

Lines changed: 75 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -91,18 +91,73 @@ struct BlockedPeersConfig {
9191
subscribe_immediately: bool,
9292
}
9393

94-
/// Runs a blocked peers test with the provided configuration
94+
/// Maximum number of retries when port allocation fails
95+
const MAX_PORT_RETRY_ATTEMPTS: usize = 5;
96+
97+
/// Runs a blocked peers test with the provided configuration.
98+
/// Wraps the test in a retry loop to handle port allocation race conditions.
9599
async fn run_blocked_peers_test(config: BlockedPeersConfig) -> TestResult {
100+
let mut last_error: Option<anyhow::Error> = None;
101+
102+
for attempt in 1..=MAX_PORT_RETRY_ATTEMPTS {
103+
match run_blocked_peers_test_inner(&config, attempt).await {
104+
Ok(()) => return Ok(()),
105+
Err(e) => {
106+
let error_str = format!("{:#}", e);
107+
// Check if this is a port collision error (EADDRINUSE)
108+
if error_str.contains("Address already in use")
109+
|| error_str.contains("os error 98")
110+
|| error_str.contains("os error 48")
111+
{
112+
// macOS: 48, Linux: 98
113+
tracing::warn!(
114+
attempt,
115+
MAX_PORT_RETRY_ATTEMPTS,
116+
"Port collision detected, retrying with new ports: {}",
117+
error_str
118+
);
119+
last_error = Some(e);
120+
// Small delay before retry to let ports be released
121+
tokio::time::sleep(Duration::from_millis(100)).await;
122+
continue;
123+
}
124+
// Non-retryable error - convert to TestResult which will panic
125+
return Err(e.into());
126+
}
127+
}
128+
}
129+
130+
// All retries exhausted
131+
Err(last_error
132+
.unwrap_or_else(|| {
133+
anyhow!(
134+
"Port allocation failed after {} attempts",
135+
MAX_PORT_RETRY_ATTEMPTS
136+
)
137+
})
138+
.into())
139+
}
140+
141+
/// Inner implementation of blocked peers test (returns anyhow::Result for error inspection)
142+
async fn run_blocked_peers_test_inner(
143+
config: &BlockedPeersConfig,
144+
attempt: usize,
145+
) -> anyhow::Result<()> {
96146
if config.verbose_logging {
97147
std::env::set_var(
98148
"RUST_LOG",
99149
"debug,freenet::operations::subscribe=trace,freenet::contract=trace",
100150
);
101151
}
102152

103-
tracing::info!("Starting {} blocked peers test...", config.test_name);
153+
tracing::info!(
154+
"Starting {} blocked peers test (attempt {}/{})...",
155+
config.test_name,
156+
attempt,
157+
MAX_PORT_RETRY_ATTEMPTS
158+
);
104159

105-
// Network setup
160+
// Network setup - get all ports upfront
106161
let network_socket_gw = TcpListener::bind("127.0.0.1:0")?;
107162
let gw_network_port = network_socket_gw.local_addr()?.port();
108163

@@ -118,14 +173,20 @@ async fn run_blocked_peers_test(config: BlockedPeersConfig) -> TestResult {
118173
let node2_network_port = network_socket_node2.local_addr()?.port();
119174
let node2_network_addr = SocketAddr::new(Ipv4Addr::LOCALHOST.into(), node2_network_port);
120175

121-
// Configure gateway
176+
// Configure gateway - include attempt number in dir suffix for isolation
177+
let dir_suffix = if attempt > 1 {
178+
format!("{}_{}", config.test_name, attempt)
179+
} else {
180+
config.test_name.to_string()
181+
};
182+
122183
let (config_gw, preset_cfg_gw, config_gw_info) = {
123184
let (cfg, preset) = base_node_test_config(
124185
true, // is_gateway
125186
vec![],
126187
Some(gw_network_port),
127188
ws_api_port_socket_gw.local_addr()?.port(),
128-
&format!("gw_{}", config.test_name),
189+
&format!("gw_{}", dir_suffix),
129190
None,
130191
None, // Gateway doesn't block any peers
131192
)
@@ -143,7 +204,7 @@ async fn run_blocked_peers_test(config: BlockedPeersConfig) -> TestResult {
143204
vec![serde_json::to_string(&config_gw_info)?],
144205
Some(node1_network_port),
145206
ws_api_port_socket_node1.local_addr()?.port(),
146-
&format!("node1_{}", config.test_name),
207+
&format!("node1_{}", dir_suffix),
147208
None,
148209
Some(vec![node2_network_addr]), // Node1 blocks Node2
149210
)
@@ -156,7 +217,7 @@ async fn run_blocked_peers_test(config: BlockedPeersConfig) -> TestResult {
156217
vec![serde_json::to_string(&config_gw_info)?],
157218
Some(node2_network_port),
158219
ws_api_port_socket_node2.local_addr()?.port(),
159-
&format!("node2_{}", config.test_name),
220+
&format!("node2_{}", dir_suffix),
160221
None,
161222
Some(vec![node1_network_addr]), // Node2 blocks Node1
162223
)
@@ -170,6 +231,8 @@ async fn run_blocked_peers_test(config: BlockedPeersConfig) -> TestResult {
170231
tracing::info!("Node 2 blocks: {:?}", node1_network_addr);
171232

172233
// Free socket resources before starting nodes
234+
// NOTE: There's a race window here where another process could grab the port.
235+
// This is handled by the retry loop in run_blocked_peers_test().
173236
std::mem::drop(network_socket_gw);
174237
std::mem::drop(ws_api_port_socket_gw);
175238
std::mem::drop(network_socket_node1);
@@ -761,15 +824,15 @@ async fn run_blocked_peers_test(config: BlockedPeersConfig) -> TestResult {
761824
select! {
762825
gw = gateway_node => {
763826
let Err(gw) = gw;
764-
Err(anyhow!("Gateway node failed: {}", gw).into())
827+
Err(anyhow!("Gateway node failed: {}", gw))
765828
}
766829
n1 = node1 => {
767830
let Err(n1) = n1;
768-
Err(anyhow!("Node 1 failed: {}", n1).into())
831+
Err(anyhow!("Node 1 failed: {}", n1))
769832
}
770833
n2 = node2 => {
771834
let Err(n2) = n2;
772-
Err(anyhow!("Node 2 failed: {}", n2).into())
835+
Err(anyhow!("Node 2 failed: {}", n2))
773836
}
774837
t = test => {
775838
match t {
@@ -779,11 +842,11 @@ async fn run_blocked_peers_test(config: BlockedPeersConfig) -> TestResult {
779842
}
780843
Ok(Err(e)) => {
781844
tracing::error!("Test failed: {}", e);
782-
Err(e.into())
845+
Err(e)
783846
}
784847
Err(_) => {
785848
tracing::error!("Test timed out!");
786-
Err(anyhow!("Test timed out").into())
849+
Err(anyhow!("Test timed out"))
787850
}
788851
}
789852
}

0 commit comments

Comments
 (0)