Skip to content

fix(scan): v0.41.1 review findings — Robot names only in case sections, no ignore files above the project root, potent walk tests - #1111

Merged
avrabe merged 2 commits into
mainfrom
fix/v0411-review-findings
Oct 22, 2026
Merged

avrabe merged 2 commits into
mainfrom
fix/v0411-review-findings

Conversation

@avrabe

@avrabe avrabe commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #1108 before tagging v0.41.1 (maintainer decision: fix first, then tag). A fresh-context clean-room review of REQ-405/406/407 confirmed REQ-406 and found:

  • REQ-407: any column-0 line counted as a Robot test name, so markers under *** Settings *** / *** Variables *** were attributed to Library Collections / ${X} 1. Names now come only from Test Cases / Tasks / Keywords sections (any *-header spelling); elsewhere a marker falls back to file:line.
  • REQ-405: the walk read ignore files above the scan root (parents(true)), reaching e.g. the home directory without git — a machine dependence. New scan_source_files_in_project: the walk starts at the project root with parents(false) and prunes to the scan root, so project-root .gitignore anchoring is correct for src//tests/ scans and nothing above the project is read. Added tests for .ignore, .git/info/exclude, the global ignore file staying off (child-process test with its own env), and a symlink test that actually fails under follow_links(true).
  • links.rs's // Verifies: REQ-406 becomes a real rivet: verifies marker.

Every new test was run against a negative control (fix broken → named test fails by assertion, restored). CHANGELOG [0.41.1] gains the fix bullet and an Upgrade note: markers inside gitignored or symlinked directories no longer count; use .rivetignore negation and compare rivet coverage --tests across the upgrade.

Gates (1.99.0): fmt, clippy --all-targets -D warnings, nextest 2635 passed, cli_commands 247, validate PASS, docs check PASS, rivet release status v0.41.1 Cuttable, changelog drift clean. Marker count on main 1189 → 1202 (+12 test markers, +1 links.rs).

🤖 Generated with Claude Code

https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9

…the project root are not read; walk-policy tests made potent

An independent review of v0.41.1 found these gaps in the marker scanner.

Robot attribution. find_enclosing_robot_case treated any non-indented line
as a test name, so a marker under Settings was named after "Library
Collections", one under Variables after a variable line, and a marker before
a Settings line took that line through the forward scan. The scan now finds
the nearest section header above the marker and names a case only inside
Test Cases, Test Case, Tasks, Task, Keywords or Keyword. Headers are matched
case-insensitively with any number of leading and optional trailing stars.
Elsewhere the marker falls back to file:line. The backward scan stops at a
header. Tests cover Settings, Variables, a Settings section between two case
sections, and nine header spellings. Negative controls: disabling the
section guard failed five of those tests by assertion, and letting the
backward scan cross a header failed the new backward-scan test by assertion.

Walk policy. New tests cover .ignore and .git/info/exclude (with .git itself
still skipped); disabling either flag failed its test. The global ignore
file is proven off in a child process of the test binary with its own
XDG_CONFIG_HOME, so no environment is shared between parallel tests;
turning git_global on failed it. walk_terminates_on_symlink_loop now builds
sibling links that only a link-following walk traverses and asserts the
exact marker paths; follow_links(true) failed it and the sibling-link test.

Scope of ignore files. The ignore crate read .gitignore and .ignore in
directories above the scan root, which with require_git(false) can reach a
home directory. The walk now starts at the project root, uses parents(false),
and is pruned to the path leading to the scan root, so ignore files from the
project root down apply with their own anchoring and nothing above it is
read. scan_source_files_in_project(project, paths, patterns) carries the
root; scan_source_files keeps its signature and treats each scanned
directory as its own root. Every call site in rivet-cli passes cli.project.
A scan path outside the project, or a symlinked one, is its own root.
Tests cover a stray ignore file above the project, a project-root ignore and
an anchored pattern applying to a sub-root scan, and an outside scan root.
Negative controls: parents(true) failed the first and scanning from the
scan root failed the second.

The links.rs comment for REQ-406 is now a real marker. REQ-405 and REQ-407
descriptions record the new bounds; the changelog gains a bullet and an
upgrade note.

Fixes: REQ-405, REQ-407
Verifies: REQ-405, REQ-406, REQ-407

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

📐 Rivet artifact delta

Change Count
Added 0
Removed 0
Modified 2
Downstream impacted (depth ≤ 5) 0

Graph

graph LR
  REQ_405["REQ-405"]:::modified
  REQ_407["REQ-407"]:::modified
  classDef added fill:#d4edda,stroke:#28a745,color:#155724
  classDef removed fill:#f8d7da,stroke:#dc3545,color:#721c24
  classDef modified fill:#fff3cd,stroke:#ffc107,color:#856404
  classDef overflow fill:#e2e3e5,stroke:#6c757d,color:#495057,stroke-dasharray: 3 3
Loading
Modified
ID Changes
REQ-405
REQ-407

📎 Full HTML dashboard attached as workflow artifact rivet-delta-pr-1111 — download from the workflow run.

Posted by rivet-delta workflow. The graph shows only changed artifacts; open the HTML dashboard (above) for full context.

@codecov

codecov Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.18699% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rivet-core/src/test_scanner.rs 99.18% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

…scanned

PR-diff mutation found that relaxing the scan-root check in scan_directory
from and to or survived: the walk from the project root yields files in the
directories it passes through, and only that check keeps them out of a
sub-root scan. The sub-root test now places a marker at the project root and
expects it to be ignored; with the mutant applied the test fails by
assertion, restored it passes.

Verifies: REQ-405

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9
@avrabe
avrabe merged commit 9e6fb60 into main Oct 22, 2026
33 of 34 checks passed
@avrabe
avrabe deleted the fix/v0411-review-findings branch October 22, 2026 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant