Skip to content

Add beginning of complex serialization / deserialization tests (#6834) #7336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add beginning of complex serialization / deserialization tests (#6834) #7336

wants to merge 3 commits into from

Conversation

Wcubed
Copy link
Contributor

@Wcubed Wcubed commented Jan 22, 2023

Objective

Lay down the basis for catching serialization / deserialization bugs, such as those listed in #6834.

Solution

Added a backend-agnostic roundtrip test, along with implementations for ron, postcard, bincode and messagepack.

@Wcubed Wcubed mentioned this pull request Jan 22, 2023
Open
@alice-i-cecile alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change C-Testing A change that impacts how we test Bevy or how users test their apps A-Scenes Serialized ECS data stored on the disk labels Jan 23, 2023
james7132
james7132 reviewed Jan 23, 2023
View reviewed changes
tests/scenes/components.rs Outdated
.write_to_world(&mut output_app.world, &mut entity_map)
.unwrap_or_else(|error| panic!("Could not add deserialized scene to world: {error}"));

// TODO (Wybe 2023-01-22): Ideally we'd check whether the input and output world are exactly equal. But the world does not implement Eq.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// TODO (Wybe 2023-01-22): Ideally we'd check whether the input and output world are exactly equal. But the world does not implement Eq.
// TODO: Ideally we'd check whether the input and output world are exactly equal. But the world does not implement Eq.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, I have a thing that auto-inserts the name and date, should probably disable that for open source projects 😄

tests/scenes/components.rs Outdated
.register_type::<Option<Entity>>()
.register_type::<VectorComponent>()
.register_type::<TupleComponent>()
// TODO (Wcubed 2023-01-22): Without the `Vec` registrations, the serialization
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// TODO (Wcubed 2023-01-22): Without the `Vec` registrations, the serialization
// TODO: Without the `Vec` registrations, the serialization

tests/scenes/components.rs Outdated
// Either both should fail, or both should work.
.register_type::<Vec<u32>>()
.register_type::<Vec<String>>()
// TODO (Wcubed 2023-01-22): Without these tuple registrations, the serialization
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// TODO (Wcubed 2023-01-22): Without these tuple registrations, the serialization
// TODO: Without these tuple registrations, the serialization

@Wcubed Wcubed force-pushed the test-scene-de-serialization branch from 0883a36 to 528eff1 Compare January 26, 2023 20:01
@Wcubed
Copy link
Contributor Author

Wcubed commented Jan 26, 2023

I created a separate crate for adding integration tests, so we can separate out the dependencies for the integration tests with those for the examples.

I was not sure whether to put the tests all in the src directory, or all in the tests directory. If we put them in the src directory, we could re-use components we create for tests in different tests. Or have I missed something and is it possible to include one test module in another?

@Wcubed Wcubed force-pushed the test-scene-de-serialization branch from 5341fcb to c64a8c2 Compare March 12, 2023 14:39
@Wcubed
Copy link
Contributor Author

Wcubed commented Mar 12, 2023

Rebased upon current master.

cBournhonesque
cBournhonesque reviewed Mar 25, 2024
View reviewed changes
integration_tests/src/scenes/components.rs
let scene = DynamicScene::from_world(&input_app.world, type_registry);
let serialized_again = serialize(scene, type_registry);

assert_eq!(serialized, serialized_again);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe you can also check the size of entity_map after the call to write_to_world?

@BenjaminBrienen BenjaminBrienen added D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jan 5, 2025
@BenjaminBrienen
Copy link
Contributor

@Wcubed are you still interested in this? If so, can you fix the merge conflicts?

@Wcubed
Copy link
Contributor Author

Wcubed commented Jan 6, 2025

Certainly, I will have a go at it this week

@Wcubed Wcubed force-pushed the test-scene-de-serialization branch from c64a8c2 to e4b39ba Compare January 6, 2025 19:31
@Wcubed
Copy link
Contributor Author

Wcubed commented Jan 6, 2025

Updated to 0.16
Currently the serialization integration tests fail due to bevy_utils::Instant not being registered.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 6, 2025

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

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

@james7132 james7132 james7132 left review comments

@cBournhonesque cBournhonesque cBournhonesque left review comments

Assignees
No one assigned
Labels
A-Scenes Serialized ECS data stored on the disk C-Code-Quality A section of code that is hard to understand or change C-Testing A change that impacts how we test Bevy or how users test their apps D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Wcubed @BenjaminBrienen @james7132 @cBournhonesque @alice-i-cecile