Deduplicate AMS2 test helpers#537
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Droid finished @EffortlessSteven's task —— View job Refactor looks straightforward and keeps behavior equivalent while reducing duplication. The main concern is the test helper’s unsafe struct-to-bytes conversion potentially reading uninitialized padding; tightening the helper to guarantee byte-initialization would make the tests more robust. |
Compatibility Layer Usage ReportCurrent usage count: 193 📈 Usage Trend (Last 30 Days)
Usage Details
... and 183 more occurrences Migration GuideTo migrate these usages, replace:
See Migration Patterns for detailed examples. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23368b470e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| //! AMS2 uses Windows shared memory, so most behavioral tests are adapter-level | ||
| //! (game_id, update_rate, etc.). Shared memory tests require a running AMS2 instance. | ||
|
|
||
| mod common; |
There was a problem hiding this comment.
When this integration test declares mod common, Cargo compiles every item in tests/common/mod.rs into the adapter_tests test crate, but this file only uses default_shared_memory and shared_memory_to_bytes; adapter() and write_fixed_str() are therefore dead code in this crate. The repository’s required cargo clippy --all-targets --all-features -- -D warnings promotes those dead_code warnings to errors, so the all-targets lint job fails before tests run. Consider splitting the helpers by use site or allowing dead code on the shared test-helper module.
Useful? React with 👍 / 👎.
![factory-droid[bot]](https://avatars.githubusercontent.com/in/358017?s=60&v=4){kind=small}
| /// Serialize an `AMS2SharedMemory` to its raw byte representation for the normalize API. | ||
| pub fn shared_memory_to_bytes(data: &AMS2SharedMemory) -> Vec<u8> { | ||
| let size = std::mem::size_of::<AMS2SharedMemory>(); | ||
| let ptr = data as *const AMS2SharedMemory as *const u8; | ||
| // SAFETY: AMS2SharedMemory is repr(C) and fully initialized via Default in tests. | ||
| unsafe { std::slice::from_raw_parts(ptr, size) }.to_vec() | ||
| } | ||
|
|
||
| /// Create a default `AMS2SharedMemory` without repeating private-field workarounds. | ||
| pub fn default_shared_memory() -> AMS2SharedMemory { | ||
| AMS2SharedMemory::default() | ||
| } |
There was a problem hiding this comment.
[P1] Avoid UB when converting AMS2SharedMemory to bytes
shared_memory_to_bytes reinterprets the entire #[repr(C)] struct as &[u8] and copies it; this can read uninitialized padding bytes even when the fields were set via Default/struct update syntax, which is UB and can cause flakiness under Miri/sanitizers. In tests, prefer constructing shared-memory instances from an all-zero base (so padding is initialized) and update the safety comment to reflect that requirement.
| /// Serialize an `AMS2SharedMemory` to its raw byte representation for the normalize API. | |
| pub fn shared_memory_to_bytes(data: &AMS2SharedMemory) -> Vec<u8> { | |
| let size = std::mem::size_of::<AMS2SharedMemory>(); | |
| let ptr = data as *const AMS2SharedMemory as *const u8; | |
| // SAFETY: AMS2SharedMemory is repr(C) and fully initialized via Default in tests. | |
| unsafe { std::slice::from_raw_parts(ptr, size) }.to_vec() | |
| } | |
| /// Create a default `AMS2SharedMemory` without repeating private-field workarounds. | |
| pub fn default_shared_memory() -> AMS2SharedMemory { | |
| AMS2SharedMemory::default() | |
| } | |
| /// Serialize an `AMS2SharedMemory` to its raw byte representation for the normalize API. | |
| pub fn shared_memory_to_bytes(data: &AMS2SharedMemory) -> Vec<u8> { | |
| let size = std::mem::size_of::<AMS2SharedMemory>(); | |
| let ptr = data as *const AMS2SharedMemory as *const u8; | |
| // SAFETY: `data` must be fully byte-initialized (including any internal padding). | |
| unsafe { std::slice::from_raw_parts(ptr, size) }.to_vec() | |
| } | |
| /// Create a default `AMS2SharedMemory` without repeating private-field workarounds. | |
| pub fn default_shared_memory() -> AMS2SharedMemory { | |
| // SAFETY: AMS2SharedMemory is a plain-old-data struct (numeric scalars + fixed-size arrays), | |
| // so the all-zero bit pattern is valid and also ensures any internal padding bytes are initialized. | |
| unsafe { std::mem::zeroed() } | |
| } |
1 participant
Motivation
Description
crates/telemetry-ams2/tests/common/mod.rsprovidingadapter(),shared_memory_to_bytes(),default_shared_memory(), andwrite_fixed_str()helper functions.crates/telemetry-ams2/tests/adapter_tests.rs,deep_tests.rs,ams2_deep_tests.rs, andsnapshot_tests.rstomod common;and import the shared helpers instead of defining local copies.write_fixed_str()andshared_memory_to_bytes()aliases where appropriate.python scripts/cargo_fmt_workspace.py.Testing
python scripts/cargo_fmt_workspace.pywhich completed successfully across the workspace.git diff --checkwhich reported no spacing/whitespace errors.cargo test -p racing-wheel-telemetry-ams2 --tests, which reached compilation but failed during a native dependency build because the environment is missing the systemlibudevrequired byhidapi(build blocked by system dependency), so test execution could not fully complete.Codex Task