Skip to content
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.

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

New Serializables (v3) #285

Merged
merged 5 commits into from
Jun 7, 2024
Merged

Conversation

LiteApplication
Copy link
Contributor

@LiteApplication LiteApplication commented May 1, 2024

This uses the Serializable (#273) methods to validate and use the data in the NBTags (#257) subclasses.
The tests also make use of the new test generator introduced in #273.

EDIT : Let's merge this with 273 !

Original PR description :

Changed the way Serializable classes are handled:

Here is how a basic Serializable class looks like:

@final
@dataclass
class ToyClass(Serializable):
    """Toy class for testing demonstrating the use of gen_serializable_test on `Serializable`."""


    # Attributes can be of any type
    a: int
    b: str

    # dataclasses.field() can be used to specify additional metadata

    def serialize_to(self, buf: Buffer):
        """Write the object to a buffer."""
        buf.write_varint(self.a)
        buf.write_utf(self.b)

    @classmethod
    def deserialize(cls, buf: Buffer) -> ToyClass:
        """Deserialize the object from a buffer."""
        a = buf.read_varint()
        if a == 0:
            raise ZeroDivisionError("a must be non-zero")
        b = buf.read_utf()
        return cls(a, b)

    def validate(self) -> None:
        """Validate the object's attributes."""
        if self.a == 0:
            raise ZeroDivisionError("a must be non-zero")
        if len(self.b) > 10:
            raise ValueError("b must be less than 10 characters")

The Serializable class must implement the following methods:

  • serialize_to(buf: Buffer) -> None: Serializes the object to a buffer.
  • deserialize(buf: Buffer) -> Serializable: Deserializes the object from a buffer.
  • validate() -> None: Validates the object's attributes, raising an exception if they are invalid.

Added a test generator for Serializable classes:

The gen_serializable_test function generates tests for Serializable classes. It takes the following arguments:

  • context: The dictionary containing the context in which the generated test class will be placed (e.g. globals()).

    Dictionary updates must reflect in the context. This is the case for globals() but implementation-specific for locals().

  • cls: The Serializable class to generate tests for.

  • fields: A list of fields where the test values will be placed.

    In the example above, the ToyClass class has two fields: a and b.

  • test_data: A list of tuples containing either:

    • ((field1_value, field2_value, ...), expected_bytes): The values of the fields and the expected serialized bytes. This needs to work both ways, i.e. cls(field1_value, field2_value, ...) == cls.deserialize(expected_bytes).
    • ((field1_value, field2_value, ...), exception): The values of the fields and the expected exception when validating the object.
    • (exception, bytes): The expected exception when deserializing the bytes and the bytes to deserialize.

The gen_serializable_test function generates a test class with the following tests:

gen_serializable_test(
    context=globals(),
    cls=ToyClass,
    fields=[("a", int), ("b", str)],
    test_data=[
        ((1, "hello"), b"\x01\x05hello"),
        ((2, "world"), b"\x02\x05world"),
        ((0, "hello"), ZeroDivisionError),
        ((1, "hello world"), ValueError),
        (ZeroDivisionError, b"\x00"),
        (IOError, b"\x01"),
    ],
)

The generated test class will have the following tests:

class TestGenToyClass:
    def test_serialization(self):
        # 2 subtests for the cases 1 and 2
    
    def test_deserialization(self):
        # 2 subtests for the cases 1 and 2

    def test_validation(self):
        # 2 subtests for the cases 3 and 4

    def test_exceptions(self):
        # 2 subtests for the cases 5 and 6

Also this has 100% test coverage for all data types and packets.

@ItsDrike
Copy link
Member

Now that NBTag support was merged, this can become a part of #273, or alternatively, this can replace #273.

@ItsDrike ItsDrike added p: 2 - normal Normal priority a: API Related to exposed core API of the project t: optimization Optimizations to the code (performance improvements, code cleanup, or other general optimizations) labels May 14, 2024
@LiteApplication
Copy link
Contributor Author

I'm currently merging the changes added right after #257. I'm OK with replacing #273 with this PR

@LiteApplication LiteApplication changed the title Use Serializable for NBTags New Serializables (v3) May 15, 2024
@LiteApplication LiteApplication marked this pull request as ready for review May 15, 2024 20:35
@LiteApplication LiteApplication force-pushed the nbt-serializable branch 2 times, most recently from fabe6b3 to be65798 Compare May 16, 2024 15:32
@LiteApplication LiteApplication mentioned this pull request May 17, 2024
mcproto/utils/abc.py Outdated Show resolved Hide resolved
mcproto/utils/abc.py Outdated Show resolved Hide resolved
tests/helpers.py Outdated Show resolved Hide resolved
mcproto/utils/abc.py Show resolved Hide resolved
mcproto/types/nbt.py Outdated Show resolved Hide resolved
changes/273.internal.md Outdated Show resolved Hide resolved
changes/273.internal.md Outdated Show resolved Hide resolved
changes/273.internal.md Outdated Show resolved Hide resolved
tests/mcproto/types/test_nbt.py Show resolved Hide resolved
tests/mcproto/types/test_nbt.py Outdated Show resolved Hide resolved
@ItsDrike ItsDrike linked an issue May 18, 2024 that may be closed by this pull request
@LiteApplication LiteApplication force-pushed the nbt-serializable branch 4 times, most recently from 53d137f to bbab587 Compare May 21, 2024 19:20
@LiteApplication LiteApplication requested a review from ItsDrike May 22, 2024 06:22
changes/285.internal.md Outdated Show resolved Hide resolved
mcproto/types/nbt.py Outdated Show resolved Hide resolved
mcproto/types/nbt.py Show resolved Hide resolved
mcproto/utils/abc.py Outdated Show resolved Hide resolved
mcproto/utils/abc.py Outdated Show resolved Hide resolved
mcproto/utils/abc.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
tests/helpers.py Outdated Show resolved Hide resolved
tests/helpers.py Outdated Show resolved Hide resolved
tests/mcproto/types/test_nbt.py Outdated Show resolved Hide resolved
@LiteApplication LiteApplication force-pushed the nbt-serializable branch 4 times, most recently from fc59b46 to d6f2921 Compare May 25, 2024 16:48
@LiteApplication LiteApplication requested a review from ItsDrike May 25, 2024 19:43
@LiteApplication LiteApplication force-pushed the nbt-serializable branch 2 times, most recently from ffe24c8 to deacc62 Compare June 6, 2024 07:08
Copy link
Member

@ItsDrike ItsDrike left a comment

Choose a reason for hiding this comment

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

Just one small nitpick and this should be good to go

mcproto/utils/abc.py Outdated Show resolved Hide resolved
Provide a way to test serialization, deserialization, validation and deserialization errors easily.
This fixes Avoid repetition in tests for serialize+deserialize tests py-mine#64 and makes it easier to add new data types
 - Added a `transform` method for type conversions and more.
 - Include NBT and Serializable into the Sphinx documentation.
 - Added more explicit error messages to serializable tests.
 - Added the possibility to specify the error message/arguments in the tests.
 - Added pytest to the docs requirements to list the test function in the docs.
- Split the changelog
- Use TypeGuard correctly
- Remove `transform` in favor of `__attrs_post_init__` to allow for a more personalized use of `validate`
- Remove `define` from the docs, change format in changelog
@ItsDrike ItsDrike merged commit 6e50d94 into py-mine:main Jun 7, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: API Related to exposed core API of the project p: 2 - normal Normal priority t: optimization Optimizations to the code (performance improvements, code cleanup, or other general optimizations)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid repetition in tests for serialize+deserialize tests
2 participants