test(explorer): cover file icon resolution chain - #1076
Conversation
fileIconUrl and folderIconUrl resolve which icon a file or folder gets, but the resolution chain had no tests. Lock the algorithm (not the icon set): a known extension resolves to something other than the default, resolution is deterministic per extension, a compound extension (.test.tsx) resolves differently from its base (.tsx), a by-name match like Dockerfile is non-default, and an unknown extension falls back to the default. Assertions are relational so they survive icon-set changes.
📝 WalkthroughWalkthroughAdds Vitest coverage for file and folder icon URL generation, including known, compound, filename-based, unknown, and default resolution cases. ChangesIcon resolution tests
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/explorer/lib/iconResolver.test.ts`:
- Around line 38-42: Strengthen the folderIconUrl tests by asserting both
collapsed and expanded results for the mapped “src” folder, and compare them
against the corresponding collapsed and expanded results for a confirmed unknown
folder name. Keep the assertions specific enough to detect a broken “src”
mapping rather than only checking for an SVG data URL.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e1478e3a-851c-49ed-a6f3-92267a1a4bf4
📒 Files selected for processing (1)
src/modules/explorer/lib/iconResolver.test.ts
| describe("folderIconUrl", () => { | ||
| it("returns an inline svg data url", () => { | ||
| expect(folderIconUrl("src", false)).toContain("data:image/svg"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Strengthen the folder resolution assertions.
This only proves that the collapsed result is some SVG data URL. Both mapped and fallback paths produce data URLs, so the test can pass if the src mapping breaks, and it never exercises expanded=true. Add assertions for the expanded variant and compare against a confirmed unknown folder name.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/explorer/lib/iconResolver.test.ts` around lines 38 - 42,
Strengthen the folderIconUrl tests by asserting both collapsed and expanded
results for the mapped “src” folder, and compare them against the corresponding
collapsed and expanded results for a confirmed unknown folder name. Keep the
assertions specific enough to detect a broken “src” mapping rather than only
checking for an SVG data URL.
1 participant
What
Adds unit tests for the file/folder icon resolution chain in
explorer/lib/iconResolver.ts. Test-only.Why
fileIconUrlresolves which icon a file gets through a by-name -> extension ->compound-extension -> default fallback chain, and that resolution logic was
untested.
How
The assertions lock the resolution algorithm, not the icon SVGs: they compare
outputs relationally (known vs default, base vs compound extension, by-name vs
default), so they stay valid when the underlying icon set changes.
Locked behavior:
Testing
Ran the full suite plus type-check and lint locally on Windows.
pnpm lintclean (unchanged: 103 pre-existing warnings, none introduced)pnpm check-typescleanpnpm testclean (the new suite passes; see reviewer note on onepre-existing suite)
src-tauri/) cargo clippy - N/A, no Rust changessrc-tauri/) cargo nextest - N/A, no Rust changes#[tauri::command]signature) - N/Apnpm tauri dev- N/A, no UI changeScreenshots / GIFs
N/A - no UI change.
Notes for reviewer
mainon purpose (my token lacksthe
workflowscope). Only adds a new file; should merge cleanly.src/app/eager-budget.test.tsfails locally under Windows with
git core.autocrlf=true; the committed blob isLF and it passes on CI.
Summary by CodeRabbit