fix(bench): validate --suites names, reject unknown suites at CLI boundary
Introduce Suite enum (runner.rs) replacing stringly-typed suite matching. BenchConfig.suites is now Option<Vec<Suite>>, making invalid states unrepresentable. Unknown suite names produce a clear error listing all valid values. Fixes: #37
This commit is contained in:
@@ -25,6 +25,21 @@ struct Args {
|
||||
fn main() {
|
||||
let args = Args::parse();
|
||||
|
||||
let suites = match args.suites {
|
||||
Some(names) => {
|
||||
let parsed: Result<Vec<_>, _> =
|
||||
names.iter().map(|s| s.parse::<log_viewer_bench::runner::Suite>()).collect();
|
||||
match parsed {
|
||||
Ok(s) => Some(s),
|
||||
Err(e) => {
|
||||
eprintln!("error: {e}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
|
||||
println!("=== Benchmark: mmap vs pread ===");
|
||||
println!("Test file: {}", args.test_file.display());
|
||||
println!("Quick mode: {}", args.quick);
|
||||
@@ -33,7 +48,7 @@ fn main() {
|
||||
let config = log_viewer_bench::runner::BenchConfig {
|
||||
test_file: args.test_file.clone(),
|
||||
quick_mode: args.quick,
|
||||
suites: args.suites,
|
||||
suites,
|
||||
};
|
||||
|
||||
if !config.test_file.exists() {
|
||||
|
||||
@@ -2,10 +2,67 @@ use crate::metrics::MetricsCollector;
|
||||
use crate::types::BenchmarkResult;
|
||||
use std::path::PathBuf;
|
||||
|
||||
/// All recognized benchmark suite identifiers.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum Suite {
|
||||
Startup,
|
||||
Render,
|
||||
Jump,
|
||||
Memory,
|
||||
Growth,
|
||||
Rotation,
|
||||
Concurrent,
|
||||
}
|
||||
|
||||
impl Suite {
|
||||
/// All valid suite identifiers, in execution order.
|
||||
pub const ALL: &[Suite] = &[
|
||||
Suite::Startup,
|
||||
Suite::Render,
|
||||
Suite::Jump,
|
||||
Suite::Memory,
|
||||
Suite::Growth,
|
||||
Suite::Rotation,
|
||||
Suite::Concurrent,
|
||||
];
|
||||
|
||||
#[must_use]
|
||||
pub const fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Suite::Startup => "startup",
|
||||
Suite::Render => "render",
|
||||
Suite::Jump => "jump",
|
||||
Suite::Memory => "memory",
|
||||
Suite::Growth => "growth",
|
||||
Suite::Rotation => "rotation",
|
||||
Suite::Concurrent => "concurrent",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl std::str::FromStr for Suite {
|
||||
type Err = String;
|
||||
|
||||
fn from_str(s: &str) -> Result<Self, Self::Err> {
|
||||
Suite::ALL
|
||||
.iter()
|
||||
.find(|suite| suite.as_str() == s)
|
||||
.copied()
|
||||
.ok_or_else(|| {
|
||||
let valid = Suite::ALL
|
||||
.iter()
|
||||
.map(|s| s.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ");
|
||||
format!("invalid value '{s}' for '--suites': valid values are {valid}")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
pub struct BenchConfig {
|
||||
pub test_file: PathBuf,
|
||||
pub quick_mode: bool,
|
||||
pub suites: Option<Vec<String>>,
|
||||
pub suites: Option<Vec<Suite>>,
|
||||
}
|
||||
|
||||
fn warn_reset_hwm() {
|
||||
@@ -19,41 +76,91 @@ fn warn_reset_hwm() {
|
||||
pub fn run_all(config: &BenchConfig) -> Vec<BenchmarkResult> {
|
||||
let mut results = Vec::new();
|
||||
|
||||
let should_run = |name: &str| -> bool {
|
||||
let should_run = |suite: Suite| -> bool {
|
||||
match &config.suites {
|
||||
Some(suites) => suites.iter().any(|s| s == name),
|
||||
Some(suites) => suites.contains(&suite),
|
||||
None => true,
|
||||
}
|
||||
};
|
||||
|
||||
if should_run("startup") {
|
||||
if should_run(Suite::Startup) {
|
||||
warn_reset_hwm();
|
||||
results.extend(crate::suites::startup::run(config));
|
||||
}
|
||||
if should_run("render") {
|
||||
if should_run(Suite::Render) {
|
||||
warn_reset_hwm();
|
||||
results.extend(crate::suites::render::run(config));
|
||||
}
|
||||
if should_run("jump") {
|
||||
if should_run(Suite::Jump) {
|
||||
warn_reset_hwm();
|
||||
results.extend(crate::suites::jump::run(config));
|
||||
}
|
||||
if should_run("memory") {
|
||||
if should_run(Suite::Memory) {
|
||||
warn_reset_hwm();
|
||||
results.extend(crate::suites::memory::run(config));
|
||||
}
|
||||
if should_run("growth") {
|
||||
if should_run(Suite::Growth) {
|
||||
warn_reset_hwm();
|
||||
results.extend(crate::suites::growth::run(config));
|
||||
}
|
||||
if should_run("rotation") {
|
||||
if should_run(Suite::Rotation) {
|
||||
warn_reset_hwm();
|
||||
results.extend(crate::suites::rotation::run(config));
|
||||
}
|
||||
if should_run("concurrent") {
|
||||
if should_run(Suite::Concurrent) {
|
||||
warn_reset_hwm();
|
||||
results.extend(crate::suites::concurrent::run(config));
|
||||
}
|
||||
|
||||
results
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::Suite;
|
||||
use std::str::FromStr;
|
||||
|
||||
#[test]
|
||||
fn parse_all_valid_suites() {
|
||||
let expected = [
|
||||
("startup", Suite::Startup),
|
||||
("render", Suite::Render),
|
||||
("jump", Suite::Jump),
|
||||
("memory", Suite::Memory),
|
||||
("growth", Suite::Growth),
|
||||
("rotation", Suite::Rotation),
|
||||
("concurrent", Suite::Concurrent),
|
||||
];
|
||||
for (s, expected_suite) in expected {
|
||||
assert_eq!(Suite::from_str(s).unwrap(), expected_suite, "failed to parse '{s}'");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn misspelled_suite_returns_error() {
|
||||
let err = Suite::from_str("startp").unwrap_err();
|
||||
assert!(
|
||||
err.contains("invalid value 'startp'"),
|
||||
"error should mention the bad value: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn error_message_lists_all_valid_values() {
|
||||
let err = Suite::from_str("bogus").unwrap_err();
|
||||
for name in Suite::ALL.iter().map(|s| s.as_str()) {
|
||||
assert!(
|
||||
err.contains(name),
|
||||
"error should list valid suite '{name}': {err}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mixed_valid_invalid_stops_at_first_error() {
|
||||
// "startup" is valid, "zzz" is not — collect hits the first Err
|
||||
let names = ["startup".to_string(), "zzz".to_string()];
|
||||
let result: Result<Vec<Suite>, _> = names.iter().map(|s| s.parse()).collect();
|
||||
assert!(result.is_err());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user