Test infrastructure, compatibility tests, and architecture improvements#31
Conversation
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Add test infrastructure components for Phase 1 MVP completion: Benchmarks (Criterion): - parser_bench.rs: Magic file parsing performance - evaluation_bench.rs: Rule evaluation against file types - io_bench.rs: Memory-mapped I/O operations Property Tests (proptest): - 15 property tests for serialization, evaluation, and safety - Tests for ELF/ZIP detection, config validation, buffer handling CI/CD Enhancements: - Enable compatibility test workflow (remove if: false) - Add benchmarks.yml for weekly runs and PR comparisons - Add coverage-report and coverage-summary recipes to justfile Documentation: - Update testing.md with current implementation status - Add benchmark documentation section Closes #22 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Caution Review failedFailed to post review comments Summary by CodeRabbit
WalkthroughSplits the parser into format/preprocessing/hierarchy/loader modules, adds Criterion benchmarks and a Benchmarks CI workflow with PR-vs-main comparison, changes evaluator APIs to Option-based matches and borrowed EvaluationConfig, adds MIME caching and output conversion helpers, and introduces extensive tests, docs, and tooling updates. Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant GH as GitHub Actions
participant Runner as Runner (ubuntu-latest)
participant CheckoutPR as Checkout PR
participant Setup as Setup Tools & Cache
participant BenchPR as Run benches (baseline=pr)
participant UploadPR as Upload PR artifact
participant CheckoutMain as Checkout main
participant BenchMain as Run benches (baseline=main)
participant Critcmp as critcmp Compare & Report
PR->>GH: trigger (PR update / manual / schedule)
GH->>Runner: start benchmark-compare job
Runner->>CheckoutPR: checkout PR branch (fetch-depth: 0)
Runner->>Setup: install tools (mise), restore caches
Runner->>BenchPR: run `cargo bench` (save baseline=pr)
BenchPR->>UploadPR: upload `target/criterion/` (pr)
Runner->>CheckoutMain: checkout main branch
Runner->>Setup: ensure tools & caches
Runner->>BenchMain: run `cargo bench` (save baseline=main)
BenchMain->>Critcmp: run `critcmp` to compare baselines
Critcmp->>GH: append formatted comparison to PR summary
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Generated SKILL.md with project patterns (architecture, co-change maps, clippy config, error handling, testing conventions) and 8 instinct files for strict clippy compliance, error handling, co-change awareness, testing, AST derives, build script testing, just tasks, and commits.
chore(deps): reorder dependencies in mise.toml for clarity Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
- clap: 4.5.54 -> 4.5.57 - criterion: 0.8.1 -> 0.8.2 - insta: 1.46.1 -> 1.46.3 - proptest: 1.9.0 -> 1.10.0 - regex: 1.12.2 -> 1.12.3 - tempfile: 3.24.0 -> 3.25.0 - Remove temp-env (unused, no imports in codebase)
Remove 10 tests that were not providing additional coverage: - 5 fake proptests using unused _seed parameter (converted 4 to regular #[test], removed 1 covered by prop_evaluation_never_panics) - 4 individual serde roundtrip tests already covered by prop_rule_serde_roundtrip - 2 random-prefix tests that are subsets of prop_evaluation_never_panics 346 -> 218 lines, 868 tests still pass.
- Cache MimeMapper on MagicDatabase to avoid rebuilding HashMap per evaluation - Pass EvaluationConfig by reference to eliminate clones at call sites - Remove thread-spawn timeout path (per-rule timeout check is sufficient) - Replace strip_suffix().to_string() with pop() for continuation lines - Track last line number during parsing instead of re-scanning on error - Pre-allocate concatenate_messages String with exact capacity - Use smaller initial Vec capacity for matches (8 vs rules.len()) - Check timeout every 16 rules instead of every rule
- Fix lossy IoError conversion: preserve ErrorKind and error chain by wrapping full IoError as source via Error::new(kind, err) instead of flattening to string with Error::other(err.to_string()) - Add output conversion methods: MatchResult::from_evaluator_match() and EvaluationResult::from_library_result() formalize the adapter pattern, removing ~40 lines of manual conversion boilerplate from main.rs - Split parser/mod.rs (2098 lines) into focused submodules: - format.rs: MagicFileFormat enum, detect_format() - preprocessing.rs: LineInfo, preprocess_lines(), parse_magic_rule_line() - hierarchy.rs: build_rule_hierarchy() - loader.rs: load_magic_directory(), load_magic_file() - mod.rs: public interface, re-exports, parse_text_magic_file()
Remove --noplot flag from criterion bench invocations (removed in criterion 0.8.2). Switch compatibility tests from --magic-file with binary .mgc to --use-builtin, since the parser rejects binary magic files by design.
…tags, and end-to-end flows Add 72 new integration tests across 4 test files: - evaluator_tests.rs (21 tests): confidence calculation, rule ordering, config variants, metadata, edge cases - mime_tests.rs (15 tests): MIME type detection via MagicDatabase API, MimeMapper direct tests for executables, archives, images, documents - tags_tests.rs (13 tests): keyword extraction, case insensitivity, custom keywords, rule path tags - integration_tests.rs (23 tests): end-to-end flows including builtin rules, custom magic files, hierarchy evaluation, directory loading, file evaluation, metadata, and error cases Total test count: 940 (all passing) Coverage: 93.86% (target: >85%)
The parser requires quoted strings for string-type values. Unquoted bare words like `test` are ambiguous and fail with a parse error.
Remove --noplot flag removed in criterion 0.8.2. Use explicit --bench targets for benchmark comparison to avoid passing --save-baseline to the lib unit test binary which doesn't recognize criterion flags. Fix unquoted string value in parser benchmark that fails to parse.
The benchmark comparison job checks out main to run benchmarks for comparison. If main doesn't have the bench targets yet, the step now gracefully skips the comparison instead of failing the workflow. Also use explicit bench targets in the run job for consistency.
The byteorder crate is actively used in src/evaluator/types.rs for endianness-aware binary data reading. The machete ignore was added when the crate was planned but not yet used; it is no longer needed.
- Eliminate redundant offset/value resolution in evaluator by returning Option<(usize, Value)> from evaluate_single_rule instead of bool - Fix lossy IoError conversion with FileError variant preserving context - Remove eprintln! calls and dead code from library evaluator - Add cross-type integer coercion (Uint/Int) in operators via i128 - Add ConfigError variant for config validation instead of misusing ParseError - Extract build_result method to deduplicate result construction logic - Cache TagExtractor with LazyLock to avoid per-call HashSet allocation
- Remove redundant double bounds check in read_byte (manual check + .get()) - Use trailing_zeros() for timeout interval check instead of modulo - Add memchr for SIMD-accelerated null terminator scanning in read_string - Simplify duplicate try_from in BitwiseAndMask operator to single call - Add release profile with lto="thin" and codegen-units=1
- Replace duplicated empty-rules early returns with build_result reuse - Use then_some() for idiomatic match-to-option conversion - Remove debug-only eprintln from set_confidence (library code)
- Add rustdoc examples to ConfigError and FileError variants - Fix incorrect error mapping in format.rs: use ParseError::IoError instead of ParseError::invalid_syntax for I/O read failures
Summary
then_some(), reusebuild_result, remove library eprintln callsTest plan
just ci-checkpasses (940 tests, clippy, fmt, cargo check)🤖 Generated with Claude Code