Conversation
Add 21 additional tests to improve coverage from 17 to 38 tests: - Add tests for delete_image (success and error cases) - Add tests for load_hito_config (nonexistent, with file, custom filename, invalid JSON) - Add tests for save_hito_config (default and custom filename) - Add edge case tests for sort_images (unknown option, None values) - Add edge case tests for list_images (empty dir, files without extensions) - Add edge case tests for load_image (directory error) - Add filter edge case tests (empty patterns, invalid values) - Improve path handling test coverage Coverage results: - Regions: 89.65% (2000 total, 207 missed) - Functions: 79.63% (108 total, 22 missed) - Lines: 89.53% (1356 total, 142 missed) Remaining uncovered code is primarily Tauri integration functions requiring AppHandle, which is appropriate to test via integration tests.
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #32 +/- ##
==========================================
+ Coverage 91.76% 94.40% +2.64%
==========================================
Files 19 19
Lines 2708 3041 +333
Branches 534 534
==========================================
+ Hits 2485 2871 +386
+ Misses 223 170 -53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src-tauri/src/lib.rs (2)
1518-1554:delete_imagetests cover main paths; watch for environment‑specific behaviorThe three new tests for
delete_image(happy path, nonexistent path, and directory input) nicely pin down the function’s contract and its error messages. The success test also correctly asserts that the original path no longer exists after deletion, which matches the intended semantics of “send to trash”.Just be aware these tests are delegating to the underlying deletion mechanism and filesystem; if you ever see sporadic failures on unusual CI environments or platforms, you may want to gate the happy‑path test with a platform/config flag rather than weakening the assertions.
1673-1957: Extensive sort/filter/list/load‑image edge‑case tests look consistent with implementationThe big block of new tests around
sort_images,list_images, andload_imagedoes a good job of pinning down edge behavior:
- Unknown sort option leaves the list as‑is.
- Files without extensions, empty directories, and directories passed to
load_imageare handled as expected.- Filters correctly no‑op when given empty patterns or size values and behave sensibly for malformed “between” ranges.
- Sorting with
Nonesize/created_atvalues matches the implementation (treated as 0 / sorted last respectively).- Combined filter tests (category + size) validate the interaction between filters and the category map.
Overall, these tests closely mirror the current logic and will be very helpful if you later tweak the filter/sort semantics. I don’t see any correctness issues here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src-tauri/src/lib.rs(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src-tauri/src/lib.rs (1)
src/types.ts (2)
CategoryAssignment(43-46)ImagePath(5-9)
🔇 Additional comments (2)
src-tauri/src/lib.rs (2)
52-65: PathBuf-basedget_parent_directory+ UTF‑8 tests look solid; double‑check bare filename semanticsThe switch to
PathBufinget_parent_directoryand the updated doc/example plus new UTF‑8 coverage tests (test_get_parent_directory_invalid_utf8_path/valid_utf8_path) are all consistent and more idiomatic for a Tauri command.One thing to verify: in
test_get_parent_directory, the"file.txt"case currently assertsis_ok(), but with the current implementation you’re relying directly onPathBuf::parent(). On some platforms,Path::new("file.txt").parent()yieldsNone, which would cause this call to return"File has no parent directory". Please confirm that the expectation for a bare filename is intentionallyOk(e.g., you conceptually treat it as being in the current directory) and, if so, consider normalizing that case in the function or adjusting the test to match the actualPathBuf::parentbehavior.Also applies to: 765-787, 1960-1984
1557-1671: Good coverage for.hitoconfig load/save, including filename overrides and invalid JSONThe new
load_hito_config/save_hito_configtests exercise all the important branches:
- No config file present returns an empty
image_categoriesvec.- Default
.hito.jsonvs custom filenames.- Round‑trip save then deserialize.
- Invalid JSON returning an error that contains
"Failed to parse".This matches the implementation’s behavior and should make future refactors of the config path logic or serialization relatively safe. No issues from my side here.
|
@coderabbitai pause |
✅ Actions performedReviews paused. |
- Fix doctests by marking Tauri commands as no_run or removing executable examples - Rename lib from tauri_app_lib to hito_lib in Cargo.toml and all references - Improve codecov workflow with Cargo cache and better doctest error handling
The --doc flag for cargo llvm-cov requires nightly toolchain. Update the command to use cargo +nightly llvm-cov --doc.
1 participant
Add 21 additional tests to improve coverage from 17 to 38 tests:
Coverage results:
Remaining uncovered code is primarily Tauri integration functions requiring AppHandle, which is appropriate to test via integration tests.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.