Repository navigation
test: cover platform-specific branches and Windows package managers - #519
shenxianpeng wants to merge 2 commits into
Conversation
- Move the wheel architecture check into PlatformTag::is_arch_compatible() and the static binary asset name into StaticDistDownloader::asset_name(), both taking the OS and architecture as arguments. Tests now check every supported OS and architecture, not only the one CI runs on. - Windows: move the parsing of `choco list` / `winget list` output into is_listed() and the install result into check_install(), with unit tests using sample output. winget is not on the CI runners, so its parsing was never tested. - clang-tidy: test a fix whose diagnostic has no matching note.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 19 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly describes the main changes: added coverage for platform-specific branches and Windows package-manager behavior. It is concise and related to the pull request. |
| Docstring Coverage | |
Docstring coverage is 82.61% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 4 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
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 help to get the list of available commands.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #519 +/- ##
==========================================
+ Coverage 99.43% 99.88% +0.44%
==========================================
Files 28 28
Lines 6774 6860 +86
==========================================
+ Hits 6736 6852 +116
+ Misses 38 8 -30 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch has not been deployed
2 participants
Summary
Raises coverage of the lines Codecov reported as missed on
main(99.43%, 38 lines). No behavior change.Platform branches (
pypi.rs,static_dist.rs): these usedstd::env::consts/cfg!()inline, so CI (Linux and Windows on x86_64) could never run the macOS and arm64 arms. They are now small functions that take the OS and architecture as arguments, and a table test checks every combination:PlatformTag::is_arch_compatible(sys_os, sys_arch)StaticDistDownloader::asset_name(tool, major, os, arch)(the binary name now usesstd::env::consts::EXE_SUFFIX)Windows package managers (
native_packages/windows.rs, 20 missed lines): winget is not on the CI runners, so its output parsing never ran, and installs never succeed there. The Chocolatey and winget parsing loops were duplicates, so they are now oneis_listed()(header lines and name column depend on the manager), and the install result ischeck_install(). Unit tests use samplechoco list/winget listoutput and fakeOutputs. These tests only build on Windows; I checked them withcargo clippy --tests --target x86_64-pc-windows-gnu -- -D warnings, and the Windows CI job runs them.clang-tidy: new test
apply_fixes_without_matching_note: a fix whose diagnostic has no matching note is still applied, and the fix for another file is returned without a note.Lines still not covered (all unreachable)
rest_client.rs?onmake_comment()/SarifLog::new(), which only fail on a poisoned mutex.post_feedback()already returned that error fromtally_tidy_advice()at the start.common_fs.rs?on writing a patch into aString(fmt::WriteforStringnever fails).progress_bar.rs?on awrite!into the buffered stdout lock; it only buffers, and the followingflush()reports errors.pypi.rs}ofif let Some(parent) = extracted_bin.parent()utils.rsComponent::Prefix(..) => unreachable!()Stable Rust coverage has no way to exclude single lines, and rewriting these just for the number seemed wrong. Happy to change any of them if you prefer.
Related Issues
Follows #486 and #491.
Summary by CodeRabbit