Skip to content

[tests] Add regression tests for Fuchsia packet types with large arrays (#2445)#3153

Open
unseen2004 wants to merge 2 commits intogoogle:mainfrom
unseen2004:fix-2445-regression-tests
Open

[tests] Add regression tests for Fuchsia packet types with large arrays (#2445)#3153
unseen2004 wants to merge 2 commits intogoogle:mainfrom
unseen2004:fix-2445-regression-tests

Conversation

@unseen2004
Copy link
Copy Markdown

This PR adds regression tests to verify that zerocopy correctly handles repr(C) structs containing large computed array fields (e.g., [u8; 2000]), as seen in Fuchsia's FxLogPacket and similar datagram types.

Specifically, it ensures that these types correctly derive and implement:
- `KnownLayout`
- `Immutable`
- `IntoBytes`

These tests confirm that zerocopy's const-generic implementation for arrays correctly scales to large sizes without hitting compiler limits or logic errors in the derive macros.

Verification:

Tests were verified locally on the stable toolchain:

  • ./cargo.sh +stable test -p zerocopy-derive --test struct_known_layout
  • ./cargo.sh +stable test -p zerocopy-derive --test struct_no_cell
  • ./cargo.sh +stable test -p zerocopy-derive --test struct_to_bytes

The tests passed successfully and were confirmed to fail if the derives were manually removed from the test cases.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.85%. Comparing base (61b2407) to head (941b49e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3153   +/-   ##
=======================================
  Coverage   91.85%   91.85%           
=======================================
  Files          20       20           
  Lines        6067     6067           
=======================================
  Hits         5573     5573           
  Misses        494      494           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@joshlf joshlf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, welcome to zerocopy and thanks for adding these!

Just so I understand, is there a reason to think we need a regression test for this behavior in particular – ie, that we previously had a bug relating to this behavior, or it's a theoretical edge case that might be hit in the future or something, or were you just looking to find a particular issue to contribute to?

#[repr(C)]
struct FxLogPacket {
metadata: FxLogMetadata,
data: [u8; FX_LOG_MAX_DATAGRAM_LEN - FX_LOG_METADATA_SIZE],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make FX_LOG_METADATA_SIZE size_of::<FxLogMetadata>() instead of hard-coding its size?

#[repr(C)]
struct FxLogPacket {
metadata: FxLogMetadata,
data: [u8; FX_LOG_MAX_DATAGRAM_LEN - FX_LOG_METADATA_SIZE],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here re: size_of.

@unseen2004
Copy link
Copy Markdown
Author

Hey, welcome to zerocopy and thanks for adding these!

Just so I understand, is there a reason to think we need a regression test for this behavior in particular – ie, that we previously had a bug relating to this behavior, or it's a theoretical edge case that might be hit in the future or something, or were you just looking to find a particular issue to contribute to?

Hi! I was looking for a way to start contributing, and this issue caught my eye because of my interest in Fuchsia in the past. While I'm not aware of a specific prior bug, these regression tests ensure that the derive macros continue to handle these complex types correctly, providing a safety net for similar Fuchsia-style headers in the future.

I've also updated the tests to use imp::core::mem::size_of::() instead of the hard-coded 32 as requested!

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.

3 participants