Rename fw-update-interface-mocks to fw-update-interface-test-mocks#905
Rename fw-update-interface-mocks to fw-update-interface-test-mocks#905RobertZ2011 wants to merge 1 commit into
fw-update-interface-mocks to fw-update-interface-test-mocks#905Conversation
Cargo Vet Audit Passed
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a new firmware-update interface test-mocks crate and updates a CFU service test to import it, along with adjusting CI configuration to exclude the std-only mock crate from embedded-target checks. The intent matches the existing pattern used by other *-interface-test-mocks crates (e.g., Type-C). However, the rename is currently incomplete/inconsistent: the new crate’s Cargo.toml still declares the old package name, while downstream code and CI use the new name.
Changes:
- Add
fw-update-interface-test-mockscrate with a basicFwUpdatemock and unit tests. - Update CFU service basic tests to import the new mock crate.
- Update CI
STD_EXCLUDED_CRATESto exclude the renamed std-only test-mocks crate.
Step-by-step review guide
-
New
FwUpdatemock crate- Adds a
Mockimplementingfw_update_interface::basic::FwUpdatethat records calls and can inject a one-shot error viaset_next_error. - This is consistent with other test-mocks patterns in the repo (std-backed
VecDequecall recording).
- Adds a
-
Downstream usage in CFU service tests
- The CFU service test switches its import path to
fw_update_interface_test_mocks::basic::{..., ...}. - This requires the workspace dependency graph to be updated to reference the renamed package consistently (package name, workspace member path, and any dev-deps).
- The CFU service test switches its import path to
-
CI exclusion for embedded-target clippy/hack
- The workflow updates
STD_EXCLUDED_CRATESto excludefw-update-interface-test-mocks. - Because
--excludeis keyed on the Cargo package name, this only works once the crate’s[package].namematches that value.
- The workflow updates
Potential issues
| # | Severity | File | Description | Code |
|---|---|---|---|---|
| 1 | High | fw-update-interface-test-mocks/Cargo.toml:2 |
Package name is still fw-update-interface-mocks, but the directory/imports/CI use fw-update-interface-test-mocks / fw_update_interface_test_mocks, so the rename won’t actually take effect and references won’t line up. |
name = "fw-update-interface-mocks" |
| 2 | High | cfu-service/src/basic/test.rs:24 |
Import was updated to fw_update_interface_test_mocks, but the workspace/dev-dependency graph still refers to fw-update-interface-mocks (workspace root + cfu-service dev-deps), so the dependency rename is inconsistent. |
use fw_update_interface_test_mocks::basic::{FnCall as FwFnCall, Mock}; |
| 3 | Medium | .github/workflows/check.yml:36 |
--exclude uses Cargo package names; excluding fw-update-interface-test-mocks won’t work until the mock crate’s Cargo package name is updated to match, which may cause embedded-target hack/clippy runs to attempt building a std-only crate. |
STD_EXCLUDED_CRATES: "--exclude fw-update-interface-test-mocks ..." |
Reviewed changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| fw-update-interface-test-mocks/src/lib.rs | New crate root exporting the basic mock module. |
| fw-update-interface-test-mocks/src/basic.rs | Adds a FwUpdate mock implementation with call recording and tests. |
| fw-update-interface-test-mocks/Cargo.toml | Defines the new test-mocks crate (currently with an inconsistent package name). |
| cfu-service/src/basic/test.rs | Switches CFU tests to import the renamed firmware-update test mock crate. |
| .github/workflows/check.yml | Updates CI exclusion list for std-only crates on embedded targets. |
9e503e3 to
f5c978a
Compare
Rename to better match crate purpose and other test mock crates.
f5c978a to
a518905
Compare
fw-update-interface-mocks to fw-update-test-interface-mocksfw-update-interface-mocks to fw-update-interface-test-mocks
Rename to better match crate purpose and other test mock crates.