This opens a clean PR to fix #60. Should close old PR #61#62
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRefactors pattern handling: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration_tests.rs (1)
363-380:⚠️ Potential issue | 🟠 MajorFix copy-paste bug: assertions use wrong variable names.
The assertions at lines 364-365 and 379-380 reference
output_pipe_excludebut should reference the local variablesoutput_pipe_edgesandoutput_double_piperespectively. This may cause the tests to pass coincidentally while not actually validating the intended behavior.🐛 Proposed fix
assert!(output_not_contains(&output_pipe_edges, "src")); assert!(output_not_contains(&output_pipe_edges, "docs")); // Other entries should still exist - assert!(output_contains(&output_pipe_exclude, "tests")); - assert!(output_contains(&output_pipe_exclude, "README.md")); + assert!(output_contains(&output_pipe_edges, "tests")); + assert!(output_contains(&output_pipe_edges, "README.md")); // Consecutive pipes (empty segment between them) should be ignored let output_double_pipe = cmd() .args(["-I", "src||docs", temp_dir.path().to_str().unwrap()]) .assert() .success() .get_output() .stdout .clone(); assert!(output_not_contains(&output_double_pipe, "src")); assert!(output_not_contains(&output_double_pipe, "docs")); // Other entries should still exist - assert!(output_contains(&output_pipe_exclude, "tests")); - assert!(output_contains(&output_pipe_exclude, "README.md")); + assert!(output_contains(&output_double_pipe, "tests")); + assert!(output_contains(&output_double_pipe, "README.md"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration_tests.rs` around lines 363 - 380, Fix the copy-paste mistake in the test assertions: replace the incorrect references to output_pipe_exclude with the correct local variables — use output_pipe_edges for the assertions validating the pipe-edge behavior and output_double_pipe for the assertions validating consecutive-pipe behavior; specifically update the calls to output_contains and output_not_contains that currently reference output_pipe_exclude so they instead reference output_pipe_edges and output_double_pipe respectively (these variables are defined near the cmd().args(...) blocks and paired with output_contains/output_not_contains assertions).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration_tests.rs`:
- Around line 315-321: The formatting error is caused by incorrect blank lines
around the test declaration: ensure there is exactly one blank line before the
#[test] attribute for the function test_exclude_pattern_matching and remove the
stray extra blank line after the call to create_test_structure() inside that
test; then run cargo fmt to apply formatting across the crate so the pipeline
passes.
---
Outside diff comments:
In `@tests/integration_tests.rs`:
- Around line 363-380: Fix the copy-paste mistake in the test assertions:
replace the incorrect references to output_pipe_exclude with the correct local
variables — use output_pipe_edges for the assertions validating the pipe-edge
behavior and output_double_pipe for the assertions validating consecutive-pipe
behavior; specifically update the calls to output_contains and
output_not_contains that currently reference output_pipe_exclude so they instead
reference output_pipe_edges and output_double_pipe respectively (these variables
are defined near the cmd().args(...) blocks and paired with
output_contains/output_not_contains assertions).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65f9fb34-7cb9-4f42-adcd-0e0a49db1812
📒 Files selected for processing (7)
src/rust_tree/cli.rssrc/rust_tree/options.rssrc/rust_tree/traversal.rstests/cli_unit_tests.rstests/integration_tests.rstests/traversal_tests.rstests/unit_tests.rs
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/integration_tests.rs (1)
342-344:⚠️ Potential issue | 🟡 MinorRun
cargo fmthere.There is still no blank line between the end of
test_include_pattern_matchingand the next#[test], which is the same formatting issue previously flagged on this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration_tests.rs` around lines 342 - 344, The file is missing a blank line between the end of the test_include_pattern_matching function and the next test annotation; open tests/integration_tests.rs, locate the closing brace of test_include_pattern_matching and the next #[test] for test_exclude_pattern_matching, and insert a single blank line (or simply run rustfmt via cargo fmt) so there is one empty line separating the two tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration_tests.rs`:
- Around line 182-341: The test suite is missing a case that verifies repeating
include/exclude flags are flattened (cli_to_options collects a Vec<Pattern>);
add one integration test using repeated -P (e.g. cmd().args(["--prune", "-P",
"*.rs", "-P", "*.md", temp_dir.path().to_str().unwrap()]) ) and one using
repeated -I or -P for directories (e.g. cmd().args(["-I", "src", "-I", "docs",
...])) that assert combined behavior via the existing helpers
output_contains/output_not_contains and cmd()/get_output(), ensuring patterns
from multiple occurrences are applied rather than only the last occurrence.
---
Duplicate comments:
In `@tests/integration_tests.rs`:
- Around line 342-344: The file is missing a blank line between the end of the
test_include_pattern_matching function and the next test annotation; open
tests/integration_tests.rs, locate the closing brace of
test_include_pattern_matching and the next #[test] for
test_exclude_pattern_matching, and insert a single blank line (or simply run
rustfmt via cargo fmt) so there is one empty line separating the two tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da2f291a-c1e7-47b0-acd4-243c9e5ce2ad
📒 Files selected for processing (1)
tests/integration_tests.rs
2 participants
The latest commit adds new integration tests and fixes prev issue with --fromfile flag
Summary by CodeRabbit
New Features
Refactor
Tests