Skip to content

🧹 Refactor tests to remove unsafe unwrap() in uf2.rs - #323

Open
undivisible wants to merge 1 commit into
masterfrom
fix-uf2-unwrap-1027258785939123781
Open

🧹 Refactor tests to remove unsafe unwrap() in uf2.rs#323
undivisible wants to merge 1 commit into
masterfrom
fix-uf2-unwrap-1027258785939123781

Conversation

@undivisible

@undivisible undivisible commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

🎯 What: Removed unsafe .unwrap() usages when reading u32 values from byte slices in the test assertions of in-cli/src/native_emit/uf2.rs.
💡 Why: To improve code health and maintainability by introducing a robust error handling approach with a read_u32 helper function that safely handles the conversion.
Verification: Verified by checking formatting with cargo fmt, running lints with cargo clippy, running local module tests with cargo test native_emit::uf2, and running the full test suite with cargo test in the in-cli crate. All steps completed successfully with zero failures.
Result: The tests no longer panic with unwrap() on slice length mismatches and safely propagate string errors.

Note: Automated code review hallucinated that returning Result<(), String> from the tests when using ? on encode_uf2(...)? would fail compilation. However, encode_uf2 itself already returns Result<Vec<u8>, String>, matching the test return types seamlessly. Local compilation and test execution confirm this change is perfectly valid and completely safe.


PR created automatically by Jules for task 1027258785939123781 started by @undivisible


Note

Low Risk
Test-only changes in uf2.rs; production UF2 encoding behavior is untouched.

Overview
Refactors UF2 encoder unit tests in uf2.rs to avoid panicking on slice conversions and encoding failures.

A test-only read_u32 helper replaces repeated try_into().unwrap() + from_le_bytes when asserting header fields. encodes_single_block_with_family and splits_large_payload now return Result<(), String> and use ? on encode_uf2 and read_u32, so assertion/setup errors surface as test failures instead of panics. rejects_empty is unchanged and still uses expect_err.

Reviewed by Cursor Bugbot for commit 1d16d94. Configure here.

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@cursor

cursor Bot commented Sep 10, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_9b6137d2-eb92-4556-abb5-ac24af2ca0ad)

@mergify

mergify Bot commented Sep 10, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant