From b0f81596be52071a9f025f4dced3f848c739b5bc Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Wed, 29 Dec 2021 09:58:12 -0800 Subject: [PATCH] All tests passing! --- python/descriptor.c | 40 +++++++----- python/descriptor_pool.c | 12 ---- python/descriptor_pool.h | 3 + python/minimal_test.py | 4 ++ .../descriptor_database_test_wrapper.py | 3 - .../descriptor_pool_test_wrapper.py | 63 +++++++++---------- .../pb_unit_tests/generator_test_wrapper.py | 1 - upb/util/def_to_proto.c | 12 ++-- 8 files changed, 70 insertions(+), 68 deletions(-) diff --git a/python/descriptor.c b/python/descriptor.c index 3877e55a5b..a9718da746 100644 --- a/python/descriptor.c +++ b/python/descriptor.c @@ -30,6 +30,7 @@ #include "python/convert.h" #include "python/descriptor_containers.h" #include "python/descriptor_pool.h" +#include "python/message.h" #include "python/protobuf.h" #include "upb/def.h" #include "upb/util/def_to_proto.h" @@ -144,13 +145,9 @@ static PyObject* PyUpb_DescriptorBase_GetOptions(PyUpb_DescriptorBase* self, typedef void* PyUpb_ToProto_Func(const void* def, upb_arena* arena); -static PyObject* PyUpb_DescriptorBase_CopyToProto(PyObject* _self, - PyUpb_ToProto_Func* func, - const upb_msglayout* layout, - PyObject* py_proto) { +static PyObject* PyUpb_DescriptorBase_GetSerializedProto( + PyObject* _self, PyUpb_ToProto_Func* func, const upb_msglayout* layout) { PyUpb_DescriptorBase* self = (void*)_self; - PyObject* ret = NULL; - PyObject* str = NULL; upb_arena* arena = upb_arena_new(); if (!arena) PYUPB_RETURN_OOM; upb_msg* proto = func(self->def, arena); @@ -158,20 +155,23 @@ static PyObject* PyUpb_DescriptorBase_CopyToProto(PyObject* _self, size_t size; char* pb = upb_encode(proto, layout, arena, &size); if (!pb) goto oom; - str = PyBytes_FromStringAndSize(pb, size); - if (!str) goto oom; - // Disabled until python/message.c is reviewed and checked in. - // PyObject *ret = PyUpb_CMessage_MergeFromString(py_proto, str); - PyErr_Format(PyExc_NotImplementedError, "Not yet implemented"); - -done: - Py_XDECREF(str); + PyObject* str = PyBytes_FromStringAndSize(pb, size); upb_arena_free(arena); - return ret; + return str; oom: PyErr_SetNone(PyExc_MemoryError); - goto done; + return NULL; +} + +static PyObject* PyUpb_DescriptorBase_CopyToProto(PyObject* _self, + PyUpb_ToProto_Func* func, + const upb_msglayout* layout, + PyObject* py_proto) { + PyObject* serialized = + PyUpb_DescriptorBase_GetSerializedProto(_self, func, layout); + if (!serialized) return NULL; + return PyUpb_CMessage_MergeFromString(py_proto, serialized); } static void PyUpb_DescriptorBase_Dealloc(PyUpb_DescriptorBase* base) { @@ -1065,6 +1065,13 @@ static PyObject* PyUpb_FileDescriptor_GetPackage(PyObject* _self, return PyUnicode_FromString(upb_filedef_package(self->def)); } +static PyObject* PyUpb_FileDescriptor_GetSerializedPb(PyObject* self, + void* closure) { + return PyUpb_DescriptorBase_GetSerializedProto( + self, (PyUpb_ToProto_Func*)&upb_FileDef_ToProto, + &google_protobuf_FileDescriptorProto_msginit); +} + static PyObject* PyUpb_FileDescriptor_GetMessageTypesByName(PyObject* _self, void* closure) { static PyUpb_ByNameMap_Funcs funcs = { @@ -1180,6 +1187,7 @@ static PyGetSetDef PyUpb_FileDescriptor_Getters[] = { {"pool", PyUpb_FileDescriptor_GetPool, NULL, "pool"}, {"name", (getter)PyUpb_FileDescriptor_GetName, NULL, "name"}, {"package", PyUpb_FileDescriptor_GetPackage, NULL, "package"}, + {"serialized_pb", PyUpb_FileDescriptor_GetSerializedPb}, {"message_types_by_name", PyUpb_FileDescriptor_GetMessageTypesByName, NULL, "Messages by name"}, {"enum_types_by_name", PyUpb_FileDescriptor_GetEnumTypesByName, NULL, diff --git a/python/descriptor_pool.c b/python/descriptor_pool.c index e48b82c243..e816f4e213 100644 --- a/python/descriptor_pool.c +++ b/python/descriptor_pool.c @@ -40,7 +40,6 @@ typedef struct { PyObject_HEAD upb_symtab* symtab; PyObject* db; - PyObject* serialized_pb_dict; } PyUpb_DescriptorPool; PyObject* PyUpb_DescriptorPool_GetDefaultPool() { @@ -53,7 +52,6 @@ static PyObject* PyUpb_DescriptorPool_DoCreateWithCache( PyUpb_DescriptorPool* pool = (void*)PyType_GenericAlloc(type, 0); pool->symtab = upb_symtab_new(); pool->db = db; - pool->serialized_pb_dict = PyDict_New(); Py_XINCREF(pool->db); PyUpb_WeakMap_Add(obj_cache, pool->symtab, &pool->ob_base); return &pool->ob_base; @@ -88,18 +86,11 @@ PyObject* PyUpb_DescriptorPool_Get(const upb_symtab* symtab) { static void PyUpb_DescriptorPool_Dealloc(PyUpb_DescriptorPool* self) { PyUpb_DescriptorPool_Clear(self); - Py_DECREF(self->serialized_pb_dict); upb_symtab_free(self->symtab); PyUpb_ObjCache_Delete(self->symtab); PyUpb_Dealloc(self); } -PyObject* PyUpb_DescriptorPool_GetSerializedPb(PyObject* _self, - const char* filename) { - PyUpb_DescriptorPool* self = (PyUpb_DescriptorPool*)_self; - return PyDict_GetItemString(self->serialized_pb_dict, filename); -} - /* * DescriptorPool.__new__() * @@ -166,9 +157,6 @@ static PyObject* PyUpb_DescriptorPool_AddSerializedFile( goto done; } - PyDict_SetItemString(self->serialized_pb_dict, upb_filedef_name(filedef), - serialized_pb); - result = PyUpb_FileDescriptor_Get(filedef); done: diff --git a/python/descriptor_pool.h b/python/descriptor_pool.h index f43764ecc2..caded66a42 100644 --- a/python/descriptor_pool.h +++ b/python/descriptor_pool.h @@ -32,8 +32,11 @@ #include "protobuf.h" +// Returns a serialized protocol buffer for the given filename, which much have +// been previously loaded into this DescriptorPool. PyObject* PyUpb_DescriptorPool_GetSerializedPb(PyObject* _self, const char* filename); + PyObject* PyUpb_DescriptorPool_Get(const upb_symtab* symtab); upb_symtab* PyUpb_DescriptorPool_GetSymtab(PyObject* pool); PyObject* PyUpb_DescriptorPool_GetDefaultPool(void); diff --git a/python/minimal_test.py b/python/minimal_test.py index 0c73a4b1a8..26d6553c84 100644 --- a/python/minimal_test.py +++ b/python/minimal_test.py @@ -31,6 +31,10 @@ import unittest from google.protobuf.pyext import _message from google.protobuf.internal import api_implementation from google.protobuf import unittest_pb2 +from google.protobuf import descriptor_pool +from google.protobuf.internal import factory_test1_pb2 +from google.protobuf.internal import factory_test2_pb2 +from google.protobuf import descriptor_pb2 class TestMessageExtension(unittest.TestCase): diff --git a/python/pb_unit_tests/descriptor_database_test_wrapper.py b/python/pb_unit_tests/descriptor_database_test_wrapper.py index 69fe7de269..125533eae6 100644 --- a/python/pb_unit_tests/descriptor_database_test_wrapper.py +++ b/python/pb_unit_tests/descriptor_database_test_wrapper.py @@ -26,8 +26,5 @@ from google.protobuf.internal import descriptor_database_test import unittest -descriptor_database_test.DescriptorDatabaseTest.testAdd.__unittest_expecting_failure__ = True -descriptor_database_test.DescriptorDatabaseTest.testConflictRegister.__unittest_expecting_failure__ = True - if __name__ == '__main__': unittest.main(module=descriptor_database_test, verbosity=2) diff --git a/python/pb_unit_tests/descriptor_pool_test_wrapper.py b/python/pb_unit_tests/descriptor_pool_test_wrapper.py index 2eadd9cb60..1314f1b5fa 100644 --- a/python/pb_unit_tests/descriptor_pool_test_wrapper.py +++ b/python/pb_unit_tests/descriptor_pool_test_wrapper.py @@ -25,48 +25,47 @@ from google.protobuf.internal import descriptor_pool_test import unittest +import copy print(unittest) descriptor_pool_test.AddDescriptorTest.testAddTypeError.__unittest_expecting_failure__ = True -descriptor_pool_test.AddDescriptorTest.testEmptyDescriptorPool.__unittest_expecting_failure__ = True descriptor_pool_test.AddDescriptorTest.testEnum.__unittest_expecting_failure__ = True descriptor_pool_test.AddDescriptorTest.testFile.__unittest_expecting_failure__ = True descriptor_pool_test.AddDescriptorTest.testFileDescriptorOptionsWithCustomDescriptorPool.__unittest_expecting_failure__ = True descriptor_pool_test.AddDescriptorTest.testMessage.__unittest_expecting_failure__ = True descriptor_pool_test.AddDescriptorTest.testService.__unittest_expecting_failure__ = True +descriptor_pool_test.CreateDescriptorPoolTest.testFindFieldByName.__unittest_expecting_failure__ = True +descriptor_pool_test.CreateDescriptorPoolTest.testFindMessageTypeByName.__unittest_expecting_failure__ = True +descriptor_pool_test.CreateDescriptorPoolTest.testFindService.__unittest_expecting_failure__ = True +descriptor_pool_test.CreateDescriptorPoolTest.testFindTypeErrors.__unittest_expecting_failure__ = True +descriptor_pool_test.CreateDescriptorPoolTest.testUserDefinedDB.__unittest_expecting_failure__ = True +descriptor_pool_test.SecondaryDescriptorFromDescriptorDB.testErrorCollector.__unittest_expecting_failure__ = True -# We must skip these tests entirely (rather than running them with -# __unittest_expecting_failure__) because they error out in setUp(): -# -# AttributeError: 'google.protobuf.pyext._message.FileDescriptor' object has no attribute 'serialized_pb' -# -# TODO: change to __unittest_expecting_failure__ when serialized_pb is available. -descriptor_pool_test.CreateDescriptorPoolTest.testAddSerializedFile.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testComplexNesting.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testConflictRegister.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testDefaultValueForCustomMessages.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testEnumDefaultValue.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testExtensionsAreNotFields.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindAllExtensions.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindEnumTypeByName.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindEnumTypeByNameFailure.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindExtensionByName.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindExtensionByNumber.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindFieldByName.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindFileByName.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindFileByNameFailure.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindFileContainingSymbol.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindFileContainingSymbolFailure.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindMessageTypeByName.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindMessageTypeByNameFailure.__unittest_skip__ = True -descriptor_pool_test.DefaultDescriptorPoolTest.testFindMethods.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindOneofByName.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindService.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testFindTypeErrors.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testUserDefinedDB.__unittest_skip__ = True -descriptor_pool_test.CreateDescriptorPoolTest.testAddFileDescriptor.__unittest_skip__ = True -descriptor_pool_test.SecondaryDescriptorFromDescriptorDB.testErrorCollector.__unittest_skip__ = True +# Some tests are defined in a base class and inherited by multiple sub-classes. +# If only some of the subclasses fail, we need to duplicate the test method +# before marking it, otherwise the mark will affect all subclasses. +def wrap(cls, method): + existing = getattr(cls, method) + setattr(cls, method, lambda self: existing(self)) + getattr(cls, method).__unittest_expecting_failure__ = True + +wrap(descriptor_pool_test.CreateDescriptorPoolTest, "testAddFileDescriptor") +wrap(descriptor_pool_test.CreateDescriptorPoolTest, "testAddSerializedFile") +wrap(descriptor_pool_test.CreateDescriptorPoolTest, "testComplexNesting") +wrap(descriptor_pool_test.CreateDescriptorPoolTest, "testExtensionsAreNotFields") +wrap(descriptor_pool_test.DefaultDescriptorPoolTest, "testAddFileDescriptor") +wrap(descriptor_pool_test.DefaultDescriptorPoolTest, "testAddSerializedFile") +wrap(descriptor_pool_test.DefaultDescriptorPoolTest, "testComplexNesting") +wrap(descriptor_pool_test.DefaultDescriptorPoolTest, "testEnumDefaultValue") +wrap(descriptor_pool_test.DefaultDescriptorPoolTest, "testExtensionsAreNotFields") +wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindAllExtensions") +wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindEnumTypeByName") +wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindExtensionByName") +wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindExtensionByNumber") +wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindFileByName") +wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindFileContainingSymbol") +wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindOneofByName") if __name__ == '__main__': unittest.main(module=descriptor_pool_test, verbosity=2) diff --git a/python/pb_unit_tests/generator_test_wrapper.py b/python/pb_unit_tests/generator_test_wrapper.py index 3cb7caf528..ea78705494 100644 --- a/python/pb_unit_tests/generator_test_wrapper.py +++ b/python/pb_unit_tests/generator_test_wrapper.py @@ -28,7 +28,6 @@ import unittest generator_test.GeneratorTest.testBadIdentifiers.__unittest_expecting_failure__ = True generator_test.GeneratorTest.testExtensionScope.__unittest_expecting_failure__ = True -generator_test.GeneratorTest.testFileDescriptor.__unittest_expecting_failure__ = True generator_test.GeneratorTest.testMessageWithCustomOptions.__unittest_expecting_failure__ = True generator_test.GeneratorTest.testOneof.__unittest_expecting_failure__ = True generator_test.GeneratorTest.testOptions.__unittest_expecting_failure__ = True diff --git a/upb/util/def_to_proto.c b/upb/util/def_to_proto.c index fdde5cc062..e813f7a986 100644 --- a/upb/util/def_to_proto.c +++ b/upb/util/def_to_proto.c @@ -392,10 +392,14 @@ static google_protobuf_FileDescriptorProto *filedef_toproto( google_protobuf_FileDescriptorProto_set_name( proto, strviewdup(ctx, upb_filedef_name(f))); - size_t n = strlen(upb_filedef_package(f)); - if (n) { - google_protobuf_FileDescriptorProto_set_package( - proto, strviewdup(ctx, upb_filedef_package(f))); + const char* package = upb_filedef_package(f); + size_t n; + if (package) { + n = strlen(package); + if (n) { + google_protobuf_FileDescriptorProto_set_package( + proto, strviewdup(ctx, upb_filedef_package(f))); + } } if (upb_filedef_syntax(f) == UPB_SYNTAX_PROTO3) {