Remove usage of GTest Throws in dwrf::ColumnWriterTest.cpp (#11270)

Summary: Gtest feature Throws is available in GTest 1.11 while system default in Ubuntu 20.04 is GTest 1.10 Remove usage instead of enforcing a GTest minimum version. Pull Request resolved: #11270 Reviewed By: kgpai Differential Revision: D65064485 Pulled By: pedroerp fbshipit-source-id: 0907dd219ece06d5773824045d952005cf56935a
facebookincubator · Oct 28, 2024 · 2e381e4 · 2e381e4
1 parent 05cc23f
commit 2e381e4
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 71 deletions.
diff --git a/velox/dwio/common/tests/OnDemandUnitLoaderTests.cpp b/velox/dwio/common/tests/OnDemandUnitLoaderTests.cpp
@@ -17,6 +17,7 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include "velox/common/base/tests/GTestUtils.h"
 #include "velox/dwio/common/OnDemandUnitLoader.h"
 #include "velox/dwio/common/UnitLoaderTools.h"
 #include "velox/dwio/common/tests/utils/UnitLoaderTestTools.h"
@@ -137,11 +138,9 @@ TEST(OnDemandUnitLoaderTests, SeekOutOfRangeReaderError) {
 
   readerMock.seek(60);
 
-  EXPECT_THAT(
-      [&]() { readerMock.seek(61); },
-      Throws<facebook::velox::VeloxRuntimeError>(Property(
-          &facebook::velox::VeloxRuntimeError::message,
-          HasSubstr("Can't seek to possition 61 in file. Must be up to 60."))));
+  VELOX_ASSERT_THROW(
+      readerMock.seek(61),
+      "Can't seek to possition 61 in file. Must be up to 60.");
 }
 
 TEST(OnDemandUnitLoaderTests, SeekOutOfRange) {
@@ -154,11 +153,7 @@ TEST(OnDemandUnitLoaderTests, SeekOutOfRange) {
 
   unitLoader->onSeek(0, 10);
 
-  EXPECT_THAT(
-      [&]() { unitLoader->onSeek(0, 11); },
-      Throws<facebook::velox::VeloxRuntimeError>(Property(
-          &facebook::velox::VeloxRuntimeError::message,
-          HasSubstr("Row out of range"))));
+  VELOX_ASSERT_THROW(unitLoader->onSeek(0, 11), "Row out of range");
 }
 
 TEST(OnDemandUnitLoaderTests, UnitOutOfRange) {
@@ -169,11 +164,8 @@ TEST(OnDemandUnitLoaderTests, UnitOutOfRange) {
 
   auto unitLoader = factory.create(std::move(units), 0);
   unitLoader->getLoadedUnit(0);
-  EXPECT_THAT(
-      [&]() { unitLoader->getLoadedUnit(1); },
-      Throws<facebook::velox::VeloxRuntimeError>(Property(
-          &facebook::velox::VeloxRuntimeError::message,
-          HasSubstr("Unit out of range"))));
+
+  VELOX_ASSERT_THROW(unitLoader->getLoadedUnit(1), "Unit out of range");
 }
 
 TEST(OnDemandUnitLoaderTests, CanRequestUnitMultipleTimes) {
@@ -209,16 +201,12 @@ TEST(OnDemandUnitLoaderTests, InitialSkip) {
   EXPECT_NO_THROW(getFactoryWithSkip(31));
   EXPECT_NO_THROW(getFactoryWithSkip(59));
   EXPECT_NO_THROW(getFactoryWithSkip(60));
-  EXPECT_THAT(
-      [&]() { getFactoryWithSkip(61); },
-      Throws<facebook::velox::VeloxRuntimeError>(Property(
-          &facebook::velox::VeloxRuntimeError::message,
-          HasSubstr("Can only skip up to the past-the-end row of the file."))));
-  EXPECT_THAT(
-      [&]() { getFactoryWithSkip(100); },
-      Throws<facebook::velox::VeloxRuntimeError>(Property(
-          &facebook::velox::VeloxRuntimeError::message,
-          HasSubstr("Can only skip up to the past-the-end row of the file."))));
+  VELOX_ASSERT_THROW(
+      getFactoryWithSkip(61),
+      "Can only skip up to the past-the-end row of the file.");
+  VELOX_ASSERT_THROW(
+      getFactoryWithSkip(100),
+      "Can only skip up to the past-the-end row of the file.");
 }
 
 TEST(OnDemandUnitLoaderTests, NoUnitButSkip) {
@@ -228,9 +216,7 @@ TEST(OnDemandUnitLoaderTests, NoUnitButSkip) {
   EXPECT_NO_THROW(factory.create(std::move(units), 0));
 
   std::vector<std::unique_ptr<LoadUnit>> units2;
-  EXPECT_THAT(
-      [&]() { factory.create(std::move(units2), 1); },
-      Throws<facebook::velox::VeloxRuntimeError>(Property(
-          &facebook::velox::VeloxRuntimeError::message,
-          HasSubstr("Can only skip up to the past-the-end row of the file."))));
+  VELOX_ASSERT_THROW(
+      factory.create(std::move(units2), 1),
+      "Can only skip up to the past-the-end row of the file.");
 }
diff --git a/velox/dwio/common/tests/UnitLoaderToolsTests.cpp b/velox/dwio/common/tests/UnitLoaderToolsTests.cpp
@@ -17,6 +17,7 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include "velox/common/base/tests/GTestUtils.h"
 #include "velox/dwio/common/UnitLoaderTools.h"
 
 using namespace ::testing;
@@ -158,26 +159,14 @@ TEST(UnitLoaderToolsTests, HowMuchToSkip) {
 
   // Test cases
   EXPECT_EQ(testSkip(0, {}), result(0, 0));
-  EXPECT_THAT(
-      [&]() { testSkip(1, {}); },
-      Throws<facebook::velox::VeloxRuntimeError>(Property(
-          &facebook::velox::VeloxRuntimeError::message,
-          HasSubstr(kErrorMessage))));
+  VELOX_ASSERT_THROW(testSkip(1, {}), kErrorMessage);
 
   EXPECT_EQ(testSkip(0, {0}), result(1, 0));
-  EXPECT_THAT(
-      [&]() { testSkip(1, {0}); },
-      Throws<facebook::velox::VeloxRuntimeError>(Property(
-          &facebook::velox::VeloxRuntimeError::message,
-          HasSubstr(kErrorMessage))));
+  VELOX_ASSERT_THROW(testSkip(1, {0}), kErrorMessage);
 
   EXPECT_EQ(testSkip(0, {1}), result(0, 0));
   EXPECT_EQ(testSkip(1, {1}), result(1, 0));
-  EXPECT_THAT(
-      [&]() { testSkip(2, {1}); },
-      Throws<facebook::velox::VeloxRuntimeError>(Property(
-          &facebook::velox::VeloxRuntimeError::message,
-          HasSubstr(kErrorMessage))));
+  VELOX_ASSERT_THROW(testSkip(2, {1}), kErrorMessage);
 
   std::vector<uint64_t> rowCount = {2, 1, 2};
   EXPECT_EQ(testSkip(0, rowCount), result(0, 0));
@@ -186,9 +175,5 @@ TEST(UnitLoaderToolsTests, HowMuchToSkip) {
   EXPECT_EQ(testSkip(3, rowCount), result(2, 0));
   EXPECT_EQ(testSkip(4, rowCount), result(2, 1));
   EXPECT_EQ(testSkip(5, rowCount), result(3, 0));
-  EXPECT_THAT(
-      [&]() { testSkip(6, rowCount); },
-      Throws<facebook::velox::VeloxRuntimeError>(Property(
-          &facebook::velox::VeloxRuntimeError::message,
-          HasSubstr(kErrorMessage))));
+  VELOX_ASSERT_THROW(testSkip(6, rowCount), kErrorMessage);
 }
diff --git a/velox/dwio/dwrf/test/ColumnWriterTest.cpp b/velox/dwio/dwrf/test/ColumnWriterTest.cpp
@@ -1267,12 +1267,9 @@ TEST_F(ColumnWriterTest, TestMapWriterDuplicatedInt64Key) {
   auto batch = b::create(
       *pool, {typename b::row{typename b::pair{5, 3}, typename b::pair{5, 4}}});
 
-  EXPECT_THAT(
-      ([&]() { testMapWriter<T, T>(*pool, batch, /* useFlatMap */ true); }),
-      Throws<
-          facebook::velox::dwio::common::exception::LoggedException>(Property(
-          &facebook::velox::dwio::common::exception::LoggedException::message,
-          HasSubstr("Duplicated key in map: 5"))));
+  VELOX_ASSERT_THROW(
+      (testMapWriter<T, T>(*pool, batch, /* useFlatMap */ true)),
+      "Duplicated key in map: 5");
 }
 
 TEST_F(ColumnWriterTest, TestMapWriterInt32Key) {
@@ -1320,14 +1317,9 @@ TEST_F(ColumnWriterTest, TestMapWriterDuplicatedStringKey) {
       *pool_,
       {b::row{b::pair{"2", "5"}, b::pair{"2", "8"}}}); // Duplicated key: 2
 
-  EXPECT_THAT(
-      ([&]() {
-        testMapWriter<keyType, valueType>(*pool_, batch, /* useFlatMap */ true);
-      }),
-      Throws<
-          facebook::velox::dwio::common::exception::LoggedException>(Property(
-          &facebook::velox::dwio::common::exception::LoggedException::message,
-          HasSubstr("Duplicated key in map: 2"))));
+  VELOX_ASSERT_THROW(
+      (testMapWriter<keyType, valueType>(*pool_, batch, /* useFlatMap */ true)),
+      "Duplicated key in map: 2");
 }
 
 TEST_F(ColumnWriterTest, TestMapWriterDifferentNumericKeyValue) {

diff --git a/velox/dwio/dwrf/test/StripeReaderBaseTests.cpp b/velox/dwio/dwrf/test/StripeReaderBaseTests.cpp
@@ -126,11 +126,12 @@ TEST_F(StripeLoadKeysTest, ThirdStripeHasKey) {
 }
 
 TEST_F(StripeLoadKeysTest, KeyMismatch) {
-  EXPECT_THAT(
-      [&]() { runTest(3); },
-      Throws<facebook::velox::dwio::common::exception::LoggedException>(
-          Property(
-              &facebook::velox::dwio::common::exception::LoggedException::
-                  failingExpression,
-              HasSubstr("keys.size() == providers_.size()"))));
+  try {
+    static_cast<void>(runTest(3));
+    FAIL() << "Expected an exception";
+  } catch (const facebook::velox::VeloxException& e) {
+    ASSERT_TRUE(
+        e.failingExpression().find("keys.size() == providers_.size()") !=
+        std::string::npos);
+  }
 }