Skip to content

Commit 5e54ee1

Browse files
karthiknadigCopilot
andcommitted
test: isolate missing-env reporting state (Fixes #500)
Inject the atomic state into state-machine helpers so parallel configure tests cannot corrupt missing-env reporting tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b618197 commit 5e54ee1

1 file changed

Lines changed: 74 additions & 40 deletions

File tree

crates/pet/src/jsonrpc.rs

Lines changed: 74 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -908,17 +908,29 @@ fn is_current_generation(
908908
fn try_begin_missing_env_reporting(
909909
configuration: &RwLock<ConfigurationState>,
910910
refresh_generation: u64,
911+
) -> bool {
912+
try_begin_missing_env_reporting_with_state(
913+
&MISSING_ENVS_REPORTING_STATE,
914+
configuration,
915+
refresh_generation,
916+
)
917+
}
918+
919+
fn try_begin_missing_env_reporting_with_state(
920+
reporting_state: &AtomicU64,
921+
configuration: &RwLock<ConfigurationState>,
922+
refresh_generation: u64,
911923
) -> bool {
912924
loop {
913-
let current_state = MISSING_ENVS_REPORTING_STATE.load(Ordering::Acquire);
925+
let current_state = reporting_state.load(Ordering::Acquire);
914926
if current_state == MISSING_ENVS_COMPLETED {
915927
return false;
916928
}
917929
if current_state != MISSING_ENVS_AVAILABLE && current_state >= refresh_generation {
918930
return false;
919931
}
920932

921-
if MISSING_ENVS_REPORTING_STATE
933+
if reporting_state
922934
.compare_exchange(
923935
current_state,
924936
refresh_generation,
@@ -931,7 +943,11 @@ fn try_begin_missing_env_reporting(
931943
return true;
932944
}
933945

934-
release_missing_env_reporting_if_stale(configuration, refresh_generation);
946+
release_missing_env_reporting_if_stale_with_state(
947+
reporting_state,
948+
configuration,
949+
refresh_generation,
950+
);
935951
return false;
936952
}
937953
}
@@ -940,9 +956,21 @@ fn try_begin_missing_env_reporting(
940956
fn release_missing_env_reporting_if_stale(
941957
configuration: &RwLock<ConfigurationState>,
942958
refresh_generation: u64,
959+
) {
960+
release_missing_env_reporting_if_stale_with_state(
961+
&MISSING_ENVS_REPORTING_STATE,
962+
configuration,
963+
refresh_generation,
964+
);
965+
}
966+
967+
fn release_missing_env_reporting_if_stale_with_state(
968+
reporting_state: &AtomicU64,
969+
configuration: &RwLock<ConfigurationState>,
970+
refresh_generation: u64,
943971
) {
944972
if !is_current_generation(configuration, refresh_generation) {
945-
let _ = MISSING_ENVS_REPORTING_STATE.compare_exchange(
973+
let _ = reporting_state.compare_exchange(
946974
refresh_generation,
947975
MISSING_ENVS_AVAILABLE,
948976
Ordering::AcqRel,
@@ -952,14 +980,17 @@ fn release_missing_env_reporting_if_stale(
952980
}
953981

954982
fn complete_missing_env_reporting(refresh_generation: u64) {
955-
let _ = MISSING_ENVS_REPORTING_STATE.compare_exchange(
983+
complete_missing_env_reporting_with_state(&MISSING_ENVS_REPORTING_STATE, refresh_generation);
984+
}
985+
986+
fn complete_missing_env_reporting_with_state(reporting_state: &AtomicU64, refresh_generation: u64) {
987+
let _ = reporting_state.compare_exchange(
956988
refresh_generation,
957989
MISSING_ENVS_COMPLETED,
958990
Ordering::AcqRel,
959991
Ordering::Acquire,
960992
);
961993
}
962-
963994
fn execute_refresh(
964995
context: &Context,
965996
refresh_options: &RefreshOptions,
@@ -1414,8 +1445,6 @@ mod tests {
14141445
telemetry: Mutex<Vec<TelemetryEvent>>,
14151446
}
14161447

1417-
static MISSING_ENVS_TEST_LOCK: Mutex<()> = Mutex::new(());
1418-
14191448
struct LockCheckingReporter {
14201449
configuration: Arc<RwLock<ConfigurationState>>,
14211450
reported: Mutex<bool>,
@@ -1857,50 +1886,54 @@ mod tests {
18571886

18581887
#[test]
18591888
fn test_stale_generation_does_not_begin_missing_env_reporting() {
1860-
let _guard = MISSING_ENVS_TEST_LOCK.lock().unwrap();
1861-
MISSING_ENVS_REPORTING_STATE.store(MISSING_ENVS_AVAILABLE, Ordering::Release);
1889+
let reporting_state = AtomicU64::new(MISSING_ENVS_AVAILABLE);
18621890
let configuration = RwLock::new(ConfigurationState {
18631891
generation: 2,
18641892
config: Configuration::default(),
18651893
});
18661894

1867-
assert!(!try_begin_missing_env_reporting(&configuration, 1));
1895+
assert!(!try_begin_missing_env_reporting_with_state(
1896+
&reporting_state,
1897+
&configuration,
1898+
1,
1899+
));
18681900
assert_eq!(
1869-
MISSING_ENVS_REPORTING_STATE.load(Ordering::Acquire),
1901+
reporting_state.load(Ordering::Acquire),
18701902
MISSING_ENVS_AVAILABLE
18711903
);
18721904
}
18731905

18741906
#[test]
18751907
fn test_stale_generation_releases_missing_env_reporting_slot() {
1876-
let _guard = MISSING_ENVS_TEST_LOCK.lock().unwrap();
1877-
MISSING_ENVS_REPORTING_STATE.store(2, Ordering::Release);
1908+
let reporting_state = AtomicU64::new(2);
18781909
let configuration = RwLock::new(ConfigurationState {
18791910
generation: 3,
18801911
config: Configuration::default(),
18811912
});
18821913

1883-
release_missing_env_reporting_if_stale(&configuration, 2);
1914+
release_missing_env_reporting_if_stale_with_state(&reporting_state, &configuration, 2);
18841915

18851916
assert_eq!(
1886-
MISSING_ENVS_REPORTING_STATE.load(Ordering::Acquire),
1917+
reporting_state.load(Ordering::Acquire),
18871918
MISSING_ENVS_AVAILABLE
18881919
);
18891920
}
18901921

18911922
#[test]
18921923
fn test_newer_generation_can_claim_missing_env_reporting_after_older_reservation() {
1893-
let _guard = MISSING_ENVS_TEST_LOCK.lock().unwrap();
1894-
MISSING_ENVS_REPORTING_STATE.store(1, Ordering::Release);
1924+
let reporting_state = AtomicU64::new(1);
18951925
let configuration = RwLock::new(ConfigurationState {
18961926
generation: 2,
18971927
config: Configuration::default(),
18981928
});
18991929

1900-
assert!(try_begin_missing_env_reporting(&configuration, 2));
1901-
assert_eq!(MISSING_ENVS_REPORTING_STATE.load(Ordering::Acquire), 2);
1930+
assert!(try_begin_missing_env_reporting_with_state(
1931+
&reporting_state,
1932+
&configuration,
1933+
2,
1934+
));
1935+
assert_eq!(reporting_state.load(Ordering::Acquire), 2);
19021936
}
1903-
19041937
#[test]
19051938
fn test_refresh_coordinator_joins_identical_requests() {
19061939
let coordinator = RefreshCoordinator::default();
@@ -2603,39 +2636,40 @@ mod tests {
26032636
));
26042637
}
26052638

2606-
/// Test for #395: configure resets MISSING_ENVS_REPORTING_STATE so that
2607-
/// subsequent refreshes can trigger missing-env reporting again.
2639+
/// Test for #395: configure resets missing-env state so that subsequent
2640+
/// refreshes can trigger reporting again.
26082641
#[test]
26092642
fn test_configure_resets_completed_missing_env_reporting() {
2610-
let _guard = MISSING_ENVS_TEST_LOCK.lock().unwrap();
2611-
2643+
let reporting_state = AtomicU64::new(MISSING_ENVS_AVAILABLE);
26122644
let configuration = Arc::new(RwLock::new(ConfigurationState {
26132645
generation: 1,
26142646
config: Configuration::default(),
26152647
}));
26162648

2617-
// Simulate a completed first refresh.
2618-
MISSING_ENVS_REPORTING_STATE.store(MISSING_ENVS_AVAILABLE, Ordering::Release);
2619-
assert!(try_begin_missing_env_reporting(configuration.as_ref(), 1));
2620-
complete_missing_env_reporting(1);
2621-
2622-
// Missing-env reporting is now exhausted.
2623-
assert!(!try_begin_missing_env_reporting(configuration.as_ref(), 1));
2649+
assert!(try_begin_missing_env_reporting_with_state(
2650+
&reporting_state,
2651+
configuration.as_ref(),
2652+
1,
2653+
));
2654+
complete_missing_env_reporting_with_state(&reporting_state, 1);
2655+
assert!(!try_begin_missing_env_reporting_with_state(
2656+
&reporting_state,
2657+
configuration.as_ref(),
2658+
1,
2659+
));
26242660

2625-
// Simulate what handle_configure does: bump generation and reset.
26262661
{
26272662
let mut state = configuration.write().unwrap();
26282663
state.generation = 2;
2629-
MISSING_ENVS_REPORTING_STATE.store(MISSING_ENVS_AVAILABLE, Ordering::Release);
2664+
reporting_state.store(MISSING_ENVS_AVAILABLE, Ordering::Release);
26302665
}
26312666

2632-
// Missing-env reporting should work again for the new generation.
2633-
assert!(try_begin_missing_env_reporting(configuration.as_ref(), 2));
2634-
2635-
// Cleanup.
2636-
MISSING_ENVS_REPORTING_STATE.store(MISSING_ENVS_AVAILABLE, Ordering::Release);
2667+
assert!(try_begin_missing_env_reporting_with_state(
2668+
&reporting_state,
2669+
configuration.as_ref(),
2670+
2,
2671+
));
26372672
}
2638-
26392673
/// Test for #461: refresh-side `configuration.read()` callers must not
26402674
/// block on the configure thread while it iterates `locator.configure()`.
26412675
#[test]

0 commit comments

Comments
 (0)