Remove descriptor_pool_test_wrapper for python's descriptor_pool_test.

The test wrappers were another way to document nonconformant behaviour between
different python backends. We can achieve the same by removing the wrapper
script and adding an if-condition in the test itself based on
api_implementation.Type(). Since we already do that for nonconformance between
pure Python vs. C++ backends, this change makes it easier to look for UPB
nonconformance instead of going through another layer of indirection.

Temporarily, we will need to hardcode the migrated test name in test_upb.yml
because not all tests under google.protobuf.internal support UPB yet.
(UPB testing for selected tests are added in 21e9aa6).

PiperOrigin-RevId: 711837521

.github/workflows/test_upb.yml

@@ -278,7 +278,7 @@ jobs:
         # We will eventually make this into a wildcard rule once all tests
         # have been migrated to be compatible with upb.
         run: |
-          TESTS=(message_test message_factory_test descriptor_test proto_builder_test)
+          TESTS=(message_test message_factory_test descriptor_test proto_builder_test descriptor_pool_test)
           for test in ${TESTS[@]}; do
             python -m unittest -v google.protobuf.internal.${test}
           done

python/google/protobuf/internal/descriptor_pool_test.py

@@ -718,6 +718,87 @@ def setUp(self):
         no_package_pb2.DESCRIPTOR.serialized_pb))
     self.pool = descriptor_pool.DescriptorPool(descriptor_db=self.db)
 
+  # TODO: b/387527786 - This is a helper function for testErrorCollector to
+  # capture some of the nonconformant, C++-specific behavior.
+  def assertCppErrorCollectorCorrect(self):
+    self.assertEqual(api_implementation.Type(), 'cpp')
+    error_msg = (
+        'Invalid proto descriptor for file "error_file":\\n  '
+        'collector.ErrorMessage.nested_message_field: "SubMessage" '
+        'is not defined.\\n  collector.ErrorMessage.MyOneof: Oneof '
+        "must have at least one field.\\n'"
+    )
+    with self.assertRaises(KeyError) as exc:
+      self.pool.FindMessageTypeByName('collector.ErrorMessage')
+    self.assertEqual(
+        str(exc.exception),
+        "'Couldn\\'t build file for message collector.ErrorMessage\\n"
+        + error_msg,
+    )
+
+    with self.assertRaises(KeyError) as exc:
+      self.pool.FindFieldByName('collector.ErrorMessage.nested_message_field')
+    self.assertEqual(
+        str(exc.exception),
+        "'Couldn\\'t build file for field"
+        ' collector.ErrorMessage.nested_message_field\\n'
+        + error_msg,
+    )
+
+    with self.assertRaises(KeyError) as exc:
+      self.pool.FindEnumTypeByName('collector.MyEnum')
+    self.assertEqual(
+        str(exc.exception),
+        "'Couldn\\'t build file for enum collector.MyEnum\\n" + error_msg,
+    )
+
+    with self.assertRaises(KeyError) as exc:
+      self.pool.FindFileContainingSymbol('collector.MyEnumValue')
+    self.assertEqual(
+        str(exc.exception),
+        "'Couldn\\'t build file for symbol collector.MyEnumValue\\n"
+        + error_msg,
+    )
+
+    with self.assertRaises(KeyError) as exc:
+      self.pool.FindOneofByName('collector.ErrorMessage.MyOneof')
+    self.assertEqual(
+        str(exc.exception),
+        "'Couldn\\'t build file for oneof collector.ErrorMessage.MyOneof\\n"
+        + error_msg,
+    )
+
+  # TODO: b/387527786 - This is a helper function for testErrorCollector to
+  # capture some of the nonconformant, UPB-specific behavior.
+  def assertUpbErrorCollectorCorrect(self):
+    self.assertEqual(api_implementation.Type(), 'upb')
+    # Nonconformance: compared with C++, UPB will have less descriptive
+    # error messages, and raise TypeError instead of KeyError.
+    error_msg = (
+        "Couldn't build proto file into descriptor pool: "
+        "couldn't resolve name 'SubMessage'"
+    )
+
+    with self.assertRaises(TypeError) as exc:
+      self.pool.FindMessageTypeByName('collector.ErrorMessage')
+    self.assertEqual(str(exc.exception), error_msg)
+
+    with self.assertRaises(TypeError) as exc:
+      self.pool.FindFieldByName('collector.ErrorMessage.nested_message_field')
+    self.assertEqual(str(exc.exception), error_msg)
+
+    with self.assertRaises(TypeError) as exc:
+      self.pool.FindEnumTypeByName('collector.MyEnum')
+    self.assertEqual(str(exc.exception), error_msg)
+
+    with self.assertRaises(TypeError) as exc:
+      self.pool.FindFileContainingSymbol('collector.MyEnumValue')
+    self.assertEqual(str(exc.exception), error_msg)
+
+    with self.assertRaises(TypeError) as exc:
+      self.pool.FindOneofByName('collector.ErrorMessage.MyOneof')
+    self.assertEqual(str(exc.exception), error_msg)
+
   def testErrorCollector(self):
     file_proto = descriptor_pb2.FileDescriptorProto()
     file_proto.package = 'collector'
@@ -739,11 +820,20 @@ def testErrorCollector(self):
     enum_value.number = 0
     self.db.Add(file_proto)
 
-    self.assertRaisesRegex(KeyError, 'SubMessage',
-                           self.pool.FindMessageTypeByName,
-                           'collector.ErrorMessage')
-    self.assertRaisesRegex(KeyError, 'SubMessage', self.pool.FindFileByName,
-                           'error_file')
+    # Nonconformance: UPB will raise a TypeError whereas other implementations
+    # will raise KeyError when SubMessage cannot be indexed.
+    # TODO: b/387527786 - Fix this nonconformance between (cpp+python)/upb.
+    error_type = TypeError if api_implementation.Type() == 'upb' else KeyError
+    self.assertRaisesRegex(
+        error_type,
+        'SubMessage',
+        self.pool.FindMessageTypeByName,
+        'collector.ErrorMessage',
+    )
+    self.assertRaisesRegex(
+        error_type, 'SubMessage', self.pool.FindFileByName, 'error_file'
+    )
+
     with self.assertRaises(KeyError) as exc:
       self.pool.FindFileByName('none_file')
     self.assertIn(str(exc.exception), ('\'none_file\'',
@@ -755,36 +845,12 @@ def testErrorCollector(self):
     # called the first time, a KeyError will be raised but call the find
     # method later will return a descriptor which is not build.
     # TODO: fix pure python to revert the load if file can not be build
+    # TODO: b/387527786 - Fix this nonconformance between python/cpp/upb.
     if api_implementation.Type() != 'python':
-      error_msg = ('Invalid proto descriptor for file "error_file":\\n  '
-                   'collector.ErrorMessage.nested_message_field: "SubMessage" '
-                   'is not defined.\\n  collector.ErrorMessage.MyOneof: Oneof '
-                   'must have at least one field.\\n\'')
-      with self.assertRaises(KeyError) as exc:
-        self.pool.FindMessageTypeByName('collector.ErrorMessage')
-      self.assertEqual(str(exc.exception), '\'Couldn\\\'t build file for '
-                       'message collector.ErrorMessage\\n' + error_msg)
-
-      with self.assertRaises(KeyError) as exc:
-        self.pool.FindFieldByName('collector.ErrorMessage.nested_message_field')
-      self.assertEqual(str(exc.exception), '\'Couldn\\\'t build file for field'
-                       ' collector.ErrorMessage.nested_message_field\\n'
-                       + error_msg)
-
-      with self.assertRaises(KeyError) as exc:
-        self.pool.FindEnumTypeByName('collector.MyEnum')
-      self.assertEqual(str(exc.exception), '\'Couldn\\\'t build file for enum'
-                       ' collector.MyEnum\\n' + error_msg)
-
-      with self.assertRaises(KeyError) as exc:
-        self.pool.FindFileContainingSymbol('collector.MyEnumValue')
-      self.assertEqual(str(exc.exception), '\'Couldn\\\'t build file for symbol'
-                       ' collector.MyEnumValue\\n' + error_msg)
-
-      with self.assertRaises(KeyError) as exc:
-        self.pool.FindOneofByName('collector.ErrorMessage.MyOneof')
-      self.assertEqual(str(exc.exception), '\'Couldn\\\'t build file for oneof'
-                       ' collector.ErrorMessage.MyOneof\\n' + error_msg)
+      if api_implementation.Type() == 'cpp':
+        self.assertCppErrorCollectorCorrect()
+      else:
+        self.assertUpbErrorCollectorCorrect()
 
 
 class ProtoFile(object):

python/pb_unit_tests/BUILD

@@ -11,8 +11,6 @@ package(default_applicable_licenses = ["//:license"])
 
 licenses(["notice"])
 
-pyproto_test_wrapper(name = "descriptor_pool_test")
-
 pyproto_test_wrapper(name = "generator_test")
 
 pyproto_test_wrapper(name = "reflection_test")