From fe81373ffe7e648a5cecef16c8cb72dac38fcfa6 Mon Sep 17 00:00:00 2001
From: Alessio Quaglino <quaglino@google.com>
Date: Sat, 31 May 2025 00:55:32 -0700
Subject: [PATCH] Enable WriteReadCompare for models with assets.

PiperOrigin-RevId: 765500742
Change-Id: I119dbea11ed6771839b5e81d04070e95a51d3ed0
---
 test/xml/xml_native_writer_test.cc | 76 +++++++++++++++++-------------
 1 file changed, 44 insertions(+), 32 deletions(-)

diff --git a/test/xml/xml_native_writer_test.cc b/test/xml/xml_native_writer_test.cc
index fd87a86d..2ad13bc7 100644
--- a/test/xml/xml_native_writer_test.cc
+++ b/test/xml/xml_native_writer_test.cc
@@ -1381,19 +1381,30 @@ TEST_F(XMLWriterTest, WriteReadCompare) {
         std::string xml = p.path().string();
 
         // if file is meant to fail, skip it
-        if (absl::StrContains(p.path().string(), "100_humanoids") ||
-            absl::StrContains(p.path().string(), "malformed_") ||
+        if (absl::StrContains(p.path().string(), "malformed_") ||
+            // exclude files that are too slow to load
             absl::StrContains(p.path().string(), "cow") ||
             absl::StrContains(p.path().string(), "gmsh_") ||
             absl::StrContains(p.path().string(), "shark_") ||
-            absl::StrContains(p.path().string(), "frameless_contact_hfield") ||
-            absl::StrContains(p.path().string(), "spheremesh")) {
+            absl::StrContains(p.path().string(), "spheremesh") ||
+            // exclude files that fail the comparison test
+            absl::StrContains(p.path().string(), "usd") ||
+            absl::StrContains(p.path().string(), "torus_maxhull") ||
+            absl::StrContains(p.path().string(), "fitmesh_") ||
+            absl::StrContains(p.path().string(), "lengthrange") ||
+            absl::StrContains(p.path().string(), "hfield_xml") ||
+            absl::StrContains(p.path().string(), "fromto_convex") ||
+            absl::StrContains(p.path().string(), "cube_skin") ||
+            absl::StrContains(p.path().string(), "cube_3x3x3")) {
           continue;
         }
         // load model
         std::array<char, 1000> error;
-        mjModel* m = mj_loadXML(
-            xml.c_str(), nullptr, error.data(), error.size());
+        mjSpec* s =
+            mj_parseXML(xml.c_str(), nullptr, error.data(), error.size());
+        ASSERT_THAT(s, NotNull())
+            << "Failed to load " << xml.c_str() << ": " << error.data();
+        mjModel* m = mj_compile(s, nullptr);
         ASSERT_THAT(m, NotNull())
             << "Failed to load " << xml.c_str() << ": " << error.data();
 
@@ -1402,32 +1413,33 @@ TEST_F(XMLWriterTest, WriteReadCompare) {
         ASSERT_THAT(d, testing::NotNull()) << "Failed to create data\n";
 
         // save and load back
-        mjModel* mtemp =
-            LoadModelFromString(SaveAndReadXml(m), error.data(), error.size());
+        auto abs_path = p.path();
+        mjSpec* stemp = mj_parseXMLString(SaveAndReadXml(s).c_str(), 0,
+                                          error.data(), error.size());
+        mjs_setString(stemp->modelfiledir,
+                      abs_path.remove_filename().string().c_str());
+        mjModel* mtemp = mj_compile(stemp, nullptr);
 
-        if (!mtemp) {
-          // if failing because assets are missing, accept the test
-          ASSERT_THAT(error.data(), HasSubstr("file"))
-              << error.data() << " from " << xml.c_str();
-        } else {
-          mjtNum tol = 0;
+        ASSERT_THAT(mtemp, NotNull())
+            << error.data() << " from " << xml.c_str();
 
-          // for particularly sensitive models, relax the tolerance
-          if (absl::StrContains(p.path().string(), "belt.xml") ||
-              absl::StrContains(p.path().string(), "cable.xml")) {
-            tol = 1e-13;
-          }
+        mjtNum tol = 0;
 
-          // compare and delete
-          std::string field = "";
-          mjtNum result = CompareModel(m, mtemp, field);
-          EXPECT_LE(result, tol)
-              << "Loaded and saved models are different!\n"
-              << "Affected file " << p.path().string() << '\n'
-              << "Different field: " << field << '\n';
-          mj_deleteModel(mtemp);
+        // for particularly sensitive models, relax the tolerance
+        if (absl::StrContains(p.path().string(), "belt.xml") ||
+            absl::StrContains(p.path().string(), "cable.xml")) {
+          tol = 1e-13;
         }
 
+        // compare and delete
+        std::string field = "";
+        mjtNum result = CompareModel(m, mtemp, field);
+        EXPECT_LE(result, tol)
+            << "Loaded and saved models are different!\n"
+            << "Affected file " << p.path().string() << '\n'
+            << "Different field: " << field << '\n';
+        mj_deleteModel(mtemp);
+
         // check for stack memory leak
         mj_step(m, d);
         EXPECT_EQ(d->pstack, 0) << "mjData stack memory leak detected in " <<
@@ -1452,21 +1464,21 @@ TEST_F(XMLWriterTest, WriteReadCompare) {
         ASSERT_THAT(mtemp, NotNull());
 
         // compare with 0 tolerance
-        std::string field = "";
-        mjtNum result = CompareModel(m, mtemp, field);
+        field = "";
+        result = CompareModel(m, mtemp, field);
         EXPECT_EQ(result, 0)
             << "Loaded and saved binary models are different!\n"
             << "Affected file " << p.path().string() << '\n'
             << "Different field: " << field << '\n';
 
         // clean up
+        mj_deleteSpec(s);
+        mj_deleteSpec(stemp);
+        mj_deleteModel(m);
         mj_deleteModel(mtemp);
         mj_deleteVFS(vfs);
         mju_free(vfs);
         mju_free(buffer);
-
-        // delete model
-        mj_deleteModel(m);
       }
     }
   }