Skip to content

refactor(evaluator): rename test functions to match RuleMatch type (#60)#139

Merged
unclesp1d3r merged 4 commits into
mainfrom
60-refactor-rename-evaluatormatchresult-to-disambiguate-from-outputmatchresult
Mar 5, 2026
Merged

refactor(evaluator): rename test functions to match RuleMatch type (#60)#139
unclesp1d3r merged 4 commits into
mainfrom
60-refactor-rename-evaluatormatchresult-to-disambiguate-from-outputmatchresult

Conversation

@unclesp1d3r
Copy link
Copy Markdown
Member

Summary

  • Rename 3 test functions in src/evaluator/mod.rs from test_match_result_* to test_rule_match_* to align with the MatchResult -> RuleMatch type rename completed earlier
  • Update VS Code settings with deno path

Closes #60

Test Plan

  • All 136 tests pass
  • Clippy clean with -D warnings
  • Pre-commit hooks pass (fmt, clippy, cargo check)

🤖 Generated with Claude Code

Align test function names with the MatchResult -> RuleMatch rename
(#60): test_match_result_* -> test_rule_match_*.

Also update VS Code settings with deno path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Copilot AI review requested due to automatic review settings March 4, 2026 05:43
@unclesp1d3r unclesp1d3r linked an issue Mar 4, 2026 that may be closed by this pull request
5 tasks
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Mar 4, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 35f04e68-de8f-48f4-8d58-6a30814efa95

📥 Commits

Reviewing files that changed from the base of the PR and between 2886fce and c11ab5a.

📒 Files selected for processing (1)
  • docs/src/architecture.md

Summary by CodeRabbit

  • Chores
    • Removed VS Code configuration to restore default editor settings.
    • Renamed several test identifiers for consistency.
  • Documentation
    • Updated architecture docs to reflect a public type rename: MatchResult → RuleMatch (API name change noted).

Walkthrough

Removed the VSCode settings file and renamed evaluator test identifiers and a public type from MatchResult to RuleMatch across docs and evaluator tests; no runtime logic or behavior was changed.

Changes

Cohort / File(s) Summary
Editor Configuration
\.vscode/settings.json
Deleted VS Code workspace settings (Git, Copilot, Ruff, Python interpreter, file exclusions and related UI/tooling flags).
Evaluator Code & Tests
src/evaluator/mod.rs, docs/src/architecture.md
Renamed internal evaluator type MatchResultRuleMatch and updated test function names from test_match_result_*test_rule_match_*; matching doc references updated. No logic changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through config, nibbled the rest,

Deleted a file to tidy my nest,
Tests got new names, a fresh little patch,
From MatchResult now called RuleMatch! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive The PR partially addresses issue #60 by renaming test functions to align with the RuleMatch type rename, but the summary indicates the main type rename from MatchResult to RuleMatch was completed earlier, not in this PR. Clarify whether this PR completes all requirements from issue #60 or if the type rename itself was completed in a prior PR. The acceptance criteria reference updating re-exports and conversions, which are not evident in these changes.
Out of Scope Changes check ❓ Inconclusive The PR includes deletion of .vscode/settings.json and test function renames (in scope), but also mentions updating VS Code settings with deno path, which contradicts the file deletion. Clarify the VS Code settings change: the PR description mentions updating settings with deno path, but the raw summary shows settings.json was deleted. These appear contradictory and need explanation.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly describes the main change: renaming test functions to align with the RuleMatch type rename, which is the primary focus of the changeset.
Description check ✅ Passed The PR description is related to the changeset, explaining the test function renames and mentioning VS Code settings update, both reflected in the code changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 60-refactor-rename-evaluatormatchresult-to-disambiguate-from-outputmatchresult

Comment @coderabbitai help to get the list of available commands and usage tips.

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 4, 2026

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 CI must pass

Wonderful, this rule succeeded.

All CI checks must pass. Release-plz PRs are exempt because they only bump versions and changelogs (code was already tested on main), and GITHUB_TOKEN-triggered force-pushes suppress CI.

  • check-success = coverage
  • check-success = quality
  • check-success = test
  • check-success = test-cross-platform (macos-latest, macOS)
  • check-success = test-cross-platform (ubuntu-22.04, Linux)
  • check-success = test-cross-platform (ubuntu-latest, Linux)
  • check-success = test-cross-platform (windows-latest, Windows)

🟢 Do not merge outdated PRs

Wonderful, this rule succeeded.

Make sure PRs are within 10 commits of the base branch before merging

  • #commits-behind <= 3

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. evaluator Rule evaluation engine and logic testing Test infrastructure and coverage and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Mar 4, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Aligns evaluator unit test names with the earlier internal type rename from MatchResult to RuleMatch, and updates workspace editor configuration.

Changes:

  • Renamed three evaluator test functions from test_match_result_* to test_rule_match_*.
  • Added a Deno path entry to the VS Code workspace settings.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/evaluator/mod.rs Renames test function identifiers to reflect the RuleMatch type name.
.vscode/settings.json Adds deno.path configuration alongside existing workspace tooling settings.
Comments suppressed due to low confidence (1)

.vscode/settings.json:31

  • deno.path is hard-coded to a user-specific absolute path (/Users/kmelton/...), which will not exist for other contributors/CI and can cause VS Code to misconfigure Deno. Prefer removing this setting (letting the extension resolve deno from PATH) or using a portable value (e.g., ${env:HOME}-based or ${workspaceFolder}-based path consistent with the existing mise-tools entries).

@dosubot
Copy link
Copy Markdown
Contributor

dosubot Bot commented Mar 4, 2026

Related Documentation

1 document(s) may need updating based on files changed in this PR:

libMagic-rs

architecture /libmagic-rs/blob/main/docs/src/architecture.md
View Suggested Changes
@@ -123,7 +123,7 @@
 
 **Structure:**
 
-- `mod.rs`: Main evaluation engine with `EvaluationContext` and `MatchResult`
+- `mod.rs`: Main evaluation engine with `EvaluationContext` and `RuleMatch`
 - `offset.rs`: Offset resolution (absolute, relative, from-end)
 - `types.rs`: Type interpretation with endianness handling and signedness coercion
 - `operators.rs`: Comparison and bitwise operations

✅ Accepted

Note: You must be authenticated to accept/decline updates.

How did I do? Any feedback?  Join Discord

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

coderabbitai[bot]
coderabbitai Bot previously approved these changes Mar 4, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 4, 2026

🧪 CI Insights

Here's what we observed from your CI run for 109a1c2.

🟢 All jobs passed!

But CI Insights is watching 👀

coderabbitai[bot]
coderabbitai Bot previously approved these changes Mar 5, 2026
Reverts the accidental deletion of .vscode/settings.json and drops
the deno.path setting that was not needed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Copilot AI review requested due to automatic review settings March 5, 2026 00:31
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Mar 5, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread src/evaluator/mod.rs

#[test]
fn test_match_result_creation() {
fn test_rule_match_creation() {
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description mentions "Update VS Code settings with deno path", but the PR diff/files shown here don’t include any change adding a Deno path (and current .vscode/settings.json contains no deno.* setting). Either update the description to match the actual changes, or include the intended VS Code settings change in this PR.

Copilot uses AI. Check for mistakes.
@unclesp1d3r unclesp1d3r merged commit 763eaf6 into main Mar 5, 2026
32 checks passed
@unclesp1d3r unclesp1d3r deleted the 60-refactor-rename-evaluatormatchresult-to-disambiguate-from-outputmatchresult branch March 5, 2026 01:13
@github-actions github-actions Bot mentioned this pull request Mar 5, 2026
mergify Bot pushed a commit that referenced this pull request Mar 5, 2026
## 🤖 New release

* `libmagic-rs`: 0.3.0 -> 0.3.1 (✓ API compatible changes)

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [0.3.1] - 2026-03-05

### Refactor

- **evaluator**: Rename test functions to match RuleMatch type
([#60](#60))
([#139](#139))
<!-- generated by git-cliff -->
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

evaluator Rule evaluation engine and logic size:XS This PR changes 0-9 lines, ignoring generated files. testing Test infrastructure and coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: rename evaluator::MatchResult to disambiguate from output::MatchResult

2 participants