Skip to content

Add roundtrip test for Foo and Bar structs - #87

Draft
google-labs-jules[bot] wants to merge 6 commits into
mainfrom
test/add-foo-bar-roundtrip-test-6948808545237087535
Draft

Add roundtrip test for Foo and Bar structs#87
google-labs-jules[bot] wants to merge 6 commits into
mainfrom
test/add-foo-bar-roundtrip-test-6948808545237087535

Conversation

@google-labs-jules

Copy link
Copy Markdown
Contributor

Adds a roundtrip unit test for Foo and Bar structs decorated with [StructLayout(LayoutKind.Sequential, Pack = 1)] and [ZeroSerializer], verifying hardcoded byte lengths (3 and 30) and roundtrip serialization correctness.


PR created automatically by Jules for task 6948808545237087535 started by @sator-imaging

- Define Foo and Bar structs with [StructLayout(LayoutKind.Sequential, Pack = 1)] and [ZeroSerializer] attributes in SerializationModels.cs.
- Add SequentialPackOneFooBarRoundTrip test in SerializationTests.cs verifying exact hardcoded byte lengths (3 for BarView and 30 for FooView) and verifying roundtrip data integrity.
@google-labs-jules

Copy link
Copy Markdown
Contributor Author

👋 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.

Comment thread tests/SerializationModels.cs Outdated
Comment on lines +315 to +316
public Bar G { get; set; }
public Bar H { get; set; }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
public Bar G { get; set; }
public Bar H { get; set; }
public byte G { get; set; }
public Bar H { get; set; }
public Bar I { get; set; }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Applied the suggested shape update to Foo in SerializationModels.cs and updated SequentialPackOneFooBarRoundTrip in SerializationTests.cs accordingly.

- Updated Foo struct in SerializationModels.cs to include byte property G and Bar properties H and I as suggested in PR review.
- Updated SequentialPackOneFooBarRoundTrip in SerializationTests.cs to check expected byte lengths (3 for BarView and 31 for FooView) and full roundtrip serialization.
Comment thread tests/SerializationModels.cs Outdated

[StructLayout(LayoutKind.Sequential, Pack = 1)]
[ZeroSerializer]
public struct Foo

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
public struct Foo
public struct BadAlignedStructWithPackOne

Rename Bar and test method name also.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Renamed Bar to BadAlignedStructWithPackOne and updated test method name to BadAlignedStructWithPackOneRoundTrip as requested.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

rename both Foo and Bar.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Renamed Foo to BadAlignedContainerStructWithPackOne and Bar to BadAlignedStructWithPackOne as requested.

- Renamed Bar struct to BadAlignedStructWithPackOne in SerializationModels.cs and updated Foo struct properties H and I accordingly.
- Renamed test method to BadAlignedStructWithPackOneRoundTrip in SerializationTests.cs.
- Renamed Foo to BadAlignedContainerStructWithPackOne and Bar to BadAlignedStructWithPackOne in SerializationModels.cs and updated SerializationTests.cs accordingly.
Comment thread tests/SerializationModels.cs Outdated

[StructLayout(LayoutKind.Sequential, Pack = 1)]
[ZeroSerializer]
public struct BadAlignedContainerStructWithPackOne

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
public struct BadAlignedContainerStructWithPackOne
public struct BadlyAlignedContainerStructWithPackOne

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Renamed to BadlyAlignedContainerStructWithPackOne.

Comment thread tests/SerializationModels.cs Outdated

[StructLayout(LayoutKind.Sequential, Pack = 1)]
[ZeroSerializer]
public struct BadAlignedStructWithPackOne

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
public struct BadAlignedStructWithPackOne
public struct BadlyAlignedStructWithPackOne

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Renamed to BadlyAlignedStructWithPackOne.

Comment thread tests/SerializationTests.cs
- Renamed BadAlignedStructWithPackOne to BadlyAlignedStructWithPackOne and BadAlignedContainerStructWithPackOne to BadlyAlignedContainerStructWithPackOne as record structs.
- Added Materialize equality assertion (foo == view.Materialize()) to BadlyAlignedStructWithPackOneRoundTrip in SerializationTests.cs.
Comment thread tests/SerializationTests.cs Outdated
I = new BadlyAlignedStructWithPackOne { A = 0xCD, B = -4321 }
};

var buffer = new byte[BadlyAlignedContainerStructWithPackOneView.RequiredByteLength];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
var buffer = new byte[BadlyAlignedContainerStructWithPackOneView.RequiredByteLength];
var array = new BadlyAlignedContainerArrayStructWithPackOne();
array.Values = new[] { default, foo, default };
var buffer = new byte[BadlyAlignedContainerStructWithPackOneView.RequiredByteLength * 3];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added BadlyAlignedContainerArrayStructWithPackOne model and updated array buffer setup.

Comment thread tests/SerializationTests.cs Outdated

TestAssert.Equal(31, writtenBytes, nameof(writtenBytes));

var view = new BadlyAlignedContainerStructWithPackOneView(buffer);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
var view = new BadlyAlignedContainerStructWithPackOneView(buffer);
var second = buffer.AsMemory().Slice(BadlyAlignedContainerStructWithPackOneView.RequiredByteLength, BadlyAlignedContainerStructWithPackOneView.RequiredByteLength)
var view = new BadlyAlignedContainerStructWithPackOneView(second);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated view initialization to slice directly from memory offset.

var buffer = new byte[BadlyAlignedContainerStructWithPackOneView.RequiredByteLength];
int writtenBytes = foo.Serialize(buffer);

TestAssert.Equal(31, writtenBytes, nameof(writtenBytes));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
TestAssert.Equal(31, writtenBytes, nameof(writtenBytes));
TestAssert.Equal(93, writtenBytes, nameof(writtenBytes));
var arrayView = new BadlyAlignedContainerArrayStructWithPackOne(buffer);
TestAssert.True(arrayView.Values[0] == default, "1st item");
TestAssert.True(arrayView.Values[2] == default, "3rd item");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated array assertions in BadlyAlignedStructWithPackOneRoundTrip.

…test

- Added BadlyAlignedContainerArrayStructWithPackOne model in SerializationModels.cs.
- Updated BadlyAlignedStructWithPackOneRoundTrip in SerializationTests.cs to test array serialization and memory slicing as requested in PR review.
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