search

googleapis / proto-plus-python

Beautiful, idiomatic protocol buffers in Python
Apache License 2.0
169 stars 35 forks source link

Three failing tests (apparently with changes in attributs on functions) #311

Closed mcepl closed 2 years ago

mcepl commented 2 years ago

Environment details

Steps to reproduce

  1. Run the test suite

Results

[   11s] + PYTHONPATH=/home/abuild/rpmbuild/BUILDROOT/python-proto-plus-test-1.20.3-0.x86_64/usr/lib/python3.9/site-packages
[   11s] + PYTHONDONTWRITEBYTECODE=1
[   11s] + pytest-3.9 --ignore=_build.python39 --ignore=_build.python310 --ignore=_build.python38 -v
[   12s] ============================= test session starts ==============================
[   12s] platform linux -- Python 3.9.12, pytest-7.1.1, pluggy-1.0.0 -- /usr/bin/python3.9
[   12s] cachedir: .pytest_cache
[   12s] rootdir: /home/abuild/rpmbuild/BUILD/proto-plus-1.20.3
[   12s] collecting ... collected 253 items
[   12s]

[ removed PASSed tests ]

[   13s]
[   13s] =================================== FAILURES ===================================
[   13s] ________________________ test_fields_mitigate_collision ________________________
[   13s]
[   13s] self = <[AttributeError("'_pb'") raised in repr()] TestMessage object at 0x7f7b9d82a100>
[   13s] mapping = {'eggs': 'has_eggs', 'spam': 'has_spam'}
[   13s] ignore_unknown_fields = False, kwargs = {}, params = {}
[   13s] marshal = <proto.marshal.marshal.Marshal object at 0x7f7b9d8237f0>, key = 'spam'
[   13s] value = 'has_spam'
[   13s]
[   13s]     def __init__(
[   13s]         self, mapping=None, *, ignore_unknown_fields=False, **kwargs,
[   13s]     ):
[   13s]         # We accept several things for `mapping`:
[   13s]         #   * An instance of this class.
[   13s]         #   * An instance of the underlying protobuf descriptor class.
[   13s]         #   * A dict
[   13s]         #   * Nothing (keyword arguments only).
[   13s]         if mapping is None:
[   13s]             if not kwargs:
[   13s]                 # Special fast path for empty construction.
[   13s]                 super().__setattr__("_pb", self._meta.pb())
[   13s]                 return
[   13s]
[   13s]             mapping = kwargs
[   13s]         elif isinstance(mapping, self._meta.pb):
[   13s]             # Make a copy of the mapping.
[   13s]             # This is a constructor for a new object, so users will assume
[   13s]             # that it will not have side effects on the arguments being
[   13s]             # passed in.
[   13s]             #
[   13s]             # The `wrap` method on the metaclass is the public API for taking
[   13s]             # ownership of the passed in protobuf object.
[   13s]             mapping = copy.deepcopy(mapping)
[   13s]             if kwargs:
[   13s]                 mapping.MergeFrom(self._meta.pb(**kwargs))
[   13s]
[   13s]             super().__setattr__("_pb", mapping)
[   13s]             return
[   13s]         elif isinstance(mapping, type(self)):
[   13s]             # Just use the above logic on mapping's underlying pb.
[   13s]             self.__init__(mapping=mapping._pb, **kwargs)
[   13s]             return
[   13s]         elif isinstance(mapping, collections.abc.Mapping):
[   13s]             # Can't have side effects on mapping.
[   13s]             mapping = copy.copy(mapping)
[   13s]             # kwargs entries take priority for duplicate keys.
[   13s]             mapping.update(kwargs)
[   13s]         else:
[   13s]             # Sanity check: Did we get something not a map? Error if so.
[   13s]             raise TypeError(
[   13s]                 "Invalid constructor input for %s: %r"
[   13s]                 % (self.__class__.__name__, mapping,)
[   13s]             )
[   13s]
[   13s]         params = {}
[   13s]         # Update the mapping to address any values that need to be
[   13s]         # coerced.
[   13s]         marshal = self._meta.marshal
[   13s]         for key, value in mapping.items():
[   13s]             try:
[   13s] >               pb_type = self._meta.fields[key].pb_type
[   13s] E               KeyError: 'spam'
[   13s]
[   13s] /usr/lib/python3.9/site-packages/proto/message.py:507: KeyError
[   13s]
[   13s] During handling of the above exception, another exception occurred:
[   13s]
[   13s]     def test_fields_mitigate_collision():
[   13s]         class TestMessage(proto.Message):
[   13s]             spam_ = proto.Field(proto.STRING, number=1)
[   13s]             eggs = proto.Field(proto.STRING, number=2)
[   13s]
[   13s]         class TextStream(proto.Message):
[   13s]             text_stream = proto.Field(TestMessage, number=1)
[   13s]
[   13s]         obj = TestMessage(spam_="has_spam")
[   13s]         obj.eggs = "has_eggs"
[   13s]         assert obj.spam_ == "has_spam"
[   13s]
[   13s]         # Test that `spam` is coerced to `spam_`
[   13s] >       modified_obj = TestMessage({"spam": "has_spam", "eggs": "has_eggs"})
[   13s]
[   13s] tests/test_fields_mitigate_collision.py:38:
[   13s] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
[   13s]
[   13s] self = <[AttributeError("'_pb'") raised in repr()] TestMessage object at 0x7f7b9d82a100>
[   13s] mapping = {'eggs': 'has_eggs', 'spam': 'has_spam'}
[   13s] ignore_unknown_fields = False, kwargs = {}, params = {}
[   13s] marshal = <proto.marshal.marshal.Marshal object at 0x7f7b9d8237f0>, key = 'spam'
[   13s] value = 'has_spam'
[   13s]
[   13s]     def __init__(
[   13s]         self, mapping=None, *, ignore_unknown_fields=False, **kwargs,
[   13s]     ):
[   13s]         # We accept several things for `mapping`:
[   13s]         #   * An instance of this class.
[   13s]         #   * An instance of the underlying protobuf descriptor class.
[   13s]         #   * A dict
[   13s]         #   * Nothing (keyword arguments only).
[   13s]         if mapping is None:
[   13s]             if not kwargs:
[   13s]                 # Special fast path for empty construction.
[   13s]                 super().__setattr__("_pb", self._meta.pb())
[   13s]                 return
[   13s]
[   13s]             mapping = kwargs
[   13s]         elif isinstance(mapping, self._meta.pb):
[   13s]             # Make a copy of the mapping.
[   13s]             # This is a constructor for a new object, so users will assume
[   13s]             # that it will not have side effects on the arguments being
[   13s]             # passed in.
[   13s]             #
[   13s]             # The `wrap` method on the metaclass is the public API for taking
[   13s]             # ownership of the passed in protobuf object.
[   13s]             mapping = copy.deepcopy(mapping)
[   13s]             if kwargs:
[   13s]                 mapping.MergeFrom(self._meta.pb(**kwargs))
[   13s]
[   13s]             super().__setattr__("_pb", mapping)
[   13s]             return
[   13s]         elif isinstance(mapping, type(self)):
[   13s]             # Just use the above logic on mapping's underlying pb.
[   13s]             self.__init__(mapping=mapping._pb, **kwargs)
[   13s]             return
[   13s]         elif isinstance(mapping, collections.abc.Mapping):
[   13s]             # Can't have side effects on mapping.
[   13s]             mapping = copy.copy(mapping)
[   13s]             # kwargs entries take priority for duplicate keys.
[   13s]             mapping.update(kwargs)
[   13s]         else:
[   13s]             # Sanity check: Did we get something not a map? Error if so.
[   13s]             raise TypeError(
[   13s]                 "Invalid constructor input for %s: %r"
[   13s]                 % (self.__class__.__name__, mapping,)
[   13s]             )
[   13s]
[   13s]         params = {}
[   13s]         # Update the mapping to address any values that need to be
[   13s]         # coerced.
[   13s]         marshal = self._meta.marshal
[   13s]         for key, value in mapping.items():
[   13s]             try:
[   13s]                 pb_type = self._meta.fields[key].pb_type
[   13s]             except KeyError:
[   13s]                 if ignore_unknown_fields:
[   13s]                     continue
[   13s]
[   13s] >               raise ValueError(
[   13s]                     "Unknown field for {}: {}".format(self.__class__.__name__, key)
[   13s]                 )
[   13s] E               ValueError: Unknown field for TestMessage: spam
[   13s]
[   13s] /usr/lib/python3.9/site-packages/proto/message.py:512: ValueError
[   13s] ___________________________________ test_dir ___________________________________
[   13s]
[   13s]     def test_dir():
[   13s]         class Mollusc(proto.Message):
[   13s]             class Class(proto.Enum):
[   13s]                 UNKNOWN = 0
[   13s]                 GASTROPOD = 1
[   13s]                 BIVALVE = 2
[   13s]                 CEPHALOPOD = 3
[   13s]
[   13s]             class Arm(proto.Message):
[   13s]                 length_cm = proto.Field(proto.INT32, number=1)
[   13s]
[   13s]             mass_kg = proto.Field(proto.INT32, number=1)
[   13s]             class_ = proto.Field(Class, number=2)
[   13s]             arms = proto.RepeatedField(Arm, number=3)
[   13s]
[   13s]         expected = (
[   13s]             {
[   13s]                 # Fields and nested message and enum types
[   13s]                 "arms",
[   13s]                 "class_",
[   13s]                 "mass_kg",
[   13s]                 "Arm",
[   13s]                 "Class",
[   13s]             }
[   13s]             | {
[   13s]                 # Other methods and attributes
[   13s]                 "__bool__",
[   13s]                 "__contains__",
[   13s]                 "__dict__",
[   13s]                 "__getattr__",
[   13s]                 "__getstate__",
[   13s]                 "__module__",
[   13s]                 "__setstate__",
[   13s]                 "__weakref__",
[   13s]             }
[   13s]             | set(dir(object))
[   13s]         )  # Gets the long tail of dunder methods and attributes.
[   13s]
[   13s]         actual = set(dir(Mollusc()))
[   13s]
[   13s]         # Check instance names
[   13s] >       assert actual == expected
[   13s] E       AssertionError: assert {'Arm', 'Clas...lattr__', ...} == {'Arm', 'Clas...lattr__', ...}
[   13s] E         Extra items in the left set:
[   13s] E         '_meta'
[   13s] E         '_pb'
[   13s] E         Extra items in the right set:
[   13s] E         'class_'
[   13s] E         'mass_kg'
[   13s] E         'arms'...
[   13s] E
[   13s] E         ...Full output truncated (42 lines hidden), use '-vv' to show
[   13s]
[   13s] tests/test_message.py:392: AssertionError
[   13s] ____________________________ test_dir_message_base _____________________________
[   13s]
[   13s]     def test_dir_message_base():
[   13s] >       assert set(dir(proto.Message)) == set(dir(type))
[   13s] E       AssertionError: assert {'__bool__', ...__dir__', ...} == {'__abstractm...class__', ...}
[   13s] E         Extra items in the left set:
[   13s] E         '__getstate__'
[   13s] E         '__contains__'
[   13s] E         '__weakref__'
[   13s] E         '__getattr__'
[   13s] E         '__bool__'
[   13s] E         '__setstate__'...
[   13s] E
[   13s] E         ...Full output truncated (75 lines hidden), use '-vv' to show
[   13s]
[   13s] tests/test_message.py:421: AssertionError
[   13s] =========================== short test summary info ============================
[   13s] FAILED tests/test_fields_mitigate_collision.py::test_fields_mitigate_collision
[   13s] FAILED tests/test_message.py::test_dir - AssertionError: assert {'Arm', 'Clas...
[   13s] FAILED tests/test_message.py::test_dir_message_base - AssertionError: assert ...
[   13s] ======================== 3 failed, 250 passed in 1.83s =========================
[   14s] error: Bad exit status from /var/tmp/rpm-tmp.Qf83UP (%check)

Complete build log with all versions of packages used and steps taken to reproduce.

parthea commented 2 years ago

Hi @mcepl,

The tests will fail with proto-plus < 1.20.3 . In the build log that you shared I see python39-proto-plus-1.19.9-10.2. Please upgrade this to 1.20.3 and the test should pass. I'm going to close this issue as it appears that an older version of proto-plus is used. Please re-open this issue if this is not the case.

mcepl commented 2 years ago

That was probably it. Thank you.