All tests passing!

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,

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:

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);

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):
 

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)

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)

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

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) {