Compare commits

..

2 Commits

Author SHA1 Message Date
overtrue 695eb89da7 test: assert four leaf-crate smoke tests, and two more census fixes
Two more census heuristics, both verified by bisecting the candidate count so a "fix" that widened the queue could not slip through:

- any `assert*!` macro counts as verification, not just the three built-ins — `assert_fields_bound!` in `protos` was being missed. The pattern deliberately stays a substring match: an earlier attempt anchored it with `\b`, which silently stopped matching prefixed macros like `const_assert!` and pushed the queue from 32 to 55 before the count caught it.
- `let _ = Type::<T>::method;` is a signature guard, the same as the already-recognised nested-fn form. That is `iam`'s deprecated-API test.

Tree-wide candidates go 32 to 30 from the script alone, then to 26 with the four tests below.

`detect_storage_media`'s two tests only checked that the call did not panic, over a `match` whose arms were all empty. What the machine reports depends on the machine, but two rules do not: an override wins over probing, including when probing is disabled, and disabled probing reports `Unknown` rather than guessing. A third test keeps the platform call and asserts it returns a known variant *and* the same one twice — a probe that flapped would make the scheduler's profile depend on when it asked.

`runtime_facade_stops_empty_replay_workers` called the stop path and asserted nothing. It now checks the worker list is empty afterwards and that a second call stays harmless, which is what shutdown paths actually do.

`test_mask_never_recurses_for_any_variant` discarded every mask. Termination is still the property under test — a regression overflows the stack rather than failing an assertion — but the masks are now collected and checked, so the loop cannot fold away and a variant that starts returning an empty mask is caught too.

Two known false positives are left in the queue rather than chased: `utils/src/string.rs:942` does assert, but its input string `"{1...2}}"` unbalances the scanner's brace counter and truncates the body before the assertion. Making the counter literal-aware needs a lexer that understands raw strings — a first attempt desynchronised on `r#"{"invalid": json}"#` and pushed the queue to 120, so it was backed out. `s3select-api:379` declares a nested exhaustive-match fn and never calls it; recognising that shape without hiding genuinely empty bodies needs more care than it is worth today.

Refs backlog#1836
2026-08-19 10:32:19 +08:00
overtrue 21d6b2a054 test(ecstore): assert the error conversions, and stop the census over-reporting
The census listed 17 candidates in ecstore. Sixteen were false positives of three shapes, and reading them showed the heuristics rather than the tests were wrong:

- `#[should_panic(expected = "...")]` (5). `should_panic` was already in the verification signals, but the check only ever ran against the function body — the attribute block was collected and then ignored, so the expected panic message, which *is* the assertion, was invisible.
- Bodies that are a single call into a shared harness (9), like `run(DurabilityMode::Strict).await` and `aborting_encode_drops_blocked_producer(EncodePipeline::Vec).await`. The delegation rule keyed off callee names (`assert_`/`verify_`/`run_`/`_harness`), which these do not match, though a body that is nothing but one call delegates by construction whatever the callee is called.
- Compile-time contracts (2): a turbofish between the callee and its parens (`assert_replication_config_ext::<T>()`) broke the delegation regex, and a nested `fn` that is only bound and discarded is the same signature guard as the already-recognised `fn _name()` form.

The script now folds the attribute block into the verification text, allows a turbofish in the delegation patterns, and recognises both a single-call body and a discarded nested-fn binding. Tree-wide candidates drop from 53 to 33, ecstore from 17 to 1.

The one that survives was real: `test_error_conversions` performed two conversions and discarded both results. It now pins what each conversion must produce — a plain `io::Error` stays `DiskError::Io` rather than being guessed at from its `NotFound` kind, a typed error boxed through `io::Error` round-trips back to itself instead of degrading to `Io`, and a serde_json error folds into `other` with its message intact.

Refs backlog#1836
2026-08-19 10:19:42 +08:00
6 changed files with 146 additions and 32 deletions
+23 -4
View File
@@ -853,13 +853,32 @@ mod tests {
#[test]
fn test_error_conversions() {
// Test From implementations
// A plain io::Error carries no typed payload to recover, so it lands in
// `Io` rather than being guessed at from its kind — `NotFound` here must
// not silently become `FileNotFound`, which quorum aggregation counts as
// a different error (rustfs/backlog#1836).
let io_error = std::io::Error::new(std::io::ErrorKind::NotFound, "test");
let _disk_error: DiskError = io_error.into();
let disk_error: DiskError = io_error.into();
match &disk_error {
DiskError::Io(inner) => assert_eq!(inner.kind(), std::io::ErrorKind::NotFound),
other => panic!("a plain io::Error must stay typed as Io, got {other:?}"),
}
let json_str = r#"{"invalid": json}"#; // Invalid JSON
// A typed DiskError boxed through io::Error round-trips back to itself
// instead of degrading to `Io`.
let boxed: std::io::Error = std::io::Error::other(DiskError::VolumeNotFound);
assert_eq!(DiskError::from(boxed), DiskError::VolumeNotFound);
// serde_json errors have no dedicated variant and fold into `other`,
// keeping the original message.
let json_str = r#"{"invalid": json}"#;
let json_error = serde_json::from_str::<serde_json::Value>(json_str).unwrap_err();
let _disk_error: DiskError = json_error.into();
let json_message = json_error.to_string();
let disk_error: DiskError = json_error.into();
assert!(
disk_error.to_string().contains(&json_message),
"the json error message must survive the conversion: {disk_error}"
);
}
#[test]
+35 -20
View File
@@ -436,30 +436,45 @@ mod tests {
assert_eq!(unknown_profile.sequential_boost_multiplier, 1.0);
}
#[cfg(target_os = "linux")]
// What platform probing returns depends on the machine, so these pin the two
// rules that do not: the override wins over probing, and probing that is
// switched off reports Unknown rather than guessing (rustfs/backlog#1836).
#[test]
fn test_linux_storage_detection_exists() {
// This test just verifies the detection function exists and doesn't panic
// The actual result depends on the system it's running on
let result = detect_storage_media(true, "");
// We should get some result (not panic)
match result {
StorageMedia::Nvme | StorageMedia::Ssd | StorageMedia::Hdd | StorageMedia::Unknown => {
// All valid results
}
fn storage_media_override_wins_over_platform_detection() {
for (override_value, expected) in [
("nvme", StorageMedia::Nvme),
("ssd", StorageMedia::Ssd),
("hdd", StorageMedia::Hdd),
] {
assert_eq!(detect_storage_media(true, override_value), expected);
assert_eq!(
detect_storage_media(false, override_value),
expected,
"an override must be honoured even with detection disabled"
);
}
}
#[cfg(target_os = "macos")]
#[test]
fn test_macos_storage_detection_exists() {
// This test just verifies the detection function exists and doesn't panic
let result = detect_storage_media(true, "");
// We should get some result (not panic)
match result {
StorageMedia::Nvme | StorageMedia::Ssd | StorageMedia::Hdd | StorageMedia::Unknown => {
// All valid results
}
}
fn disabled_detection_reports_unknown_instead_of_guessing() {
assert_eq!(detect_storage_media(false, ""), StorageMedia::Unknown);
assert_eq!(
detect_storage_media(false, "not-a-medium"),
StorageMedia::Unknown,
"an unparseable override falls through to the disabled path"
);
}
#[test]
fn enabled_detection_returns_a_medium_for_this_platform() {
// Whatever this machine reports, it must be one of the known variants and
// it must be stable across calls — a probe that flapped would make the
// scheduler's profile depend on when it asked.
let first = detect_storage_media(true, "");
assert!(matches!(
first,
StorageMedia::Nvme | StorageMedia::Ssd | StorageMedia::Hdd | StorageMedia::Unknown
));
assert_eq!(detect_storage_media(true, ""), first);
}
}
+11 -1
View File
@@ -527,9 +527,19 @@ mod tests {
}
#[tokio::test]
async fn runtime_facade_stops_empty_replay_workers() {
async fn stopping_replay_workers_is_a_no_op_when_there_are_none() {
let (facade, _, _) = build_facade();
facade.stop_replay_workers().await;
// The stop path takes the worker list and hands it to the adapter, so an
// empty facade must come back with the list still empty and dispatch
// released rather than left paused (rustfs/backlog#1836).
assert!(facade.replay_workers.read().await.is_empty());
// Calling it twice must stay harmless: shutdown paths do exactly that.
facade.stop_replay_workers().await;
assert!(facade.replay_workers.read().await.is_empty());
}
#[tokio::test]
+10 -3
View File
@@ -873,9 +873,16 @@ mod tests {
/// now return a finite, non-panicking mask.
#[test]
fn test_mask_never_recurses_for_any_variant() {
for ev in ALL_EVENT_NAMES {
// Must terminate (no infinite recursion / stack overflow).
let _ = ev.mask();
// Terminating is the point — a regression here overflows the stack rather
// than failing an assertion — but the masks are collected and checked so
// the loop cannot be optimised into nothing and so a variant that starts
// returning an empty mask is caught too (rustfs/backlog#1836).
let masks: Vec<u64> = ALL_EVENT_NAMES.iter().map(|ev| ev.mask()).collect();
assert_eq!(masks.len(), ALL_EVENT_NAMES.len());
for (ev, mask) in ALL_EVENT_NAMES.iter().zip(&masks) {
assert_ne!(*mask, 0, "{ev:?} must carry at least one bit");
assert_eq!(ev.mask(), *mask, "{ev:?} must return the same mask every call");
}
}
@@ -1766,9 +1766,11 @@ mod tests {
};
use rustfs_io_core::io_profile::{AccessPattern, StorageMedia};
use rustfs_io_metrics::bandwidth::{BandwidthSnapshot, BandwidthTier};
use serial_test::serial;
use std::time::Duration;
#[tokio::test]
#[serial]
async fn test_io_priority_queue_basic() {
let config = IoPriorityQueueConfig::default();
let queue = IoPriorityQueue::new(config);
@@ -1787,6 +1789,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_io_priority_queue_dequeue_order() {
let config = IoPriorityQueueConfig::default();
let queue = IoPriorityQueue::new(config);
@@ -1814,6 +1817,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_io_priority_queue_status() {
let config = IoPriorityQueueConfig::default();
let queue = IoPriorityQueue::new(config);
@@ -1831,6 +1835,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_io_priority_queue_starvation_prevention() {
let config = IoPriorityQueueConfig {
starvation_threshold_secs: 1,
@@ -1854,6 +1859,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_io_priority_from_size() {
// High priority: < 1MB
assert_eq!(IoPriority::from_size(100 * 1024), IoPriority::High);
@@ -1869,6 +1875,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_io_load_level_from_wait_duration() {
use std::time::Duration;
@@ -1886,6 +1893,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_io_scheduler_config_default() {
let config = IoSchedulerConfig::default();
@@ -1899,6 +1907,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_io_scheduler_config_to_core_config() {
let config = IoSchedulerConfig::default();
let core = config.to_core_config();
@@ -1914,6 +1923,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_io_priority_queue_config_to_core_config() {
let config = IoPriorityQueueConfig::default();
let core = config.to_core_config();
@@ -1925,6 +1935,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_io_priority_queue_config_from_scheduler_config() {
let scheduler_config = IoSchedulerConfig {
queue_high_capacity: 128,
@@ -1947,6 +1958,7 @@ mod tests {
// ============================================
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_nvme_sequential_low_load() {
// NVMe + Sequential + Low load = maximum buffer size
let context = IoSchedulingContext {
@@ -1973,6 +1985,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_hdd_random_high_load() {
// HDD + Random + High load = conservative buffer size
let context = IoSchedulingContext {
@@ -1999,6 +2012,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_ssd_mixed_medium_load() {
// SSD + Mixed + Medium load = moderate buffer
let context = IoSchedulingContext {
@@ -2026,6 +2040,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_critical_load_disables_features() {
// Any media + Critical load = minimal features
let context = IoSchedulingContext {
@@ -2050,6 +2065,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_buffer_cap_enforcement() {
// Test that storage media caps are enforced
let context = IoSchedulingContext {
@@ -2074,6 +2090,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_applies_sequential_hint_when_pattern_unknown() {
let context = IoSchedulingContext {
file_size: 2 * 1024 * 1024 * 1024, // 2GiB
@@ -2098,6 +2115,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_bandwidth_low_reduces_buffer() {
// Low bandwidth should reduce buffer
let context = IoSchedulingContext {
@@ -2121,6 +2139,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_high_concurrency_reduction() {
// High concurrency should reduce buffer
let context = IoSchedulingContext {
@@ -2143,6 +2162,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_sequential_boost() {
// Sequential reads should get boost
let sequential_context = IoSchedulingContext {
@@ -2184,6 +2204,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_unknown_media_conservative() {
// Unknown media should be conservative
let context = IoSchedulingContext {
@@ -2209,6 +2230,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_priority_classification() {
// Test priority classification based on file size
let small_context = IoSchedulingContext {
@@ -2255,6 +2277,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_readahead_decision_matrix() {
// Test readahead enable/disable logic
let configs = vec![
@@ -2340,6 +2363,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_buffer_multiplier_stages() {
// Test that all multiplier stages are applied
let context = IoSchedulingContext {
@@ -2374,6 +2398,7 @@ mod tests {
}
#[tokio::test]
#[serial]
async fn test_multi_factor_strategy_compatibility_path() {
// Test that compatibility path (from_wait_duration) still works
let wait_duration = Duration::from_millis(50);
+42 -4
View File
@@ -49,19 +49,47 @@ import sys
from pathlib import Path
VERIFY_SIGNALS = re.compile(
r"assert!|assert_eq!|assert_ne!|debug_assert|panic!\(|\.expect\(|\.unwrap\(|"
r"assert[a-z0-9_]*!|debug_assert|panic!\(|\.expect\(|\.unwrap\(|"
r"unreachable!|matches!\(|insta::|proptest!|\.await\?|\)\?|\?;|should_panic"
)
DELEGATION = re.compile(
r"\b(?:assert|verify|check|expect|ensure|run)_[a-z0-9_]*\s*\(|"
r"\b[a-z0-9_]+_(?:case|cases|harness|roundtrip|round_trip)\s*\("
r"\b(?:assert|verify|check|expect|ensure|run)_[a-z0-9_]*(?:::<[^>]*>)?\s*\(|"
r"\b[a-z0-9_]+_(?:case|cases|harness|roundtrip|round_trip)(?:::<[^>]*>)?\s*\("
)
# A body whose whole content is one call delegates by construction, whatever the
# callee is named: `run(DurabilityMode::Strict).await` and
# `aborting_encode_drops_blocked_producer(EncodePipeline::Vec).await` both hand
# every assertion to a shared harness.
SINGLE_CALL_BODY = re.compile(
r"\A\s*[a-zA-Z_][a-zA-Z0-9_:]*(?:::<[^>]*>)?\s*\([^;]*\)\s*(?:\.await\s*)?;?\s*\Z",
re.S,
)
# A nested `fn` that is only bound and discarded is a signature guard: the type
# system is the assertion, exactly like the `fn _name()` form below.
SIGNATURE_GUARD = re.compile(r"\bfn\s+[a-zA-Z0-9_]+\s*(?:<[^>]*>)?\s*\([^;]*\)[^;]*\{", re.S)
DISCARDED_BINDING = re.compile(r"\blet\s+_\s*=\s*[a-zA-Z_][a-zA-Z0-9_]*\s*;")
# `let _ = Type::<T>::method;` — a path item referenced but never called can only
# be a signature guard; the call form (`let _ = x.foo();`) is excluded by the
# absence of parens before the semicolon.
DISCARDED_PATH_ITEM = re.compile(r"\blet\s+_\s*=\s*[a-zA-Z_][a-zA-Z0-9_]*(?:::(?:<[^>]*>|[a-zA-Z_][a-zA-Z0-9_]*))+\s*;")
COMPILE_TIME_CHECK = re.compile(r"\bfn\s+_[a-zA-Z0-9_]*\s*(?:<[^>]*>)?\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 extract_body(text: str) -> str:
"""Return what is between the outermost braces of a scanned function."""
start = text.find("{")
end = text.rfind("}")
if start == -1 or end <= start:
return text
return text[start + 1 : end]
def scan_file(path: Path):
try:
lines = path.read_text(encoding="utf-8").split("\n")
@@ -105,7 +133,17 @@ def scan_file(path: Path):
break
k += 1
text = "\n".join(body)
if not VERIFY_SIGNALS.search(text) and not DELEGATION.search(text) and not COMPILE_TIME_CHECK.search(text):
# The attribute block carries verification too: `#[should_panic(expected
# = "...")]` makes the panic message the assertion.
attr_text = "\n".join(attrs)
inner = extract_body(text)
delegates = (
DELEGATION.search(text)
or SINGLE_CALL_BODY.match(inner)
or (SIGNATURE_GUARD.search(inner) and DISCARDED_BINDING.search(inner))
or DISCARDED_PATH_ITEM.search(inner)
)
if not VERIFY_SIGNALS.search(text) and not VERIFY_SIGNALS.search(attr_text) and not delegates and not COMPILE_TIME_CHECK.search(text):
print(f"{path}:{j + 1}: {name}")
i = k + 1