From 0f893724cabe513a5a9f9c8428dbd31f1b4f1d52 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Tue, 4 Jun 2024 07:01:38 -0400 Subject: [PATCH] fix: Add compatibility with protobuf==5.x (#433) * ci: add support for pre-release sessions * add compatibility with protobuf 5.x * fix RecursionError: maximum recursion depth exceeded while calling a Python object in tests * update required checks * update comments * update required checks * See https://github.com/protocolbuffers/protobuf/issues/16596;avoid breaking change * lint * Address review feedback * lint * add tests * Update tests/test_fields_mitigate_collision.py * add deprecation warning * Cater for protobuf 5.x+ * style * Filter deprecation warning * remove redundant code * revert * add comment * add comment * refactor code * style * update warning filter * address review comments * map_composite_types_str->map_composite_type_names * update comment * lint * add more test cases in test_json_default_values * add more test cases to tests/test_message.py * address review feedback * add comment * formatting * formatting * typo * Address review feedback * Update proto/marshal/marshal.py Co-authored-by: Victor Chudnovsky * typo * address review feedback * address review feedback * fix test case * stye * add more test cases * add test case * Update tests/test_message.py Co-authored-by: Victor Chudnovsky --------- Co-authored-by: Victor Chudnovsky --- .github/sync-repo-settings.yaml | 17 +-- .github/workflows/tests.yml | 40 ++++++- noxfile.py | 80 +++++++++++--- proto/marshal/compat.py | 19 +++- proto/marshal/marshal.py | 9 +- proto/message.py | 188 ++++++++++++++++++++++++++++---- pytest.ini | 2 + setup.py | 2 +- testing/constraints-3.13.txt | 0 tests/conftest.py | 16 ++- tests/test_json.py | 73 ++++++++++++- tests/test_message.py | 59 +++++++++- 12 files changed, 447 insertions(+), 58 deletions(-) create mode 100644 testing/constraints-3.13.txt diff --git a/.github/sync-repo-settings.yaml b/.github/sync-repo-settings.yaml index f1068589..1bb27933 100644 --- a/.github/sync-repo-settings.yaml +++ b/.github/sync-repo-settings.yaml @@ -7,24 +7,27 @@ branchProtectionRules: requiredStatusCheckContexts: - 'style-check' - 'docs' - - 'unit (3.6)' - 'unit (3.6, cpp)' - - 'unit (3.7)' + - 'unit (3.6, python)' + - 'unit (3.6, upb)' - 'unit (3.7, cpp)' + - 'unit (3.7, python)' - 'unit (3.7, upb)' - - 'unit (3.8)' - 'unit (3.8, cpp)' + - 'unit (3.8, python)' - 'unit (3.8, upb)' - - 'unit (3.9)' - 'unit (3.9, cpp)' + - 'unit (3.9, python)' - 'unit (3.9, upb)' - - 'unit (3.10)' - 'unit (3.10, cpp)' + - 'unit (3.10, python)' - 'unit (3.10, upb)' - - 'unit (3.11)' + - 'unit (3.11, python)' - 'unit (3.11, upb)' - - 'unit (3.12)' + - 'unit (3.12, python)' - 'unit (3.12, upb)' + - 'prerelease (3.12, python)' + - 'prerelease (3.12, upb)' - cover - OwlBot Post Processor - 'cla/google' diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 4fad78dc..03f19896 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -42,19 +42,27 @@ jobs: runs-on: ubuntu-20.04 strategy: matrix: - python: ['3.6', '3.7', '3.8', '3.9', '3.10', '3.11', '3.12'] - variant: ['', 'cpp', 'upb'] + python: ['3.6', '3.7', '3.8', '3.9', '3.10', '3.11', '3.12', '3.13'] + variant: ['cpp', 'python', 'upb'] + # TODO(https://github.com/googleapis/proto-plus-python/issues/389): + # Remove the 'cpp' implementation once support for Protobuf 3.x is dropped. + # The 'cpp' implementation requires Protobuf == 3.x however version 3.x + # does not support Python 3.11 and newer. The 'cpp' implementation + # must be excluded from the test matrix for these runtimes. exclude: - variant: "cpp" python: 3.11 - variant: "cpp" python: 3.12 + - variant: "cpp" + python: 3.13 steps: - uses: actions/checkout@v4 - name: Set up Python ${{ matrix.python }} uses: actions/setup-python@v5 with: python-version: ${{ matrix.python }} + allow-prereleases: true - name: Install nox run: | pip install nox @@ -68,12 +76,38 @@ jobs: env: COVERAGE_FILE: .coverage-${{ matrix.variant }}-${{ env.PYTHON_VERSION_TRIMMED }} run: | - nox -s unit${{ matrix.variant }}-${{ env.PYTHON_VERSION_TRIMMED }} + nox -s "unit-${{ env.PYTHON_VERSION_TRIMMED }}(implementation='${{ matrix.variant }}')" - name: Upload coverage results uses: actions/upload-artifact@v4 with: name: coverage-artifact-${{ matrix.variant }}-${{ env.PYTHON_VERSION_TRIMMED }} path: .coverage-${{ matrix.variant }}-${{ env.PYTHON_VERSION_TRIMMED }} + prerelease: + runs-on: ubuntu-20.04 + strategy: + matrix: + python: ['3.12'] + variant: ['python', 'upb'] + steps: + - uses: actions/checkout@v4 + - name: Set up Python ${{ matrix.python }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + allow-prereleases: true + - name: Install nox + run: | + pip install nox + - name: Run unit tests + env: + COVERAGE_FILE: .coverage-prerelease-${{ matrix.variant }} + run: | + nox -s "prerelease_deps(implementation='${{ matrix.variant }}')" + - name: Upload coverage results + uses: actions/upload-artifact@v4 + with: + name: coverage-artifact-prerelease-${{ matrix.variant }} + path: .coverage-prerelease-${{ matrix.variant }} cover: runs-on: ubuntu-latest needs: diff --git a/noxfile.py b/noxfile.py index d4da4b9b..4ba0161a 100644 --- a/noxfile.py +++ b/noxfile.py @@ -13,10 +13,9 @@ # limitations under the License. from __future__ import absolute_import -import os -import pathlib import nox +import pathlib CURRENT_DIRECTORY = pathlib.Path(__file__).parent.absolute() @@ -30,6 +29,7 @@ "3.10", "3.11", "3.12", + "3.13", ] # Error if a python version is missing @@ -37,26 +37,30 @@ @nox.session(python=PYTHON_VERSIONS) -def unit(session, proto="python"): +@nox.parametrize("implementation", ["cpp", "upb", "python"]) +def unit(session, implementation): """Run the unit test suite.""" constraints_path = str( CURRENT_DIRECTORY / "testing" / f"constraints-{session.python}.txt" ) - session.env["PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION"] = proto + session.env["PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION"] = implementation session.install("coverage", "pytest", "pytest-cov", "pytz") session.install("-e", ".[testing]", "-c", constraints_path) - if proto == "cpp": # 4.20 does not have cpp. - session.install("protobuf==3.19.0") + # TODO(https://github.com/googleapis/proto-plus-python/issues/389): + # Remove the 'cpp' implementation once support for Protobuf 3.x is dropped. + # The 'cpp' implementation requires Protobuf<4. + if implementation == "cpp": + session.install("protobuf<4") # TODO(https://github.com/googleapis/proto-plus-python/issues/403): re-enable `-W=error` # The warnings-as-errors flag `-W=error` was removed in # https://github.com/googleapis/proto-plus-python/pull/400. # It should be re-added once issue - # https://github.com/protocolbuffers/protobuf/issues/12186 is fixed. + # https://github.com/protocolbuffers/protobuf/issues/15077 is fixed. session.run( - "py.test", + "pytest", "--quiet", *( session.posargs # Coverage info when running individual tests is annoying. @@ -71,17 +75,59 @@ def unit(session, proto="python"): ) -# Check if protobuf has released wheels for new python versions -# https://pypi.org/project/protobuf/#files -# This list will generally be shorter than 'unit' -@nox.session(python=["3.6", "3.7", "3.8", "3.9", "3.10"]) -def unitcpp(session): - return unit(session, proto="cpp") +# Only test upb and python implementation backends. +# As of protobuf 4.x, the "ccp" implementation is not available in the PyPI package as per +# https://github.com/protocolbuffers/protobuf/tree/main/python#implementation-backends +@nox.session(python=PYTHON_VERSIONS[-2]) +@nox.parametrize("implementation", ["python", "upb"]) +def prerelease_deps(session, implementation): + """Run the unit test suite against pre-release versions of dependencies.""" + session.env["PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION"] = implementation -@nox.session(python=PYTHON_VERSIONS) -def unitupb(session): - return unit(session, proto="upb") + # Install test environment dependencies + session.install("coverage", "pytest", "pytest-cov", "pytz") + + # Install the package without dependencies + session.install("-e", ".", "--no-deps") + + prerel_deps = [ + "google-api-core", + # dependency of google-api-core + "googleapis-common-protos", + ] + + for dep in prerel_deps: + session.install("--pre", "--no-deps", "--upgrade", dep) + + session.install("--pre", "--upgrade", "protobuf") + # Print out prerelease package versions + session.run( + "python", "-c", "import google.protobuf; print(google.protobuf.__version__)" + ) + session.run( + "python", "-c", "import google.api_core; print(google.api_core.__version__)" + ) + + # TODO(https://github.com/googleapis/proto-plus-python/issues/403): re-enable `-W=error` + # The warnings-as-errors flag `-W=error` was removed in + # https://github.com/googleapis/proto-plus-python/pull/400. + # It should be re-added once issue + # https://github.com/protocolbuffers/protobuf/issues/15077 is fixed. + session.run( + "pytest", + "--quiet", + *( + session.posargs # Coverage info when running individual tests is annoying. + or [ + "--cov=proto", + "--cov-config=.coveragerc", + "--cov-report=term", + "--cov-report=html", + "tests", + ] + ), + ) @nox.session(python="3.9") diff --git a/proto/marshal/compat.py b/proto/marshal/compat.py index e10999bb..dc77f3ac 100644 --- a/proto/marshal/compat.py +++ b/proto/marshal/compat.py @@ -19,6 +19,9 @@ # not be included. from google.protobuf.internal import containers +import google.protobuf + +PROTOBUF_VERSION = google.protobuf.__version__ # Import protobuf 4.xx first and fallback to earlier version # if not present. @@ -37,14 +40,28 @@ repeated_scalar_types = (containers.RepeatedScalarFieldContainer,) map_composite_types = (containers.MessageMap,) +# In `proto/marshal.py`, for compatibility with protobuf 5.x, +# we'll use `map_composite_type_names` to check whether +# the name of the class of a protobuf type is +# `MessageMapContainer`, and, if `True`, return a MapComposite. +# See https://github.com/protocolbuffers/protobuf/issues/16596 +map_composite_type_names = ("MessageMapContainer",) + if _message: repeated_composite_types += (_message.RepeatedCompositeContainer,) repeated_scalar_types += (_message.RepeatedScalarContainer,) - map_composite_types += (_message.MessageMapContainer,) + # In `proto/marshal.py`, for compatibility with protobuf 5.x, + # we'll use `map_composite_type_names` to check whether + # the name of the class of a protobuf type is + # `MessageMapContainer`, and, if `True`, return a MapComposite. + # See https://github.com/protocolbuffers/protobuf/issues/16596 + if PROTOBUF_VERSION[0:2] in ["3.", "4."]: + map_composite_types += (_message.MessageMapContainer,) __all__ = ( "repeated_composite_types", "repeated_scalar_types", "map_composite_types", + "map_composite_type_names", ) diff --git a/proto/marshal/marshal.py b/proto/marshal/marshal.py index e9fbbb9f..d278421a 100644 --- a/proto/marshal/marshal.py +++ b/proto/marshal/marshal.py @@ -188,7 +188,14 @@ def to_python(self, proto_type, value, *, absent: bool = None): return Repeated(value, marshal=self) # Same thing for maps of messages. - if value_type in compat.map_composite_types: + # See https://github.com/protocolbuffers/protobuf/issues/16596 + # We need to look up the name of the type in compat.map_composite_type_names + # as class `MessageMapContainer` is no longer exposed + # This is done to avoid taking a breaking change in proto-plus. + if ( + value_type in compat.map_composite_types + or value_type.__name__ in compat.map_composite_type_names + ): return MapComposite(value, marshal=self) return self.get_rule(proto_type=proto_type).to_python(value, absent=absent) diff --git a/proto/message.py b/proto/message.py index 7232d42f..2772fb42 100644 --- a/proto/message.py +++ b/proto/message.py @@ -16,8 +16,10 @@ import collections.abc import copy import re -from typing import List, Type +from typing import List, Optional, Type +import warnings +import google.protobuf from google.protobuf import descriptor_pb2 from google.protobuf import message from google.protobuf.json_format import MessageToDict, MessageToJson, Parse @@ -32,6 +34,8 @@ from proto.utils import has_upb +PROTOBUF_VERSION = google.protobuf.__version__ + _upb = has_upb() # Important to cache result here. @@ -369,16 +373,101 @@ def deserialize(cls, payload: bytes) -> "Message": """ return cls.wrap(cls.pb().FromString(payload)) + def _warn_if_including_default_value_fields_is_used_protobuf_5( + cls, including_default_value_fields: Optional[bool] + ) -> None: + """ + Warn Protobuf 5.x+ users that `including_default_value_fields` is deprecated if it is set. + + Args: + including_default_value_fields (Optional(bool)): The value of `including_default_value_fields` set by the user. + """ + if ( + PROTOBUF_VERSION[0] not in ("3", "4") + and including_default_value_fields is not None + ): + warnings.warn( + """The argument `including_default_value_fields` has been removed from + Protobuf 5.x. Please use `always_print_fields_with_no_presence` instead. + """, + DeprecationWarning, + ) + + def _raise_if_print_fields_values_are_set_and_differ( + cls, + always_print_fields_with_no_presence: Optional[bool], + including_default_value_fields: Optional[bool], + ) -> None: + """ + Raise Exception if both `always_print_fields_with_no_presence` and `including_default_value_fields` are set + and the values differ. + + Args: + always_print_fields_with_no_presence (Optional(bool)): The value of `always_print_fields_with_no_presence` set by the user. + including_default_value_fields (Optional(bool)): The value of `including_default_value_fields` set by the user. + Returns: + None + Raises: + ValueError: if both `always_print_fields_with_no_presence` and `including_default_value_fields` are set and + the values differ. + """ + if ( + always_print_fields_with_no_presence is not None + and including_default_value_fields is not None + and always_print_fields_with_no_presence != including_default_value_fields + ): + raise ValueError( + "Arguments `always_print_fields_with_no_presence` and `including_default_value_fields` must match" + ) + + def _normalize_print_fields_without_presence( + cls, + always_print_fields_with_no_presence: Optional[bool], + including_default_value_fields: Optional[bool], + ) -> bool: + """ + Return true if fields with no presence should be included in the results. + By default, fields with no presence will be included in the results + when both `always_print_fields_with_no_presence` and + `including_default_value_fields` are not set + + Args: + always_print_fields_with_no_presence (Optional(bool)): The value of `always_print_fields_with_no_presence` set by the user. + including_default_value_fields (Optional(bool)): The value of `including_default_value_fields` set by the user. + Returns: + None + Raises: + ValueError: if both `always_print_fields_with_no_presence` and `including_default_value_fields` are set and + the values differ. + """ + + cls._warn_if_including_default_value_fields_is_used_protobuf_5( + including_default_value_fields + ) + cls._raise_if_print_fields_values_are_set_and_differ( + always_print_fields_with_no_presence, including_default_value_fields + ) + # Default to True if neither `always_print_fields_with_no_presence` or `including_default_value_fields` is set + return ( + ( + always_print_fields_with_no_presence is None + and including_default_value_fields is None + ) + or always_print_fields_with_no_presence + or including_default_value_fields + ) + def to_json( cls, instance, *, use_integers_for_enums=True, - including_default_value_fields=True, + including_default_value_fields=None, preserving_proto_field_name=False, sort_keys=False, indent=2, float_precision=None, + always_print_fields_with_no_presence=None, ) -> str: """Given a message instance, serialize it to json @@ -388,6 +477,11 @@ def to_json( use_integers_for_enums (Optional(bool)): An option that determines whether enum values should be represented by strings (False) or integers (True). Default is True. + including_default_value_fields (Optional(bool)): Deprecated. Use argument + `always_print_fields_with_no_presence` instead. An option that + determines whether the default field values should be included in the results. + This value must match `always_print_fields_with_no_presence`, + if both arguments are explictly set. preserving_proto_field_name (Optional(bool)): An option that determines whether field name representations preserve proto case (snake_case) or use lowerCamelCase. Default is False. @@ -398,19 +492,46 @@ def to_json( Pass None for the most compact representation without newlines. float_precision (Optional(int)): If set, use this to specify float field valid digits. Default is None. + always_print_fields_with_no_presence (Optional(bool)): If True, fields without + presence (implicit presence scalars, repeated fields, and map fields) will + always be serialized. Any field that supports presence is not affected by + this option (including singular message fields and oneof fields). + This value must match `including_default_value_fields`, + if both arguments are explictly set. Returns: str: The json string representation of the protocol buffer. """ - return MessageToJson( - cls.pb(instance), - use_integers_for_enums=use_integers_for_enums, - including_default_value_fields=including_default_value_fields, - preserving_proto_field_name=preserving_proto_field_name, - sort_keys=sort_keys, - indent=indent, - float_precision=float_precision, + + print_fields = cls._normalize_print_fields_without_presence( + always_print_fields_with_no_presence, including_default_value_fields ) + if PROTOBUF_VERSION[0] in ("3", "4"): + return MessageToJson( + cls.pb(instance), + use_integers_for_enums=use_integers_for_enums, + including_default_value_fields=print_fields, + preserving_proto_field_name=preserving_proto_field_name, + sort_keys=sort_keys, + indent=indent, + float_precision=float_precision, + ) + else: + # The `including_default_value_fields` argument was removed from protobuf 5.x + # and replaced with `always_print_fields_with_no_presence` which very similar but has + # handles optional fields consistently by not affecting them. + # The old flag accidentally had inconsistent behavior between proto2 + # optional and proto3 optional fields. + return MessageToJson( + cls.pb(instance), + use_integers_for_enums=use_integers_for_enums, + always_print_fields_with_no_presence=print_fields, + preserving_proto_field_name=preserving_proto_field_name, + sort_keys=sort_keys, + indent=indent, + float_precision=float_precision, + ) + def from_json(cls, payload, *, ignore_unknown_fields=False) -> "Message": """Given a json string representing an instance, parse it into a message. @@ -434,39 +555,66 @@ def to_dict( *, use_integers_for_enums=True, preserving_proto_field_name=True, - including_default_value_fields=True, + including_default_value_fields=None, float_precision=None, + always_print_fields_with_no_presence=None, ) -> "Message": """Given a message instance, return its representation as a python dict. Args: instance: An instance of this message type, or something - compatible (accepted by the type's constructor). + compatible (accepted by the type's constructor). use_integers_for_enums (Optional(bool)): An option that determines whether enum values should be represented by strings (False) or integers (True). Default is True. preserving_proto_field_name (Optional(bool)): An option that determines whether field name representations preserve proto case (snake_case) or use lowerCamelCase. Default is True. - including_default_value_fields (Optional(bool)): An option that + including_default_value_fields (Optional(bool)): Deprecated. Use argument + `always_print_fields_with_no_presence` instead. An option that determines whether the default field values should be included in the results. - Default is True. + This value must match `always_print_fields_with_no_presence`, + if both arguments are explictly set. float_precision (Optional(int)): If set, use this to specify float field valid digits. Default is None. + always_print_fields_with_no_presence (Optional(bool)): If True, fields without + presence (implicit presence scalars, repeated fields, and map fields) will + always be serialized. Any field that supports presence is not affected by + this option (including singular message fields and oneof fields). This value + must match `including_default_value_fields`, if both arguments are explictly set. Returns: dict: A representation of the protocol buffer using pythonic data structures. Messages and map fields are represented as dicts, repeated fields are represented as lists. """ - return MessageToDict( - cls.pb(instance), - including_default_value_fields=including_default_value_fields, - preserving_proto_field_name=preserving_proto_field_name, - use_integers_for_enums=use_integers_for_enums, - float_precision=float_precision, + + print_fields = cls._normalize_print_fields_without_presence( + always_print_fields_with_no_presence, including_default_value_fields ) + if PROTOBUF_VERSION[0] in ("3", "4"): + return MessageToDict( + cls.pb(instance), + including_default_value_fields=print_fields, + preserving_proto_field_name=preserving_proto_field_name, + use_integers_for_enums=use_integers_for_enums, + float_precision=float_precision, + ) + else: + # The `including_default_value_fields` argument was removed from protobuf 5.x + # and replaced with `always_print_fields_with_no_presence` which very similar but has + # handles optional fields consistently by not affecting them. + # The old flag accidentally had inconsistent behavior between proto2 + # optional and proto3 optional fields. + return MessageToDict( + cls.pb(instance), + always_print_fields_with_no_presence=print_fields, + preserving_proto_field_name=preserving_proto_field_name, + use_integers_for_enums=use_integers_for_enums, + float_precision=float_precision, + ) + def copy_from(cls, instance, other): """Equivalent for protobuf.Message.CopyFrom diff --git a/pytest.ini b/pytest.ini index 985c2559..ba89114a 100644 --- a/pytest.ini +++ b/pytest.ini @@ -4,3 +4,5 @@ filterwarnings = error # Remove once https://github.com/protocolbuffers/protobuf/issues/12186 is fixed ignore:.*custom tp_new.*in Python 3.14:DeprecationWarning + # Remove once deprecated field `including_default_value_fields` is removed + ignore:.*The argument `including_default_value_fields` has been removed.*:DeprecationWarning diff --git a/setup.py b/setup.py index 796f0a91..1de20941 100644 --- a/setup.py +++ b/setup.py @@ -45,7 +45,7 @@ install_requires=("protobuf >= 3.19.0, <5.0.0dev",), extras_require={ "testing": [ - "google-api-core[grpc] >= 1.31.5", + "google-api-core >= 1.31.5", ], }, python_requires=">=3.6", diff --git a/testing/constraints-3.13.txt b/testing/constraints-3.13.txt new file mode 100644 index 00000000..e69de29b diff --git a/tests/conftest.py b/tests/conftest.py index 765326c6..9f2f5c95 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -41,9 +41,11 @@ def pytest_runtest_setup(item): item._mocks.append( mock.patch( - "google._upb._message.default_pool" - if has_upb() - else "google.protobuf.pyext._message.default_pool", + ( + "google._upb._message.default_pool" + if has_upb() + else "google.protobuf.pyext._message.default_pool" + ), pool, ) ) @@ -67,6 +69,14 @@ def pytest_runtest_setup(item): for name in dir(item.module): if name.endswith("_pb2") and not name.startswith("test_"): module = getattr(item.module, name) + + # Exclude `google.protobuf.descriptor_pb2` which causes error + # `RecursionError: maximum recursion depth exceeded while calling a Python object` + # when running the test suite and is not required for tests. + # See https://github.com/googleapis/proto-plus-python/issues/425 + if module.__package__ == "google.protobuf" and name == "descriptor_pb2": + continue + pool.AddSerializedFile(module.DESCRIPTOR.serialized_pb) fd = pool.FindFileByName(module.DESCRIPTOR.name) diff --git a/tests/test_json.py b/tests/test_json.py index e94e935a..ae3cf59e 100644 --- a/tests/test_json.py +++ b/tests/test_json.py @@ -121,10 +121,77 @@ class Squid(proto.Message): ) assert json1 == '{"name":"Steve"}' - json2 = Squid.to_json(s).replace(" ", "").replace("\n", "") - assert ( - json2 == '{"name":"Steve","massKg":0}' or json2 == '{"massKg":0,"name":"Steve"}' + json1 = ( + Squid.to_json(s, always_print_fields_with_no_presence=False) + .replace(" ", "") + .replace("\n", "") ) + assert json1 == '{"name":"Steve"}' + + json1 = ( + Squid.to_json( + s, + including_default_value_fields=False, + always_print_fields_with_no_presence=False, + ) + .replace(" ", "") + .replace("\n", "") + ) + assert json1 == '{"name":"Steve"}' + + with pytest.raises( + ValueError, + match="Arguments.*always_print_fields_with_no_presence.*including_default_value_fields.*must match", + ): + Squid.to_json( + s, + including_default_value_fields=True, + always_print_fields_with_no_presence=False, + ).replace(" ", "").replace("\n", "") + + with pytest.raises( + ValueError, + match="Arguments.*always_print_fields_with_no_presence.*including_default_value_fields.*must match", + ): + Squid.to_json( + s, + including_default_value_fields=False, + always_print_fields_with_no_presence=True, + ).replace(" ", "").replace("\n", "") + + json2 = ( + Squid.to_json( + s, + including_default_value_fields=True, + always_print_fields_with_no_presence=True, + ) + .replace(" ", "") + .replace("\n", "") + ) + assert json2 == '{"name":"Steve","massKg":0}' + + json2 = ( + Squid.to_json( + s, + including_default_value_fields=True, + ) + .replace(" ", "") + .replace("\n", "") + ) + assert json2 == '{"name":"Steve","massKg":0}' + + json2 = ( + Squid.to_json( + s, + always_print_fields_with_no_presence=True, + ) + .replace(" ", "") + .replace("\n", "") + ) + assert json2 == '{"name":"Steve","massKg":0}' + + json2 = Squid.to_json(s).replace(" ", "").replace("\n", "") + assert json2 == '{"name":"Steve","massKg":0}' s1 = Squid.from_json(json1) s2 = Squid.from_json(json2) diff --git a/tests/test_message.py b/tests/test_message.py index 983cde82..720995a9 100644 --- a/tests/test_message.py +++ b/tests/test_message.py @@ -267,8 +267,63 @@ class Color(proto.Enum): expected_dict = {"mass_kg": 20} assert s_dict_2 == expected_dict - new_s = Squid(s_dict) - assert new_s == s + s_dict_2 = Squid.to_dict(s_new_2, always_print_fields_with_no_presence=False) + expected_dict = {"mass_kg": 20} + assert s_dict_2 == expected_dict + + s_dict_2 = Squid.to_dict( + s_new_2, + including_default_value_fields=False, + always_print_fields_with_no_presence=False, + ) + expected_dict = {"mass_kg": 20} + assert s_dict_2 == expected_dict + + s_dict_2 = Squid.to_dict( + s_new_2, + including_default_value_fields=True, + ) + expected_dict = {"mass_kg": 20, "chromatophores": []} + assert s_dict_2 == expected_dict + + s_dict_2 = Squid.to_dict( + s_new_2, + always_print_fields_with_no_presence=True, + ) + expected_dict = {"mass_kg": 20, "chromatophores": []} + assert s_dict_2 == expected_dict + + s_dict_2 = Squid.to_dict( + s_new_2, + including_default_value_fields=True, + always_print_fields_with_no_presence=True, + ) + expected_dict = {"mass_kg": 20, "chromatophores": []} + assert s_dict_2 == expected_dict + + s_dict_2 = Squid.to_dict(s_new_2) + expected_dict = {"mass_kg": 20, "chromatophores": []} + assert s_dict_2 == expected_dict + + with pytest.raises( + ValueError, + match="Arguments.*always_print_fields_with_no_presence.*including_default_value_fields.*must match", + ): + s_dict_2 = Squid.to_dict( + s_new_2, + including_default_value_fields=True, + always_print_fields_with_no_presence=False, + ) + + with pytest.raises( + ValueError, + match="Arguments.*always_print_fields_with_no_presence.*including_default_value_fields.*must match", + ): + s_dict_2 = Squid.to_dict( + s_new_2, + including_default_value_fields=False, + always_print_fields_with_no_presence=True, + ) # TODO: https://github.com/googleapis/proto-plus-python/issues/390