test(io-metrics): assert metric emission in six modules of record_* smoke tests (#6021)

crates/io-metrics/src/adaptive_ttl.rs

@@ -315,6 +315,44 @@ impl Default for AccessTracker {
 mod tests {
     use super::*;
 
+    /// Replaces the per-helper smoke tests that called the record_* helpers
+    /// and asserted nothing: the calls (same literals) now run against a local
+    /// DebuggingRecorder and every metric name the helpers own must actually
+    /// be emitted (rustfs/backlog#1836 PR3).
+    #[test]
+    fn record_helpers_emit_their_metrics() {
+        let recorder = metrics_util::debugging::DebuggingRecorder::new();
+        let snapshotter = recorder.snapshotter();
+        metrics::with_local_recorder(&recorder, || {
+            record_ttl_adjustment("test-key", 100, 150);
+            record_ttl_adjustment("test-key", 100, 50);
+            record_ttl_expiration();
+            record_early_eviction("cold");
+            record_early_eviction("low_priority");
+            record_access_pattern_change("sequential", "random");
+            record_access_pattern_change("random", "sequential");
+        });
+
+        let emitted: std::collections::HashSet<String> = snapshotter
+            .snapshot()
+            .into_vec()
+            .into_iter()
+            .map(|(composite, _, _, _)| composite.key().name().to_string())
+            .collect();
+        for expected in [
+            "rustfs_cache_ttl_adjustments",
+            "rustfs_cache_ttl_base",
+            "rustfs_cache_ttl_adjusted",
+            "rustfs_cache_ttl_extensions",
+            "rustfs_cache_ttl_reductions",
+            "rustfs_cache_ttl_expirations",
+            "rustfs_cache_evictions_early",
+            "rustfs_cache_access_pattern_changes",
+        ] {
+            assert!(emitted.contains(expected), "{expected} must be emitted by its record helper");
+        }
+    }
+
     #[test]
     fn test_adaptive_ttl_stats() {
         let mut stats = AdaptiveTTLStats::new();
@@ -335,30 +373,6 @@ mod tests {
         assert!((stats.reduction_rate() - 0.3333333333333333).abs() < 0.01);
     }
 
-    #[test]
-    fn test_record_ttl_adjustment() {
-        // This test verifies the function compiles and runs
-        record_ttl_adjustment("test-key", 100, 150);
-        record_ttl_adjustment("test-key", 100, 50);
-    }
-
-    #[test]
-    fn test_record_ttl_expiration() {
-        record_ttl_expiration();
-    }
-
-    #[test]
-    fn test_record_early_eviction() {
-        record_early_eviction("cold");
-        record_early_eviction("low_priority");
-    }
-
-    #[test]
-    fn test_record_access_pattern_change() {
-        record_access_pattern_change("sequential", "random");
-        record_access_pattern_change("random", "sequential");
-    }
-
     #[test]
     fn test_access_record() {
         let mut record = AccessRecord::new();

crates/io-metrics/src/backpressure_metrics.rs

@@ -53,30 +53,38 @@ pub fn record_backpressure_deactivation() {
 mod tests {
     use super::*;
 
+    /// Replaces the per-helper smoke tests that called the record_* helpers
+    /// and asserted nothing: the calls (same literals) now run against a local
+    /// DebuggingRecorder and every metric name the helpers own must actually
+    /// be emitted (rustfs/backlog#1836 PR3).
     #[test]
-    fn test_record_backpressure_state_change() {
+    fn record_helpers_emit_their_metrics() {
-        record_backpressure_state_change("normal", "warning");
+        let recorder = metrics_util::debugging::DebuggingRecorder::new();
-        record_backpressure_state_change("warning", "critical");
+        let snapshotter = recorder.snapshotter();
-    }
+        metrics::with_local_recorder(&recorder, || {
+            record_backpressure_state_change("normal", "warning");
+            record_backpressure_state_change("warning", "critical");
+            record_backpressure_rejection();
+            record_concurrent_operations(10);
+            record_concurrent_operations(32);
+            record_backpressure_activation();
+            record_backpressure_deactivation();
+        });
 
-    #[test]
+        let emitted: std::collections::HashSet<String> = snapshotter
-    fn test_record_backpressure_rejection() {
+            .snapshot()
-        record_backpressure_rejection();
+            .into_vec()
-    }
+            .into_iter()
-
+            .map(|(composite, _, _, _)| composite.key().name().to_string())
-    #[test]
+            .collect();
-    fn test_record_concurrent_operations() {
+        for expected in [
-        record_concurrent_operations(10);
+            "rustfs_backpressure_state_changes",
-        record_concurrent_operations(32);
+            "rustfs_backpressure_rejections",
-    }
+            "rustfs_backpressure_concurrent",
-
+            "rustfs_backpressure_activations",
-    #[test]
+            "rustfs_backpressure_deactivations",
-    fn test_record_backpressure_activation() {
+        ] {
-        record_backpressure_activation();
+            assert!(emitted.contains(expected), "{expected} must be emitted by its record helper");
-    }
+        }
-
-    #[test]
-    fn test_record_backpressure_deactivation() {
-        record_backpressure_deactivation();
     }
 }

crates/io-metrics/src/deadlock_metrics.rs

@@ -72,39 +72,48 @@ pub fn record_wait_edge_removed() {
 mod tests {
     use super::*;
 
+    /// Replaces the per-helper smoke tests that called the record_* helpers
+    /// and asserted nothing: the calls (same literals) now run against a local
+    /// DebuggingRecorder and every metric name the helpers own must actually
+    /// be emitted (rustfs/backlog#1836 PR3).
     #[test]
-    fn test_record_deadlock_detected() {
+    fn record_helpers_emit_their_metrics() {
-        record_deadlock_detected(3);
+        let recorder = metrics_util::debugging::DebuggingRecorder::new();
-        record_deadlock_detected(5);
+        let snapshotter = recorder.snapshotter();
-    }
+        metrics::with_local_recorder(&recorder, || {
+            record_deadlock_detected(3);
+            record_deadlock_detected(5);
+            record_long_held_lock(1, Duration::from_secs(30));
+            record_long_held_lock(2, Duration::from_secs(60));
+            record_lock_acquisition("mutex");
+            record_lock_acquisition("rwlock");
+            record_lock_release("mutex", Duration::from_millis(10));
+            record_lock_release("rwlock", Duration::from_millis(5));
+            record_lock_contention("mutex");
+            record_lock_contention("rwlock");
+            record_wait_edge_added();
+            record_wait_edge_removed();
+        });
 
-    #[test]
+        let emitted: std::collections::HashSet<String> = snapshotter
-    fn test_record_long_held_lock() {
+            .snapshot()
-        record_long_held_lock(1, Duration::from_secs(30));
+            .into_vec()
-        record_long_held_lock(2, Duration::from_secs(60));
+            .into_iter()
-    }
+            .map(|(composite, _, _, _)| composite.key().name().to_string())
-
+            .collect();
-    #[test]
+        for expected in [
-    fn test_record_lock_acquisition() {
+            "rustfs_deadlock_detected_total",
-        record_lock_acquisition("mutex");
+            "rustfs_deadlock_cycle_length",
-        record_lock_acquisition("rwlock");
+            "rustfs_deadlock_long_held",
-    }
+            "rustfs_deadlock_hold_time_secs",
-
+            "rustfs_lock_acquisitions",
-    #[test]
+            "rustfs_lock_releases",
-    fn test_record_lock_release() {
+            "rustfs_lock_hold_time_secs",
-        record_lock_release("mutex", Duration::from_millis(10));
+            "rustfs_lock_contentions",
-        record_lock_release("rwlock", Duration::from_millis(5));
+            "rustfs_deadlock_wait_edges_added",
-    }
+            "rustfs_deadlock_wait_edges_removed",
-
+        ] {
-    #[test]
+            assert!(emitted.contains(expected), "{expected} must be emitted by its record helper");
-    fn test_record_lock_contention() {
+        }
-        record_lock_contention("mutex");
-        record_lock_contention("rwlock");
-    }
-
-    #[test]
-    fn test_record_wait_edge() {
-        record_wait_edge_added();
-        record_wait_edge_removed();
     }
 }

crates/io-metrics/src/io_metrics.rs

@@ -169,46 +169,58 @@ impl IoSchedulerStats {
 mod tests {
     use super::*;
 
+    /// Replaces the per-helper smoke tests that called the record_* helpers
+    /// and asserted nothing: the calls (same literals) now run against a local
+    /// DebuggingRecorder and every metric name the helpers own must actually
+    /// be emitted (rustfs/backlog#1836 PR3).
     #[test]
-    fn test_record_io_scheduler_decision() {
+    fn record_helpers_emit_their_metrics() {
-        record_io_scheduler_decision(128 * 1024, "low", "sequential");
+        let recorder = metrics_util::debugging::DebuggingRecorder::new();
-        record_io_scheduler_decision(64 * 1024, "high", "random");
+        let snapshotter = recorder.snapshotter();
-    }
+        metrics::with_local_recorder(&recorder, || {
+            record_io_scheduler_decision(128 * 1024, "low", "sequential");
+            record_io_scheduler_decision(64 * 1024, "high", "random");
+            record_io_priority_decision("high", 1024);
+            record_io_priority_decision("normal", 1024 * 1024);
+            record_io_priority_decision("low", 10 * 1024 * 1024);
+            record_load_level_change("low", "medium");
+            record_load_level_change("medium", "high");
+            record_bandwidth_observation(100 * 1024 * 1024);
+            record_bandwidth_observation(500 * 1024 * 1024);
+            record_buffer_size_adjustment(128 * 1024, 64 * 1024, "concurrency");
+            record_buffer_size_adjustment(128 * 1024, 256 * 1024, "sequential");
+            record_queue_operation("enqueue", "high", 10);
+            record_queue_operation("dequeue", "high", 9);
+            record_starvation_event("low");
+        });
 
-    #[test]
+        let emitted: std::collections::HashSet<String> = snapshotter
-    fn test_record_io_priority_decision() {
+            .snapshot()
-        record_io_priority_decision("high", 1024);
+            .into_vec()
-        record_io_priority_decision("normal", 1024 * 1024);
+            .into_iter()
-        record_io_priority_decision("low", 10 * 1024 * 1024);
+            .map(|(composite, _, _, _)| composite.key().name().to_string())
-    }
+            .collect();
-
+        for expected in [
-    #[test]
+            "rustfs_io_scheduler_decisions",
-    fn test_record_load_level_change() {
+            "rustfs_io_scheduler_buffer_size",
-        record_load_level_change("low", "medium");
+            "rustfs_io_scheduler_load",
-        record_load_level_change("medium", "high");
+            "rustfs_io_scheduler_strategy",
-    }
+            "rustfs_io_scheduler_buffer_size_histogram",
-
+            "rustfs_io_priority_decisions",
-    #[test]
+            "rustfs_io_priority_by_level",
-    fn test_record_bandwidth_observation() {
+            "rustfs_io_priority_request_size",
-        record_bandwidth_observation(100 * 1024 * 1024);
+            "rustfs_io_load_changes",
-        record_bandwidth_observation(500 * 1024 * 1024);
+            "rustfs_io_bandwidth_bps",
-    }
+            "rustfs_io_bandwidth_histogram",
-
+            "rustfs_io_buffer_adjustments",
-    #[test]
+            "rustfs_io_buffer_original",
-    fn test_record_buffer_size_adjustment() {
+            "rustfs_io_buffer_adjusted",
-        record_buffer_size_adjustment(128 * 1024, 64 * 1024, "concurrency");
+            "rustfs_io_queue_operations",
-        record_buffer_size_adjustment(128 * 1024, 256 * 1024, "sequential");
+            "rustfs_io_queue_size",
-    }
+            "rustfs_io_starvation_events",
-
+        ] {
-    #[test]
+            assert!(emitted.contains(expected), "{expected} must be emitted by its record helper");
-    fn test_record_queue_operation() {
+        }
-        record_queue_operation("enqueue", "high", 10);
-        record_queue_operation("dequeue", "high", 9);
-    }
-
-    #[test]
-    fn test_record_starvation_event() {
-        record_starvation_event("low");
     }
 
     #[test]

crates/io-metrics/src/lock_metrics.rs

@@ -163,6 +163,46 @@ impl LockMetricsSummary {
 #[cfg(test)]
 mod tests {
     use super::*;
+
+    /// Replaces the per-helper smoke tests that called the record_* helpers
+    /// and asserted nothing: the calls (same literals) now run against a local
+    /// DebuggingRecorder and every metric name the helpers own must actually
+    /// be emitted (rustfs/backlog#1836 PR3).
+    #[test]
+    fn record_helpers_emit_their_metrics() {
+        let recorder = metrics_util::debugging::DebuggingRecorder::new();
+        let snapshotter = recorder.snapshotter();
+        metrics::with_local_recorder(&recorder, || {
+            record_lock_optimization_enabled(true);
+            record_lock_optimization_enabled(false);
+            record_spin_attempt(true);
+            record_spin_attempt(false);
+            record_spin_count_change(100);
+            record_spin_count_change(200);
+            record_lock_hold_time(Duration::from_millis(10));
+            record_lock_hold_time(Duration::from_millis(100));
+            record_early_release();
+            record_contention_event();
+        });
+
+        let emitted: std::collections::HashSet<String> = snapshotter
+            .snapshot()
+            .into_vec()
+            .into_iter()
+            .map(|(composite, _, _, _)| composite.key().name().to_string())
+            .collect();
+        for expected in [
+            "rustfs_lock_optimization_enabled",
+            "rustfs_lock_spin_successes",
+            "rustfs_lock_spin_failures",
+            "rustfs_lock_spin_count",
+            "rustfs_lock_hold_time_secs",
+            "rustfs_lock_early_releases",
+            "rustfs_lock_contentions",
+        ] {
+            assert!(emitted.contains(expected), "{expected} must be emitted by its record helper");
+        }
+    }
     use metrics::{Counter, CounterFn, Gauge, GaugeFn, Histogram, HistogramFn, Key, KeyName, Metadata, SharedString, Unit};
     use std::sync::{Arc, Mutex};
 
@@ -255,40 +295,6 @@ mod tests {
         fn record(&self, _value: f64) {}
     }
 
-    #[test]
-    fn test_record_lock_optimization_enabled() {
-        record_lock_optimization_enabled(true);
-        record_lock_optimization_enabled(false);
-    }
-
-    #[test]
-    fn test_record_spin_attempt() {
-        record_spin_attempt(true);
-        record_spin_attempt(false);
-    }
-
-    #[test]
-    fn test_record_spin_count_change() {
-        record_spin_count_change(100);
-        record_spin_count_change(200);
-    }
-
-    #[test]
-    fn test_record_lock_hold_time() {
-        record_lock_hold_time(Duration::from_millis(10));
-        record_lock_hold_time(Duration::from_millis(100));
-    }
-
-    #[test]
-    fn test_record_early_release() {
-        record_early_release();
-    }
-
-    #[test]
-    fn test_record_contention_event() {
-        record_contention_event();
-    }
-
     #[test]
     fn test_record_object_lock_diag_enabled() {
         let recorder = SeenMetricsRecorder::default();

crates/io-metrics/src/timeout_metrics.rs

@@ -114,39 +114,46 @@ impl TimeoutMetricsSummary {
 mod tests {
     use super::*;
 
+    /// Replaces the per-helper smoke tests that called the record_* helpers
+    /// and asserted nothing: the calls (same literals) now run against a local
+    /// DebuggingRecorder and every metric name the helpers own must actually
+    /// be emitted (rustfs/backlog#1836 PR3).
     #[test]
-    fn test_record_timeout_event() {
+    fn record_helpers_emit_their_metrics() {
-        record_timeout_event("get_object");
+        let recorder = metrics_util::debugging::DebuggingRecorder::new();
-        record_timeout_event("put_object");
+        let snapshotter = recorder.snapshotter();
-    }
+        metrics::with_local_recorder(&recorder, || {
+            record_timeout_event("get_object");
+            record_timeout_event("put_object");
+            record_operation_duration("get_object", Duration::from_millis(100));
+            record_operation_duration("put_object", Duration::from_millis(500));
+            record_dynamic_timeout(1024 * 1024, Duration::from_secs(10));
+            record_dynamic_timeout(100 * 1024 * 1024, Duration::from_secs(30));
+            record_operation_progress("get_object", 50.0);
+            record_operation_progress("get_object", 100.0);
+            record_stalled_operation("get_object");
+            record_operation_completion("get_object", true);
+            record_operation_completion("get_object", false);
+        });
 
-    #[test]
+        let emitted: std::collections::HashSet<String> = snapshotter
-    fn test_record_operation_duration() {
+            .snapshot()
-        record_operation_duration("get_object", Duration::from_millis(100));
+            .into_vec()
-        record_operation_duration("put_object", Duration::from_millis(500));
+            .into_iter()
-    }
+            .map(|(composite, _, _, _)| composite.key().name().to_string())
-
+            .collect();
-    #[test]
+        for expected in [
-    fn test_record_dynamic_timeout() {
+            "rustfs_io_timeout_events_total",
-        record_dynamic_timeout(1024 * 1024, Duration::from_secs(10));
+            "rustfs_io_operation_duration_seconds",
-        record_dynamic_timeout(100 * 1024 * 1024, Duration::from_secs(30));
+            "rustfs_timeout_dynamic_size",
-    }
+            "rustfs_timeout_dynamic_secs",
-
+            "rustfs_timeout_dynamic_size_histogram",
-    #[test]
+            "rustfs_operation_progress",
-    fn test_record_operation_progress() {
+            "rustfs_operation_stalled",
-        record_operation_progress("get_object", 50.0);
+            "rustfs_operation_completions",
-        record_operation_progress("get_object", 100.0);
+        ] {
-    }
+            assert!(emitted.contains(expected), "{expected} must be emitted by its record helper");
-
+        }
-    #[test]
-    fn test_record_stalled_operation() {
-        record_stalled_operation("get_object");
-    }
-
-    #[test]
-    fn test_record_operation_completion() {
-        record_operation_completion("get_object", true);
-        record_operation_completion("get_object", false);
     }
 
     #[test]

scripts/find_assertless_tests.py

@@ -0,0 +1,112 @@
+#!/usr/bin/env python3
+# Copyright 2024 RustFS Team
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Census of assertion-less tests (rustfs/backlog#1836 PR3).
+
+Flags `#[test]` / `#[tokio::test]` functions whose bodies contain no
+verification signal: no assert!/assert_eq!/assert_ne!/panic! macro, no
+`.expect(`/`.unwrap(`, no `?` operator, no `#[should_panic]`, and no
+`insta` snapshot / proptest / matches! usage. Such a test is green no
+matter what the code under test does.
+
+This is a heuristic REVIEW QUEUE, not a lint: a hit still needs human
+reading before it is fixed or deleted, because assertions may live in a
+called helper. Known false-positive classes are excluded up front:
+
+- `#[test_case(...)]`-driven functions (the values are the assertion's
+  parameters; the assert lives in the shared body — still scanned, but a
+  body that asserts is not flagged anyway; the exclusion covers wrappers
+  that only delegate to a suite runner).
+- Functions whose body calls a helper with `assert`, `verify`, `check`,
+  `expect`, `run_` or `_case` in its name (suite-delegation pattern).
+
+Usage:
+    scripts/find_assertless_tests.py [path ...]     # default: crates rustfs/src
+
+Exit code is always 0; the output is the queue.
+"""
+
+import re
+import sys
+from pathlib import Path
+
+VERIFY_SIGNALS = re.compile(
+    r"assert!|assert_eq!|assert_ne!|debug_assert|panic!\(|\.expect\(|\.unwrap\(|"
+    r"unreachable!|matches!\(|insta::|proptest!|\.await\?|\)\?|\?;|should_panic"
+)
+DELEGATION = re.compile(r"\b[a-z0-9_]*(?:assert|verify|check|expect|run_case|_case|harness|round_trip|roundtrip)[a-z0-9_]*\s*\(")
+TEST_ATTR = re.compile(r"#\[(?:tokio::)?test[\](]")
+TEST_CASE_ATTR = re.compile(r"#\[test_case")
+FN_LINE = re.compile(r"^\s*(?:pub\s+)?(?:async\s+)?fn\s+([a-zA-Z0-9_]+)")
+
+
+def scan_file(path: Path):
+    try:
+        lines = path.read_text(encoding="utf-8").split("\n")
+    except (UnicodeDecodeError, OSError):
+        return
+    i = 0
+    while i < len(lines):
+        if not TEST_ATTR.search(lines[i]):
+            i += 1
+            continue
+        # collect the whole attribute block (may include #[serial], #[test_case], ...)
+        attrs = []
+        j = i
+        while j < len(lines) and (lines[j].strip().startswith("#[") or lines[j].strip().startswith("//")):
+            attrs.append(lines[j])
+            j += 1
+        if j >= len(lines):
+            break
+        m = FN_LINE.match(lines[j])
+        if not m:
+            i = j + 1
+            continue
+        name = m.group(1)
+        if any(TEST_CASE_ATTR.search(a) for a in attrs):
+            i = j + 1
+            continue
+        # brace-match the body
+        depth = 0
+        begun = False
+        body = []
+        k = j
+        while k < len(lines):
+            for ch in lines[k]:
+                if ch == "{":
+                    depth += 1
+                    begun = True
+                elif ch == "}":
+                    depth -= 1
+            body.append(lines[k])
+            if begun and depth <= 0:
+                break
+            k += 1
+        text = "\n".join(body)
+        if not VERIFY_SIGNALS.search(text) and not DELEGATION.search(text):
+            print(f"{path}:{j + 1}: {name}")
+        i = k + 1
+
+
+def main():
+    roots = [Path(p) for p in (sys.argv[1:] or ["crates", "rustfs/src"])]
+    for root in roots:
+        for path in sorted(root.rglob("*.rs")):
+            if "target" in path.parts:
+                continue
+            scan_file(path)
+
+
+if __name__ == "__main__":
+    main()