Fix scenario filtering to use exact matching without wildcards

[jeremyandrews]

PR Description: Fix scenario filtering substring matching bug

Problem

Issue #612: When filtering scenarios, --scenarios put1024bytes incorrectly matched both put1024bytes and getput1024bytes due to substring matching instead of exact matching.

Root Cause

scenario_is_active() method used scenario_name.contains(&scenario), causing substring matches.

Solution

Replace substring matching with exact matching for non-wildcard patterns:

• No wildcard: Exact string comparison only
• With * wildcard: Pattern matching (maintains backward compatibility)

Technical Changes

• src/lib.rs: Replace contains() with wildcard_match() helper
• src/config.rs: Allow * characters in scenario validation
• tests/scenarios.rs: Add comprehensive test coverage (14 new tests)
• Documentation: Update scenario filtering behavior description

Behavior Change

Before: --scenarios put1024bytes matches put1024bytes, getput1024bytes, preput1024bytes
After: --scenarios put1024bytes matches only put1024bytes

Backward Compatibility: --scenarios * continues to match all scenarios

Test Coverage

• Exact matching without wildcards
• Wildcard patterns: *suffix, prefix*, prefix*suffix
• Multiple wildcards: *prefix*middle*suffix*
• Edge cases and complex patterns

Fixes #612

[github-actions]

📋 Review Summary

This pull request effectively addresses the scenario filtering issue by replacing substring matching with exact matching and introducing wildcard support. The changes are well-implemented, and the addition of comprehensive tests ensures the new functionality is robust.

🔍 General Feedback

• The core logic in wildcard_match is sound and handles various wildcard patterns correctly.
• The documentation has been updated clearly to reflect the new matching behavior.
• The test coverage is excellent, covering a wide range of scenarios and edge cases.

Overall, this is a high-quality contribution that improves the usability and correctness of scenario filtering.

[LionsAd]

Comprehensive Review: Scenario Wildcard Matching

✅ Core Implementation Quality

The PR successfully implements wildcard matching for scenario names with excellent test coverage and documentation. All technical checks pass (cargo test, clippy, fmt).

🔶 Architectural Recommendations

Custom Wildcard Implementation: Multiple reviewers suggest considering a specialized crate like wildmatch or glob instead of the custom implementation. This would:

• Reduce maintenance burden
• Eliminate potential edge case bugs
• Leverage proven implementations

Test Suite Structure: The comprehensive test suite could be streamlined using parameterized tests or helper functions to reduce code duplication while maintaining coverage.

Final Assessment

Excellent functionality that successfully resolves issue #612 with comprehensive testing and documentation. The architectural suggestions are non-blocking improvements that could be addressed in follow-up PRs.

Recommendation: Approve and merge - the current implementation is solid and delivers valuable wildcard support.

(Powered by Claude Code)

PS: Some details follow in two additional comments:

[LionsAd]

Consider Third-Party Crate

The custom wildcard_match implementation works well and is thoroughly tested, but consider using a specialized crate like wildmatch or glob instead. This would:

• Reduce maintenance overhead
• Leverage battle-tested implementations
• Handle edge cases more robustly

(Powered by Claude Code)

[LionsAd]

Test Suite Optimization

Excellent test coverage! Consider refactoring the test suite to use parameterized tests or helper functions to reduce code duplication:

#[test] 
fn test_wildcard_patterns() {
    let test_cases = vec![
        ("*", vec!["scenarioa1", "scenariob1"]),
        ("*a1", vec!["scenarioa1"]),
        // ... more cases
    ];
    
    for (pattern, expected) in test_cases {
        // test logic
    }
}

(Powered by Claude Code)

[jeremyandrews]

Completed wildcard test deduplication and enhanced functionality. Replaced custom wildcard implementation with wildcard crate, added support for ? and [abc] patterns, eliminated 500+ lines of test boilerplate through helper functions, and updated documentation. All 19 tests pass with comprehensive coverage maintained.

[LionsAd]

Technical Review: Pattern Pre-compilation and Code Improvements

1. Performance Optimization - Pre-compile Wildcard Patterns

Current Issue: wildcard::Wildcard::new() called repeatedly in scenario_is_active()

Suggested Change in src/config.rs:

pub enum ScenarioPattern {
    Exact(String),
    Wildcard(wildcard::Wildcard),
}

pub struct Scenarios {
    pub active: Vec<String>,
    pub compiled_patterns: Vec<ScenarioPattern>,
}

impl FromStr for Scenarios {
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let mut active: Vec<String> = Vec::new();
        let mut compiled_patterns: Vec<ScenarioPattern> = Vec::new();
        
        for line in s.split(',') {
            let scenario = line.trim().to_lowercase();
            
            if scenario.contains('*') {
                match wildcard::Wildcard::new(scenario.as_bytes()) {
                    Ok(pattern) => {
                        active.push(scenario);
                        compiled_patterns.push(ScenarioPattern::Wildcard(pattern));
                    }
                    Err(_) => return Err(/* invalid pattern error */),
                }
            } else {
                active.push(scenario.clone());
                compiled_patterns.push(ScenarioPattern::Exact(scenario));
            }
        }
        
        Ok(Scenarios { active, compiled_patterns })
    }
}

Update matching logic in src/lib.rs:

fn scenario_is_active(&self, scenario: &Scenario) -> bool {
    if self.configuration.scenarios.compiled_patterns.is_empty() {
        return true;
    }
    
    self.configuration.scenarios.compiled_patterns.iter().any(|pattern| {
        match pattern {
            ScenarioPattern::Exact(name) => scenario.machine_name == *name,
            ScenarioPattern::Wildcard(wildcard) => wildcard.is_match(scenario.machine_name.as_bytes()),
        }
    })
}

2. Remove Unnecessary extern crate

Current Code in src/lib.rs:

extern crate wildcard;

Suggested Change:

// Remove the extern crate line completely
// The existing use statements are sufficient in Rust 2018+

3. Clarify Character Validation

Current Code in src/config.rs:

if scenario.chars().all(|c| {
    char::is_alphanumeric(c)
        || c == '*'
        || c == '?'
        || c == '['
        || c == ']'
        || c == '-'
        || c == '\!'
        || c == '_'
}) {

Suggested Change:

// Suggested - be more explicit about purpose
if scenario.chars().all(|c| {
    char::is_alphanumeric(c)
        || c == '*'   // Wildcard pattern matching
        || c == '_'   // Valid scenario name character
        || c == '-'   // Valid scenario name character
}) {

Update error message:

eprintln\!("{{one}}, {{two}}, {{three}}, etc must be alphanumeric or contain wildcard (*) for pattern matching");

These changes improve performance, modernize the code, and clarify wildcard support scope. The core fix is solid.

(Powered by Claude Code)