[parsing] Delete FormatFirstError; fix call sites (#22418)

Author: rpoyner-tri

common/test_utilities/diagnostic_policy_test_base.h

@@ -104,27 +104,6 @@ class DiagnosticPolicyTestBase : public ::testing::Test {
         });
   }
 
-  // Returns the first error as a string (or else fails the test case,
-  // if there were no errors).
-  std::string FormatFirstError() {
-    ScopedTrace trace;
-    if (error_records_.empty()) {
-      auto format_warnings = [this]() {
-        std::deque<std::string> warnings;
-        for (const auto& record : warning_records_) {
-          warnings.push_back(record.FormatWarning());
-        }
-        return warnings;
-      };
-      ADD_FAILURE() << fmt::format(
-          "FormatFirstError did not get any errors\n"
-          "found {} warnings:\n{}",
-          NumWarnings(), fmt::join(format_warnings(), "\n"));
-      return {};
-    }
-    return error_records_[0].FormatError();
-  }
-
   template <typename T>
   T Take(std::deque<T>* c) {
     if (c->empty()) {

multibody/parsing/test/detail_sdf_geometry_test.cc

@@ -268,9 +268,8 @@ TEST_F(SceneGraphParserDetail, CheckInvalidDrakeCapsules) {
   unique_ptr<Shape> shape_no_radius =
       MakeShapeFromSdfGeometry(*no_radius_geometry);
   EXPECT_EQ(shape_no_radius, nullptr);
-  EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+  EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
       ".*Element <radius> is required within element <drake:capsule>."));
-  ClearDiagnostics();
 
   unique_ptr<sdf::Geometry> no_length_geometry = MakeSdfGeometryFromString(
       "<drake:capsule>"
@@ -279,9 +278,8 @@ TEST_F(SceneGraphParserDetail, CheckInvalidDrakeCapsules) {
   unique_ptr<Shape> shape_no_length =
       MakeShapeFromSdfGeometry(*no_length_geometry);
   EXPECT_EQ(shape_no_length, nullptr);
-  EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+  EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
       ".*Element <length> is required within element <drake:capsule>."));
-  ClearDiagnostics();
 }
 
 // Verify MakeShapeFromSdfGeometry can make a capsule from an sdf::Geometry.
@@ -342,9 +340,8 @@ TEST_F(SceneGraphParserDetail, CheckInvalidEllipsoids) {
       "</drake:ellipsoid>");
   unique_ptr<Shape> shape_no_a = MakeShapeFromSdfGeometry(*no_a_geometry);
   EXPECT_EQ(shape_no_a, nullptr);
-  EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+  EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
       ".*Element <a> is required within element <drake:ellipsoid>."));
-  ClearDiagnostics();
 
   unique_ptr<sdf::Geometry> no_b_geometry = MakeSdfGeometryFromString(
       "<drake:ellipsoid>"
@@ -353,9 +350,8 @@ TEST_F(SceneGraphParserDetail, CheckInvalidEllipsoids) {
       "</drake:ellipsoid>");
   unique_ptr<Shape> shape_no_b = MakeShapeFromSdfGeometry(*no_b_geometry);
   EXPECT_EQ(shape_no_b, nullptr);
-  EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+  EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
       ".*Element <b> is required within element <drake:ellipsoid>."));
-  ClearDiagnostics();
 
   unique_ptr<sdf::Geometry> no_c_geometry = MakeSdfGeometryFromString(
       "<drake:ellipsoid>"
@@ -364,9 +360,8 @@ TEST_F(SceneGraphParserDetail, CheckInvalidEllipsoids) {
       "</drake:ellipsoid>");
   unique_ptr<Shape> shape_no_c = MakeShapeFromSdfGeometry(*no_c_geometry);
   EXPECT_EQ(shape_no_c, nullptr);
-  EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+  EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
       ".*Element <c> is required within element <drake:ellipsoid>."));
-  ClearDiagnostics();
 }
 
 
@@ -441,10 +436,9 @@ TEST_F(SceneGraphParserDetail, MakeMeshFromSdfGeometryIsotropicError) {
       "</mesh>");
   unique_ptr<Shape> shape = MakeShapeFromSdfGeometry(*sdf_geometry);
   EXPECT_EQ(shape, nullptr);
-  EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+  EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
       ".*Drake meshes only support isotropic scaling. Therefore"
       " all three scaling factors must be exactly equal."));
-  ClearDiagnostics();
 }
 
 // Verify MakeShapeFromSdfGeometry can make a convex mesh from an sdf::Geometry.
@@ -1071,9 +1065,8 @@ TEST_F(SceneGraphParserDetail, ParseVisualMaterial) {
     internal::MakeVisualPropertiesFromSdfVisual(sdf_diagnostic_,
         *sdf_visual, [](const SDFormatDiagnostic&, std::string filename)
             -> std::string {return {};});
-    EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+    EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
         ".*Unable to locate the texture file: empty.png"));
-    ClearDiagnostics();
   }
 
   // Case: drake:{illustration|perception}_properties is missing the enabled
@@ -1244,7 +1237,7 @@ TEST_F(SceneGraphParserDetail, AcceptingRenderers) {
           "  <drake:accepting_renderer> </drake:accepting_renderer>"
           "</visual>");
     MakeVisualPropertiesFromSdfVisual(*sdf_visual);
-    EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+    EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
         ".*<drake:accepting_renderer> tag given without any name"));
   }
 
@@ -1260,8 +1253,6 @@ TEST_F(SceneGraphParserDetail, AcceptingRenderers) {
     EXPECT_THAT(TakeWarning(), ::testing::MatchesRegex(
         ".*<drake:accepting_renderer> specified .* disabled perception role."));
   }
-
-  ClearDiagnostics();
 }
 
 // Verify that the <drake:*_properties> are accounted for in disabling sets of
@@ -1548,11 +1539,10 @@ TEST_F(SceneGraphParserDetail, MakeProximityPropertiesForCollision) {
     std::optional<ProximityProperties> properties =
         MakeProximityPropertiesForCollision(sdf_diagnostic_, *sdf_collision);
     EXPECT_FALSE(properties.has_value());
-    EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+    EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
         ".*A <collision> geometry has defined the unsupported tag "
         "<drake:soft_hydroelastic>. Please change it to "
         "<drake:compliant_hydroelastic>."));
-    ClearDiagnostics();
   }
 
   // Case: specifies both -- should be an error.
@@ -1565,10 +1555,9 @@ TEST_F(SceneGraphParserDetail, MakeProximityPropertiesForCollision) {
     std::optional<ProximityProperties> properties =
         MakeProximityPropertiesForCollision(sdf_diagnostic_, *sdf_collision);
     EXPECT_FALSE(properties.has_value());
-    EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+    EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
         ".*A <collision> geometry has defined mutually-exclusive tags "
         ".*rigid.* and .*compliant.*"));
-    ClearDiagnostics();
   }
 
   // Case: has no drake coefficients, only mu & m2 in ode: contains mu, mu2

multibody/parsing/test/detail_sdf_parser_test.cc

@@ -1997,6 +1997,8 @@ TEST_F(SdfParserTest, TestUnsupportedFrames) {
       R"(.*(attached_to|relative_to) name\[world\] specified by frame )"
       R"(with name\[.*\] does not match a nested model, link, joint, or )"
       R"(frame name in model with name\[bad\].*)"));
+  // Ignore additional errors for this test.
+  EXPECT_EQ(NumErrors(), 5);
   ClearDiagnostics();
 
   ParseTestString(R"""(
@@ -2011,6 +2013,8 @@ TEST_F(SdfParserTest, TestUnsupportedFrames) {
       R"(.*(attached_to|relative_to) name\[world\] specified by frame )"
       R"(with name\[.*\] does not match a nested model, link, joint, or )"
       R"(frame name in model with name\[bad\].*)"));
+  // Ignore additional errors for this test.
+  EXPECT_EQ(NumErrors(), 2);
   ClearDiagnostics();
 
   for (std::string bad_name : {"world", "__model__", "__anything__"}) {
@@ -2023,6 +2027,7 @@ TEST_F(SdfParserTest, TestUnsupportedFrames) {
 )""", bad_name));
     EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
         R"(.*The supplied frame name \[.*\] is reserved..*)"));
+    // Ignore additional errors for this test. The number ignored varies.
     ClearDiagnostics();
   }
 
@@ -2034,6 +2039,8 @@ TEST_F(SdfParserTest, TestUnsupportedFrames) {
   EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
       R"(.*Attribute //pose\[@relative_to\] of top level model )"
       R"(must be left empty.*)"));
+  // Ignore additional errors for this test.
+  EXPECT_EQ(NumErrors(), 2);
   ClearDiagnostics();
 
   ParseTestString(R"""(
@@ -3216,9 +3223,8 @@ TEST_F(SdfParserTest, ErrorsFromIncludedUrdf) {
     <name>arm</name>
  </include>
 </model>)""", "1.8");
-  EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+  EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
       ".*bad.urdf.*XML_ERROR.*"));
-  ClearDiagnostics();
 }
 
 // TODO(SeanCurtis-TRI) The logic testing for collision filter group parsing
@@ -3647,9 +3653,8 @@ TEST_F(SdfParserTest, TestSingleModelEnforcement) {
   resolver.Resolve(diagnostic_policy_);
   EXPECT_FALSE(result.has_value());
 
-  EXPECT_THAT(FormatFirstError(), ::testing::MatchesRegex(
+  EXPECT_THAT(TakeError(), ::testing::MatchesRegex(
       ".*Root object can only contain one model.*"));
-  ClearDiagnostics();
 }
 
 // Verify merge-include works with Interface API.