feat: enable roundtrip testing of textplan conversion (substrait-io#81)

EpsilonPrime · Aug 24, 2023 · 362552d · 362552d
1 parent f81a527
commit 362552d
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 51 deletions.
diff --git a/src/substrait/textplan/tests/CMakeLists.txt b/src/substrait/textplan/tests/CMakeLists.txt
@@ -19,50 +19,38 @@ add_test_case(
   gtest
   gtest_main)
 
-option(SUBSTRAIT_CPP_ROUNDTRIP_TESTING
-       "Enable substrait-cpp textplan roundtrip tests." OFF)
-
-if(${SUBSTRAIT_CPP_ROUNDTRIP_TESTING})
-  add_test_case(
-    round_trip_test
-    SOURCES
-    RoundtripTest.cpp
-    EXTRA_LINK_LIBS
-    substrait_textplan_converter
-    substrait_textplan_loader
-    substrait_textplan_normalizer
-    substrait_common
-    substrait_proto
-    parse_result_matchers
-    protobuf-matchers
-    fmt::fmt-header-only
-    gmock
-    gtest
-    gtest_main)
-
-  cmake_path(GET CMAKE_CURRENT_SOURCE_DIR PARENT_PATH TEXTPLAN_SOURCE_DIR)
-
-  add_custom_command(
-    TARGET round_trip_test
-    POST_BUILD
-    COMMAND ${CMAKE_COMMAND} -E echo "Copying unit test data.."
-    COMMAND ${CMAKE_COMMAND} -E make_directory
-            "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data"
-    COMMAND
-      ${CMAKE_COMMAND} -E copy
-      "${TEXTPLAN_SOURCE_DIR}/converter/data/q6_first_stage.json"
-      "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data/q6_first_stage.json"
-    COMMAND ${CMAKE_COMMAND} -E copy "${TEXTPLAN_SOURCE_DIR}/data/*.json"
-            "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data/")
-
-  message(
-    STATUS
-      "test data will be here:  ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data")
-else()
-
-  message(
-    STATUS
-      "Round trip testing is turned off.  Add SUBSTRAIT_CPP_ROUNDTRIP_TESTING=on to enable."
-  )
+add_test_case(
+  substrait_textplan_round_trip_test
+  SOURCES
+  RoundtripTest.cpp
+  EXTRA_LINK_LIBS
+  substrait_textplan_converter
+  substrait_textplan_loader
+  substrait_textplan_normalizer
+  substrait_common
+  substrait_proto
+  parse_result_matchers
+  protobuf-matchers
+  fmt::fmt-header-only
+  gmock
+  gtest
+  gtest_main)
 
-endif()
+cmake_path(GET CMAKE_CURRENT_SOURCE_DIR PARENT_PATH TEXTPLAN_SOURCE_DIR)
+
+add_custom_command(
+  TARGET substrait_textplan_round_trip_test
+  POST_BUILD
+  COMMAND ${CMAKE_COMMAND} -E echo "Copying unit test data.."
+  COMMAND ${CMAKE_COMMAND} -E make_directory
+          "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data"
+  COMMAND
+    ${CMAKE_COMMAND} -E copy
+    "${TEXTPLAN_SOURCE_DIR}/converter/data/q6_first_stage.json"
+    "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data/q6_first_stage.json"
+  COMMAND ${CMAKE_COMMAND} -E copy "${TEXTPLAN_SOURCE_DIR}/data/*.json"
+          "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data/")
+
+message(
+  STATUS "test data will be here:  ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data"
+)
diff --git a/src/substrait/textplan/tests/RoundtripTest.cpp b/src/substrait/textplan/tests/RoundtripTest.cpp
@@ -19,8 +19,7 @@
 #include "substrait/textplan/tests/ParseResultMatchers.h"
 
 using ::protobuf_matchers::EqualsProto;
-using ::protobuf_matchers::IgnoringFieldPaths;
-using ::protobuf_matchers::Partially;
+using ::protobuf_matchers::IgnoringFields;
 using ::testing::AllOf;
 
 namespace io::substrait::textplan {
@@ -98,7 +97,11 @@ TEST_P(RoundTripBinaryToTextFixture, RoundTrip) {
   ASSERT_THAT(
       result,
       ::testing::AllOf(
-          ParsesOk(), HasErrors({}), AsBinaryPlan(EqualsProto(normalizedPlan))))
+          ParsesOk(),
+          HasErrors({}),
+          AsBinaryPlan(IgnoringFields(
+              {"substrait.proto.RelCommon.Emit.output_mapping"},
+              EqualsProto(normalizedPlan)))))
       << std::endl
       << "Intermediate result:" << std::endl
       << addLineNumbers(outputText) << std::endl
@@ -115,8 +118,7 @@ INSTANTIATE_TEST_SUITE_P(
       if (lastSlash != std::string::npos) {
         identifier = identifier.substr(lastSlash);
       }
-      if (identifier.length() > 5 &&
-          identifier.substr(identifier.length() - 5) == ".json") {
+      if (endsWith(identifier, ".json")) {
         identifier = identifier.substr(0, identifier.length() - 5);
       }