Skip to content

Commit 4205ac5

Browse files
apollo_infra: concurrent tests
1 parent 1b765d5 commit 4205ac5

File tree

5 files changed

+26
-22
lines changed

5 files changed

+26
-22
lines changed

.config/nextest.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ max-threads = 1
77
# serialized tests (mostly integration tests)
88
[[profile.default.overrides]]
99
# TODO(victork): evaluate whether these are indeed required to run sequentially
10-
filter = '(package(apollo_integration_tests) & kind(test)) | package(apollo_infra) | package(apollo_network) | (package(apollo_l1_provider) & kind(test))'
10+
filter = '(package(apollo_integration_tests) & kind(test)) | package(apollo_network) | (package(apollo_l1_provider) & kind(test))'
1111
test-group = 'serialized'
1212

1313
# slow test exception whitelist

crates/apollo_infra/src/tests/concurrent_servers_test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ use crate::component_server::{
3131
RemoteComponentServer,
3232
};
3333
use crate::tests::{
34+
available_ports,
3435
dummy_remote_server_config,
35-
AVAILABLE_PORTS,
3636
TEST_LOCAL_CLIENT_METRICS,
3737
TEST_LOCAL_SERVER_METRICS,
3838
TEST_REMOTE_CLIENT_METRICS,
@@ -157,7 +157,7 @@ async fn setup_concurrent_local_test() -> LocalConcurrentComponentClient {
157157

158158
async fn setup_concurrent_remote_test() -> RemoteConcurrentComponentClient {
159159
let local_client = setup_concurrent_local_test().await;
160-
let socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
160+
let socket = available_ports(7).get_next_local_host_socket();
161161
let remote_client_config = RemoteClientConfig::default();
162162

163163
let max_concurrency = 10;

crates/apollo_infra/src/tests/mod.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,11 @@ use apollo_metrics::metrics::{
1717
MetricScope,
1818
};
1919
use async_trait::async_trait;
20-
use once_cell::sync::Lazy;
2120
use serde::{Deserialize, Serialize};
2221
use starknet_types_core::felt::Felt;
2322
use strum::{EnumVariantNames, VariantNames};
2423
use strum_macros::{AsRefStr, EnumDiscriminants, EnumIter, IntoStaticStr};
25-
use tokio::sync::{Mutex, Semaphore};
24+
use tokio::sync::Semaphore;
2625

2726
use crate::component_client::ClientResult;
2827
use crate::component_definitions::{ComponentRequestHandler, ComponentStarter, PrioritizedRequest};
@@ -185,11 +184,13 @@ const TEST_LOCAL_CLIENT_RESPONSE_TIMES: LabeledMetricHistogram = LabeledMetricHi
185184
pub(crate) const TEST_LOCAL_CLIENT_METRICS: LocalClientMetrics =
186185
LocalClientMetrics::new(&TEST_LOCAL_CLIENT_RESPONSE_TIMES);
187186

188-
// Define the shared fixture
189-
pub static AVAILABLE_PORTS: Lazy<Arc<Mutex<AvailablePorts>>> = Lazy::new(|| {
190-
let available_ports = AvailablePorts::new(TestIdentifier::InfraUnitTests.into(), 0);
191-
Arc::new(Mutex::new(available_ports))
192-
});
187+
/// Creates an `AvailablePorts` instance with a unique `instance_index`.
188+
/// Each test that binds ports should use a different instance_index to get disjoint port ranges.
189+
/// This is necessary because nextest runs each test in its own process, so a shared static
190+
/// counter doesn't work - each process would start from the same base port.
191+
pub fn available_ports(instance_index: u16) -> AvailablePorts {
192+
AvailablePorts::new(TestIdentifier::InfraUnitTests.into(), instance_index)
193+
}
193194

194195
#[derive(Serialize, Deserialize, Clone, AsRefStr, EnumDiscriminants)]
195196
#[strum_discriminants(

crates/apollo_infra/src/tests/remote_component_client_server_test.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ use crate::component_server::{
4141
};
4242
use crate::serde_utils::SerdeWrapper;
4343
use crate::tests::{
44+
available_ports,
4445
dummy_remote_server_config,
4546
test_a_b_functionality,
4647
ComponentA,
@@ -55,7 +56,6 @@ use crate::tests::{
5556
ResultB,
5657
ValueA,
5758
ValueB,
58-
AVAILABLE_PORTS,
5959
TEST_LOCAL_CLIENT_METRICS,
6060
TEST_LOCAL_SERVER_METRICS,
6161
TEST_REMOTE_CLIENT_METRICS,
@@ -135,7 +135,7 @@ async fn create_client_and_faulty_server<T>(body: T) -> ComponentAClient
135135
where
136136
T: Serialize + DeserializeOwned + Debug + Send + Sync + 'static + Clone,
137137
{
138-
let socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
138+
let socket = available_ports(0).get_next_local_host_socket();
139139
task::spawn(async move {
140140
async fn handler<T>(
141141
_http_request: Request<Body>,
@@ -183,8 +183,9 @@ async fn remote_connection_concurrency() {
183183
const MAX_ATTEMPTS: usize = 50;
184184

185185
let setup_value: ValueB = Felt::from(90);
186-
let a_socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
187-
let b_socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
186+
let mut ports = available_ports(1);
187+
let a_socket = ports.get_next_local_host_socket();
188+
let b_socket = ports.get_next_local_host_socket();
188189

189190
// Shared semaphore used inside ComponentA::handle_request
190191
let semaphore = Arc::new(Semaphore::new(0));
@@ -401,8 +402,9 @@ async fn setup_for_tests(
401402
#[tokio::test]
402403
async fn proper_setup() {
403404
let setup_value: ValueB = Felt::from(90);
404-
let a_socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
405-
let b_socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
405+
let mut ports = available_ports(2);
406+
let a_socket = ports.get_next_local_host_socket();
407+
let b_socket = ports.get_next_local_host_socket();
406408

407409
setup_for_tests(setup_value, a_socket, b_socket, MAX_CONCURRENCY, None).await;
408410
let a_client_config = RemoteClientConfig::default();
@@ -426,8 +428,9 @@ async fn proper_setup() {
426428

427429
#[tokio::test]
428430
async fn faulty_client_setup() {
429-
let a_socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
430-
let b_socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
431+
let mut ports = available_ports(3);
432+
let a_socket = ports.get_next_local_host_socket();
433+
let b_socket = ports.get_next_local_host_socket();
431434
// Todo(uriel): Find a better way to pass expected value to the setup
432435
// 123 is some arbitrary value, we don't check it anyway.
433436
setup_for_tests(Felt::from(123), a_socket, b_socket, MAX_CONCURRENCY, None).await;
@@ -461,7 +464,7 @@ async fn faulty_client_setup() {
461464

462465
#[tokio::test]
463466
async fn unconnected_server() {
464-
let socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
467+
let socket = available_ports(4).get_next_local_host_socket();
465468
let client = ComponentAClient::new(
466469
FAST_FAILING_CLIENT_CONFIG,
467470
&socket.ip().to_string(),
@@ -494,7 +497,7 @@ async fn faulty_server(
494497

495498
#[tokio::test]
496499
async fn retry_request() {
497-
let socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
500+
let socket = available_ports(5).get_next_local_host_socket();
498501
// Spawn a server that responses with OK every other request.
499502
task::spawn(async move {
500503
let should_send_ok = Arc::new(Mutex::new(false));

crates/apollo_infra/src/tests/server_metrics_test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ use crate::component_server::{
3333
RemoteComponentServer,
3434
};
3535
use crate::tests::{
36+
available_ports,
3637
dummy_remote_server_config,
37-
AVAILABLE_PORTS,
3838
TEST_LOCAL_CLIENT_METRICS,
3939
TEST_LOCAL_SERVER_METRICS,
4040
TEST_REMOTE_CLIENT_METRICS,
@@ -173,7 +173,7 @@ async fn setup_remote_server_test(
173173
max_concurrency: usize,
174174
) -> (Arc<Semaphore>, RemoteTestComponentClient) {
175175
let (test_sem, local_client) = setup_local_server_test().await;
176-
let socket = AVAILABLE_PORTS.lock().await.get_next_local_host_socket();
176+
let socket = available_ports(6).get_next_local_host_socket();
177177
let config = RemoteClientConfig::default();
178178

179179
let mut remote_server = RemoteComponentServer::new(

0 commit comments

Comments
 (0)