From 9c2e0ca9df4727e2c2a4aacd55aa596377027a03 Mon Sep 17 00:00:00 2001 From: yingsu00 Date: Tue, 19 Nov 2024 16:28:47 -0800 Subject: [PATCH] refactor: Extract OutputMatcher class in common --- velox/common/testutil/CMakeLists.txt | 4 +- velox/common/testutil/OutputMatcher.cpp | 52 +++++++++++++++++++++ velox/common/testutil/OutputMatcher.h | 32 +++++++++++++ velox/exec/tests/PrintPlanWithStatsTest.cpp | 50 +++----------------- 4 files changed, 93 insertions(+), 45 deletions(-) create mode 100644 velox/common/testutil/OutputMatcher.cpp create mode 100644 velox/common/testutil/OutputMatcher.h diff --git a/velox/common/testutil/CMakeLists.txt b/velox/common/testutil/CMakeLists.txt index f33bb98be06f2..824933f3f98c0 100644 --- a/velox/common/testutil/CMakeLists.txt +++ b/velox/common/testutil/CMakeLists.txt @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -velox_add_library(velox_test_util ScopedTestTime.cpp TestValue.cpp) -velox_link_libraries(velox_test_util PUBLIC velox_exception) +velox_add_library(velox_test_util ScopedTestTime.cpp TestValue.cpp OutputMatcher.cpp) +velox_link_libraries(velox_test_util PUBLIC velox_exception GTest::gtest) if(${VELOX_BUILD_TESTING}) add_subdirectory(tests) diff --git a/velox/common/testutil/OutputMatcher.cpp b/velox/common/testutil/OutputMatcher.cpp new file mode 100644 index 0000000000000..7d3c5a66f1fb0 --- /dev/null +++ b/velox/common/testutil/OutputMatcher.cpp @@ -0,0 +1,52 @@ +/* +* Copyright (c) Facebook, Inc. and its affiliates. +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +#include "velox/common/testutil/OutputMatcher.h" + +#include + +#include +#include +#include + +void OutputMatcher::compareOutputs( + const std::string& testName, + const std::string& result, + const std::vector& expectedRegex) { + std::string line; + std::string eline; + std::istringstream iss(result); + int lineCount = 0; + int expectedLineIndex = 0; + for (; std::getline(iss, line);) { + lineCount++; + std::vector potentialLines; + auto expectedLine = expectedRegex.at(expectedLineIndex++); + while (!RE2::FullMatch(line, expectedLine.line)) { + potentialLines.push_back(expectedLine.line); + if (!expectedLine.optional) { + ASSERT_FALSE(true) << "Output did not match " << "Source:" << testName + << ", Line number:" << lineCount + << ", Line: " << line << ", Expected Line one of: " + << folly::join(",", potentialLines); + } + expectedLine = expectedRegex.at(expectedLineIndex++); + } + } + for (int i = expectedLineIndex; i < expectedRegex.size(); i++) { + ASSERT_TRUE(expectedRegex[expectedLineIndex].optional); + } +} diff --git a/velox/common/testutil/OutputMatcher.h b/velox/common/testutil/OutputMatcher.h new file mode 100644 index 0000000000000..b697734dd942d --- /dev/null +++ b/velox/common/testutil/OutputMatcher.h @@ -0,0 +1,32 @@ +/* +* Copyright (c) Facebook, Inc. and its affiliates. +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +#pragma once + +#include + +struct ExpectedLine { + std::string line; + bool optional = false; +}; + +class OutputMatcher { + public: + static void compareOutputs( + const std::string& testName, + const std::string& result, + const std::vector& expectedRegex); +}; + diff --git a/velox/exec/tests/PrintPlanWithStatsTest.cpp b/velox/exec/tests/PrintPlanWithStatsTest.cpp index b563d8931b258..ee976896aae15 100644 --- a/velox/exec/tests/PrintPlanWithStatsTest.cpp +++ b/velox/exec/tests/PrintPlanWithStatsTest.cpp @@ -14,15 +14,13 @@ * limitations under the License. */ +#include "velox/common/testutil/OutputMatcher.h" #include "velox/exec/PlanNodeStats.h" #include "velox/exec/tests/utils/AssertQueryBuilder.h" #include "velox/exec/tests/utils/HiveConnectorTestBase.h" #include "velox/exec/tests/utils/PlanBuilder.h" #include "velox/exec/tests/utils/TempDirectoryPath.h" -#include -#include - using namespace facebook::velox; using namespace facebook::velox::exec::test; @@ -30,40 +28,6 @@ using facebook::velox::exec::test::PlanBuilder; class PrintPlanWithStatsTest : public HiveConnectorTestBase {}; -struct ExpectedLine { - std::string line; - bool optional = false; -}; - -void compareOutputs( - const std::string& testName, - const std::string& result, - const std::vector& expectedRegex) { - std::string line; - std::string eline; - std::istringstream iss(result); - int lineCount = 0; - int expectedLineIndex = 0; - for (; std::getline(iss, line);) { - lineCount++; - std::vector potentialLines; - auto expectedLine = expectedRegex.at(expectedLineIndex++); - while (!RE2::FullMatch(line, expectedLine.line)) { - potentialLines.push_back(expectedLine.line); - if (!expectedLine.optional) { - ASSERT_FALSE(true) << "Output did not match " << "Source:" << testName - << ", Line number:" << lineCount - << ", Line: " << line << ", Expected Line one of: " - << folly::join(",", potentialLines); - } - expectedLine = expectedRegex.at(expectedLineIndex++); - } - } - for (int i = expectedLineIndex; i < expectedRegex.size(); i++) { - ASSERT_TRUE(expectedRegex[expectedLineIndex].optional); - } -} - void ensureTaskCompletion(exec::Task* task) { // ASSERT_TRUE requires a function with return type void. ASSERT_TRUE(waitForTaskCompletion(task)); @@ -129,7 +93,7 @@ TEST_F(PrintPlanWithStatsTest, innerJoinWithTableScan) { "SELECT t.c0, t.c1 + 1, t.c1 + u.c1 FROM t, u WHERE t.c0 = u.c0"); ensureTaskCompletion(task.get()); - compareOutputs( + OutputMatcher::compareOutputs( ::testing::UnitTest::GetInstance()->current_test_info()->name(), printPlanWithStats(*op, task->taskStats()), {{"-- Project\\[4\\]\\[expressions: \\(c0:INTEGER, ROW\\[\"c0\"\\]\\), \\(p1:BIGINT, plus\\(ROW\\[\"c1\"\\],1\\)\\), \\(p2:BIGINT, plus\\(ROW\\[\"c1\"\\],ROW\\[\"u_c1\"\\]\\)\\)\\] -> c0:INTEGER, p1:BIGINT, p2:BIGINT"}, @@ -146,7 +110,7 @@ TEST_F(PrintPlanWithStatsTest, innerJoinWithTableScan) { {" Input: 0 rows \\(.+\\), Output: 100 rows \\(.+\\), Cpu time: .+, Blocked wall time: .+, Peak memory: 0B, Memory allocations: .+, Threads: 1, CPU breakdown: I/O/F (.+/.+/.+)"}}); // with custom stats - compareOutputs( + OutputMatcher::compareOutputs( ::testing::UnitTest::GetInstance()->current_test_info()->name(), printPlanWithStats(*op, task->taskStats(), true), {{"-- Project\\[4\\]\\[expressions: \\(c0:INTEGER, ROW\\[\"c0\"\\]\\), \\(p1:BIGINT, plus\\(ROW\\[\"c1\"\\],1\\)\\), \\(p2:BIGINT, plus\\(ROW\\[\"c1\"\\],ROW\\[\"u_c1\"\\]\\)\\)\\] -> c0:INTEGER, p1:BIGINT, p2:BIGINT"}, @@ -256,7 +220,7 @@ TEST_F(PrintPlanWithStatsTest, partialAggregateWithTableScan) { .assertResults( "SELECT c5, max(c0), sum(c1), sum(c2), sum(c3), sum(c4) FROM tmp group by c5"); ensureTaskCompletion(task.get()); - compareOutputs( + OutputMatcher::compareOutputs( ::testing::UnitTest::GetInstance()->current_test_info()->name(), printPlanWithStats(*op, task->taskStats()), {{"-- Aggregation\\[1\\]\\[PARTIAL \\[c5\\] a0 := max\\(ROW\\[\"c0\"\\]\\), a1 := sum\\(ROW\\[\"c1\"\\]\\), a2 := sum\\(ROW\\[\"c2\"\\]\\), a3 := sum\\(ROW\\[\"c3\"\\]\\), a4 := sum\\(ROW\\[\"c4\"\\]\\)\\] -> c5:VARCHAR, a0:BIGINT, a1:BIGINT, a2:BIGINT, a3:DOUBLE, a4:DOUBLE"}, @@ -264,7 +228,7 @@ TEST_F(PrintPlanWithStatsTest, partialAggregateWithTableScan) { {" -- TableScan\\[0\\]\\[table: hive_table\\] -> c0:BIGINT, c1:INTEGER, c2:SMALLINT, c3:REAL, c4:DOUBLE, c5:VARCHAR"}, {" Input: 10000 rows \\(.+\\), Output: 10000 rows \\(.+\\), Cpu time: .+, Blocked wall time: .+, Peak memory: .+, Memory allocations: .+, Threads: 1, Splits: 1, CPU breakdown: I/O/F (.+/.+/.+)"}}); - compareOutputs( + OutputMatcher::compareOutputs( ::testing::UnitTest::GetInstance()->current_test_info()->name(), printPlanWithStats(*op, task->taskStats(), true), {{"-- Aggregation\\[1\\]\\[PARTIAL \\[c5\\] a0 := max\\(ROW\\[\"c0\"\\]\\), a1 := sum\\(ROW\\[\"c1\"\\]\\), a2 := sum\\(ROW\\[\"c2\"\\]\\), a3 := sum\\(ROW\\[\"c3\"\\]\\), a4 := sum\\(ROW\\[\"c4\"\\]\\)\\] -> c5:VARCHAR, a0:BIGINT, a1:BIGINT, a2:BIGINT, a3:DOUBLE, a4:DOUBLE"}, @@ -333,7 +297,7 @@ TEST_F(PrintPlanWithStatsTest, tableWriterWithTableScan) { .splits(makeHiveConnectorSplits({filePath})) .copyResults(pool(), task); ensureTaskCompletion(task.get()); - compareOutputs( + OutputMatcher::compareOutputs( ::testing::UnitTest::GetInstance()->current_test_info()->name(), printPlanWithStats(*writePlan, task->taskStats()), {{R"(-- TableWrite\[1\]\[.+InsertTableHandle .+)"}, @@ -341,7 +305,7 @@ TEST_F(PrintPlanWithStatsTest, tableWriterWithTableScan) { {R"( -- TableScan\[0\]\[table: hive_table\] -> c0:BIGINT, c1:INTEGER, c2:SMALLINT, c3:REAL, c4:DOUBLE, c5:VARCHAR)"}, {R"( Input: 100 rows \(.+\), Output: 100 rows \(.+\), Cpu time: .+, Blocked wall time: .+, Peak memory: .+, Memory allocations: .+, Threads: 1, Splits: 1, CPU breakdown: I/O/F (.+/.+/.+))"}}); - compareOutputs( + OutputMatcher::compareOutputs( ::testing::UnitTest::GetInstance()->current_test_info()->name(), printPlanWithStats(*writePlan, task->taskStats(), true), {{R"(-- TableWrite\[1\]\[.+InsertTableHandle .+)"},