From 0049c5968c212a6f1512cfb9619701ad8054b38f Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 9 Aug 2025 07:17:16 +0100 Subject: [PATCH 1/3] Replace connection panic integration test with unit test --- src/server/connection.rs | 47 ---------------------------------------- tests/metrics.rs | 31 +++++++++++++------------- 2 files changed, 16 insertions(+), 62 deletions(-) diff --git a/src/server/connection.rs b/src/server/connection.rs index 503640d8..77fe61a9 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -86,7 +86,6 @@ async fn process_stream( #[cfg(test)] mod tests { - use metrics_util::debugging::{DebugValue, DebuggingRecorder}; use rstest::rstest; use tokio::{ net::{TcpListener, TcpStream}, @@ -212,50 +211,4 @@ mod tests { .ok_or_else(|| "panic log not found".to_string()) }); } - - /// Panics increment the connection panic counter. - #[rstest] - #[tokio::test] - async fn connection_panic_metric_increments( - factory: impl Fn() -> WireframeApp + Send + Sync + Clone + 'static, - ) { - let recorder = DebuggingRecorder::new(); - let snapshotter = recorder.snapshotter(); - recorder.install().expect("recorder install"); - - let app_factory = move || { - factory() - .on_connection_setup(|| async { panic!("boom") }) - .unwrap() - }; - let tracker = TaskTracker::new(); - let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let addr = listener.local_addr().unwrap(); - - let task = tokio::spawn({ - let tracker = tracker.clone(); - let app_factory = app_factory; - async move { - let (stream, _) = listener.accept().await.unwrap(); - spawn_connection_task::<_, ()>(stream, app_factory, None, None, &tracker); - tracker.close(); - tracker.wait().await; - } - }); - - let client = TcpStream::connect(addr).await.unwrap(); - client.writable().await.unwrap(); - client.try_write(&[0; 8]).unwrap(); - drop(client); - - task.await.unwrap(); - tokio::task::yield_now().await; - - let metrics = snapshotter.snapshot().into_vec(); - let found = metrics.iter().any(|(k, _, _, v)| { - k.key().name() == crate::metrics::CONNECTION_PANICS - && matches!(v, DebugValue::Counter(c) if *c > 0) - }); - assert!(found, "connection panic metric not recorded"); - } } diff --git a/tests/metrics.rs b/tests/metrics.rs index 684f0bce..c5d441ad 100644 --- a/tests/metrics.rs +++ b/tests/metrics.rs @@ -63,21 +63,22 @@ fn error_metric_increments() { } #[test] -fn connection_panic_metric_increments() { - let (snapshotter, recorder) = debugging_recorder_setup(); - metrics::with_local_recorder(&recorder, || { - wireframe::metrics::inc_connection_panics(); - }); +fn inc_connection_panics_increments_counter() { + // Arrange: install a debugging recorder + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + recorder.install().expect("failed to install recorder"); + + // Act: invoke the panic-increment function directly + wireframe::metrics::inc_connection_panics(); + // Assert: we saw exactly one increment on the CONNECTION_PANICS counter let metrics = snapshotter.snapshot().into_vec(); - let count = metrics - .iter() - .find_map(|(k, _, _, v)| { - (k.key().name() == wireframe::metrics::CONNECTION_PANICS).then_some(match v { - DebugValue::Counter(c) => *c, - _ => 0, - }) - }) - .unwrap_or(0); - assert_eq!(1, count, "panic metric not recorded"); + assert!( + metrics.iter().any(|(key, _, _, value)| { + key.key().name() == wireframe::metrics::CONNECTION_PANICS + && matches!(value, DebugValue::Counter(c) if *c == 1) + }), + "expected CONNECTION_PANICS == 1, got {metrics:#?}" + ); } From b49923a77955964eb1ff7fcf5301754461083bd3 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 9 Aug 2025 19:41:51 +0100 Subject: [PATCH 2/3] Test connection panic metrics locally --- tests/metrics.rs | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/tests/metrics.rs b/tests/metrics.rs index c5d441ad..5a3a13d6 100644 --- a/tests/metrics.rs +++ b/tests/metrics.rs @@ -64,13 +64,13 @@ fn error_metric_increments() { #[test] fn inc_connection_panics_increments_counter() { - // Arrange: install a debugging recorder - let recorder = DebuggingRecorder::new(); - let snapshotter = recorder.snapshotter(); - recorder.install().expect("failed to install recorder"); + // Arrange: create a debugging recorder + let (snapshotter, recorder) = debugging_recorder_setup(); - // Act: invoke the panic-increment function directly - wireframe::metrics::inc_connection_panics(); + // Act: invoke within a local recorder scope to avoid global state + metrics::with_local_recorder(&recorder, || { + wireframe::metrics::inc_connection_panics(); + }); // Assert: we saw exactly one increment on the CONNECTION_PANICS counter let metrics = snapshotter.snapshot().into_vec(); @@ -82,3 +82,25 @@ fn inc_connection_panics_increments_counter() { "expected CONNECTION_PANICS == 1, got {metrics:#?}" ); } + +#[test] +fn inc_connection_panics_accumulates_multiple_calls() { + // Arrange: create a debugging recorder + let (snapshotter, recorder) = debugging_recorder_setup(); + + // Act: increment twice within a local recorder scope + metrics::with_local_recorder(&recorder, || { + wireframe::metrics::inc_connection_panics(); + wireframe::metrics::inc_connection_panics(); + }); + + // Assert: the CONNECTION_PANICS counter equals two + let metrics = snapshotter.snapshot().into_vec(); + assert!( + metrics.iter().any(|(key, _, _, value)| { + key.key().name() == wireframe::metrics::CONNECTION_PANICS + && matches!(value, DebugValue::Counter(c) if *c == 2) + }), + "expected CONNECTION_PANICS == 2, got {metrics:#?}" + ); +} From ac08b140e37f725dbc33892ed07f64f17a270a9a Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 10 Aug 2025 11:32:21 +0100 Subject: [PATCH 3/3] Parametrize connection panic metric test --- tests/metrics.rs | 44 ++++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/tests/metrics.rs b/tests/metrics.rs index 5a3a13d6..e7b7079c 100644 --- a/tests/metrics.rs +++ b/tests/metrics.rs @@ -3,6 +3,7 @@ //! These tests verify that counters and gauges update as expected using //! `metrics_util::debugging::DebuggingRecorder`. use metrics_util::debugging::{DebugValue, DebuggingRecorder, Snapshotter}; +use rstest::rstest; /// Creates a debugging recorder and snapshotter for metrics testing. fn debugging_recorder_setup() -> (Snapshotter, DebuggingRecorder) { @@ -62,45 +63,32 @@ fn error_metric_increments() { assert!(found, "error metric not recorded"); } -#[test] -fn inc_connection_panics_increments_counter() { - // Arrange: create a debugging recorder +#[rstest] +#[case(1)] +#[case(2)] +fn inc_connection_panics_counts(#[case] expected: u64) { + // Arrange let (snapshotter, recorder) = debugging_recorder_setup(); - // Act: invoke within a local recorder scope to avoid global state + // Act metrics::with_local_recorder(&recorder, || { - wireframe::metrics::inc_connection_panics(); + (0..expected).for_each(|_| wireframe::metrics::inc_connection_panics()); }); - // Assert: we saw exactly one increment on the CONNECTION_PANICS counter - let metrics = snapshotter.snapshot().into_vec(); - assert!( - metrics.iter().any(|(key, _, _, value)| { - key.key().name() == wireframe::metrics::CONNECTION_PANICS - && matches!(value, DebugValue::Counter(c) if *c == 1) - }), - "expected CONNECTION_PANICS == 1, got {metrics:#?}" + // Assert + assert_counter_eq( + &snapshotter, + wireframe::metrics::CONNECTION_PANICS, + expected, ); } -#[test] -fn inc_connection_panics_accumulates_multiple_calls() { - // Arrange: create a debugging recorder - let (snapshotter, recorder) = debugging_recorder_setup(); - - // Act: increment twice within a local recorder scope - metrics::with_local_recorder(&recorder, || { - wireframe::metrics::inc_connection_panics(); - wireframe::metrics::inc_connection_panics(); - }); - - // Assert: the CONNECTION_PANICS counter equals two +fn assert_counter_eq(snapshotter: &Snapshotter, name: &str, expected: u64) { let metrics = snapshotter.snapshot().into_vec(); assert!( metrics.iter().any(|(key, _, _, value)| { - key.key().name() == wireframe::metrics::CONNECTION_PANICS - && matches!(value, DebugValue::Counter(c) if *c == 2) + key.key().name() == name && matches!(value, DebugValue::Counter(c) if *c == expected) }), - "expected CONNECTION_PANICS == 2, got {metrics:#?}" + "expected {name} == {expected}, got {metrics:#?}" ); }