Conversation
- Add cache-workspace-crates input (default true) for rust-cache - Add cache-directories input for extra cache paths - Add Python helper to resolve cache directories - Add unit tests for the cache directory resolver - Update README with rustdoc caching documentation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 4 seconds. ⌛ 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe PR adds two new action inputs: Possibly related PRs
🚥 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
🧹 Nitpick comments (2)
action.yml (1)
131-138: Gate cache-directory normalization behind cache enablement.This step still runs when
setup-rust-cacheis disabled. In no-cache mode, it’s unnecessary work and avoidable failure surface.♻️ Proposed change
- name: Normalize rust-cache directories + if: inputs.setup-rust-cache == 'true' shell: bash run: python3 "${{ github.action_path }}/scripts/resolve_cache_directories.py" env: INPUT_RUST_PROJECT_PATH: ${{ inputs.rust-project-path }} INPUT_TARGET: ${{ inputs.target }} INPUT_CACHE_DIRECTORIES: ${{ inputs.cache-directories }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@action.yml` around lines 131 - 138, The "Normalize rust-cache directories" step is running unconditionally; gate it by the cache enablement input so it only runs when caching is enabled. Update the step named "Normalize rust-cache directories" to include a conditional (if:) that checks the workflow input (e.g., inputs.setup-rust-cache == 'true') before executing the python script that uses INPUT_RUST_PROJECT_PATH, INPUT_TARGET and INPUT_CACHE_DIRECTORIES, so the step is skipped in no-cache mode.tests/test_resolve_cache_directories.py (1)
44-48: Add a Windows absolute-path test case.Current absolute-path coverage only checks POSIX-style paths. A Windows-style absolute path case would prevent regressions in cross-platform path handling.
🧪 Suggested test addition
def test_preserves_absolute_paths(self) -> None: self.assertEqual( resolve_cache_directories("src", "", "/tmp/cache-dir\n"), ["src/target/doc", "/tmp/cache-dir"], ) + + def test_preserves_windows_absolute_paths(self) -> None: + self.assertEqual( + resolve_cache_directories("src", "", "C:\\cache-dir\n"), + ["src/target/doc", "C:/cache-dir"], + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_resolve_cache_directories.py` around lines 44 - 48, Extend the absolute-path test for resolve_cache_directories to include a Windows-style absolute path case (e.g., "C:\\\\cache-dir" or "C:/cache-dir") so the test_preserves_absolute_paths assertion also checks that Windows absolute paths are returned unchanged; update the call to resolve_cache_directories in tests/test_resolve_cache_directories.py to include a Windows absolute path in the expected list alongside "src/target/doc" and "/tmp/cache-dir" and add any necessary escaping for backslashes in the test string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/resolve_cache_directories.py`:
- Around line 115-121: The current absolute-path check uses only
posixpath.isabs(path) and misclassifies Windows absolute paths like
"C:\cache-dir"; update the branching in the function containing
posixpath.isabs(path) to treat Windows-style absolute paths as absolute by also
checking ntpath.isabs(path) (and/or posixpath.isabs on a normalized input);
ensure you compute the normalized input used for posix checks, then call
_normalize_path(path) for absolute cases (whether posix or nt) and only join
project_path for truly relative paths (keep references to posixpath.isabs,
ntpath.isabs, _normalize_path, project_path).
---
Nitpick comments:
In `@action.yml`:
- Around line 131-138: The "Normalize rust-cache directories" step is running
unconditionally; gate it by the cache enablement input so it only runs when
caching is enabled. Update the step named "Normalize rust-cache directories" to
include a conditional (if:) that checks the workflow input (e.g.,
inputs.setup-rust-cache == 'true') before executing the python script that uses
INPUT_RUST_PROJECT_PATH, INPUT_TARGET and INPUT_CACHE_DIRECTORIES, so the step
is skipped in no-cache mode.
In `@tests/test_resolve_cache_directories.py`:
- Around line 44-48: Extend the absolute-path test for resolve_cache_directories
to include a Windows-style absolute path case (e.g., "C:\\\\cache-dir" or
"C:/cache-dir") so the test_preserves_absolute_paths assertion also checks that
Windows absolute paths are returned unchanged; update the call to
resolve_cache_directories in tests/test_resolve_cache_directories.py to include
a Windows absolute path in the expected list alongside "src/target/doc" and
"/tmp/cache-dir" and add any necessary escaping for backslashes in the test
string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9537e563-6f82-4d8d-a125-adbe0f9fce34
📒 Files selected for processing (7)
.github/workflows/test-packages-input.yml.github/workflows/test-workflow.yml.gitignoreREADME.MDaction.ymlscripts/resolve_cache_directories.pytests/test_resolve_cache_directories.py
Windows-style paths like `C:\cache-dir` were misclassified as relative by the POSIX-only `isabs` check, causing them to be incorrectly joined with `project_path`. Added `ntpath.isabs` as a fallback check. EOF'
The "Normalize rust-cache directories" step was running unconditionally regardless of the setup-rust-cache input setting. Added conditional check so it only executes when caching is enabled (inputs.setup-rust-cache == 'true'), matching the behavior of the dependent "Setup Rust Caching" step.
Replace four separate test methods with a single comprehensive test: - test_preserves_absolute_paths - test_preserves_windows_absolute_paths - test_windows_absolute_path_not_joined_with_project_path - test_mixed_posix_and_windows_absolute_paths Now covered by test_preserves_posix_and_windows_absolute_paths which tests: - POSIX absolute paths (/tmp/cache-dir) - Windows absolute paths (C:\cache-dir) - Backslash normalization to forward slashes - Relative paths still joined to project root
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v1-master #12 +/- ##
===============================
===============================
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary
cache-workspace-cratesinput (defaulttrue) to preserve workspace crate artifacts in rust-cachecache-directoriesinput for specifying additional cache paths relative to the project rootcargo docrunsChanges
action.ymlfor cache configurationscripts/resolve_cache_directories.pyhandles path resolutionTesting
Unit tests run automatically in
test-workflow.ymlon path changes affecting the helper script.