Skip to content

Commit

Permalink
Improve oximeter self-stat tests (#4577)
Browse files Browse the repository at this point in the history
Reduces the tick interval in calls to `tokio::time::advance()` to ensure
all timers complete reliably. See #4566 for context.
  • Loading branch information
bnaecker authored Nov 29, 2023
1 parent 22a70e4 commit f24447b
Showing 1 changed file with 30 additions and 21 deletions.
51 changes: 30 additions & 21 deletions oximeter/collector/src/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,24 @@ mod tests {
use tokio::time::Instant;
use uuid::Uuid;

// Interval on which oximeter collects from producers in these tests.
const COLLECTION_INTERVAL: Duration = Duration::from_secs(1);

// Interval in calls to `tokio::time::advance`. This must be sufficiently
// small relative to `COLLECTION_INTERVAL` to ensure all ticks of internal
// timers complete as expected.
const TICK_INTERVAL: Duration = Duration::from_millis(10);

// Total number of collection attempts.
const N_COLLECTIONS: u64 = 5;

// Period these tests wait using `tokio::time::advance()` before checking
// their test conditions.
const TEST_WAIT_PERIOD: Duration = Duration::from_millis(
COLLECTION_INTERVAL.as_millis() as u64 * N_COLLECTIONS
+ COLLECTION_INTERVAL.as_millis() as u64 / 2,
);

// Test that we count successful collections from a target correctly.
#[tokio::test]
async fn test_self_stat_collection_count() {
Expand Down Expand Up @@ -692,13 +710,12 @@ mod tests {
let _task = tokio::task::spawn(server);

// Register the dummy producer.
let interval = Duration::from_secs(1);
let endpoint = ProducerEndpoint {
id: Uuid::new_v4(),
kind: Some(ProducerKind::Service),
address,
base_route: String::from("/"),
interval,
interval: COLLECTION_INTERVAL,
};
collector
.register_producer(endpoint)
Expand All @@ -708,10 +725,8 @@ mod tests {
// Step time until there has been exactly `N_COLLECTIONS` collections.
tokio::time::pause();
let now = Instant::now();
const N_COLLECTIONS: usize = 5;
let wait_for = interval * N_COLLECTIONS as u32 + interval / 2;
while now.elapsed() < wait_for {
tokio::time::advance(interval / 10).await;
while now.elapsed() < TEST_WAIT_PERIOD {
tokio::time::advance(TICK_INTERVAL).await;
}

// Request the statistics from the task itself.
Expand All @@ -729,7 +744,7 @@ mod tests {
.await
.expect("failed to request statistics from task");
let stats = rx.await.expect("failed to receive statistics from task");
assert_eq!(stats.collections.datum.value(), N_COLLECTIONS as u64);
assert_eq!(stats.collections.datum.value(), N_COLLECTIONS);
assert!(stats.failed_collections.is_empty());
logctx.cleanup_successful();
}
Expand All @@ -751,7 +766,6 @@ mod tests {

// Register a bogus producer, which is equivalent to a producer that is
// unreachable.
let interval = Duration::from_secs(1);
let endpoint = ProducerEndpoint {
id: Uuid::new_v4(),
kind: Some(ProducerKind::Service),
Expand All @@ -762,7 +776,7 @@ mod tests {
0,
)),
base_route: String::from("/"),
interval,
interval: COLLECTION_INTERVAL,
};
collector
.register_producer(endpoint)
Expand All @@ -772,10 +786,8 @@ mod tests {
// Step time until there has been exactly `N_COLLECTIONS` collections.
tokio::time::pause();
let now = Instant::now();
const N_COLLECTIONS: usize = 5;
let wait_for = interval * N_COLLECTIONS as u32 + interval / 2;
while now.elapsed() < wait_for {
tokio::time::advance(interval / 10).await;
while now.elapsed() < TEST_WAIT_PERIOD {
tokio::time::advance(TICK_INTERVAL).await;
}

// Request the statistics from the task itself.
Expand All @@ -801,7 +813,7 @@ mod tests {
.unwrap()
.datum
.value(),
N_COLLECTIONS as u64
N_COLLECTIONS,
);
assert_eq!(stats.failed_collections.len(), 1);
logctx.cleanup_successful();
Expand Down Expand Up @@ -840,13 +852,12 @@ mod tests {
let _task = tokio::task::spawn(server);

// Register the rather flaky producer.
let interval = Duration::from_secs(1);
let endpoint = ProducerEndpoint {
id: Uuid::new_v4(),
kind: Some(ProducerKind::Service),
address,
base_route: String::from("/"),
interval,
interval: COLLECTION_INTERVAL,
};
collector
.register_producer(endpoint)
Expand All @@ -856,10 +867,8 @@ mod tests {
// Step time until there has been exactly `N_COLLECTIONS` collections.
tokio::time::pause();
let now = Instant::now();
const N_COLLECTIONS: usize = 5;
let wait_for = interval * N_COLLECTIONS as u32 + interval / 2;
while now.elapsed() < wait_for {
tokio::time::advance(interval / 10).await;
while now.elapsed() < TEST_WAIT_PERIOD {
tokio::time::advance(TICK_INTERVAL).await;
}

// Request the statistics from the task itself.
Expand All @@ -885,7 +894,7 @@ mod tests {
.unwrap()
.datum
.value(),
N_COLLECTIONS as u64
N_COLLECTIONS,
);
assert_eq!(stats.failed_collections.len(), 1);
logctx.cleanup_successful();
Expand Down

0 comments on commit f24447b

Please sign in to comment.