8000 Some cleanup for new executor test code, which accidentally fixes ASA… · arangodb/arangodb@9998b4e · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 9998b4e

Browse files
markuspfjsteemann
markuspf
and
jsteemann
authored
Some cleanup for new executor test code, which accidentally fixes ASAN failures in ExecutorTestHelper (#11283)
* Fix memory leak in ExecutorTestHelper * Remove useless SubqueryExecutorTest * Some cleanup and decrud * Fix fallout from cleanup and decrud * remove unused concatPipelines * More cleanups in Executor Tests * Even more cleanup * Fix the bleepin registers again * fix a memleak in ngram tests Co-authored-by: jsteemann <jan@arangodb.com>
1 parent f59b3c6 commit 9998b4e

24 files changed

+665
-1140
lines changed

‎tests/Aql/AqlExecutorTestCase.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
////////////////////////////////////////////////////////////////////////////////
2+
/// DISCLAIMER
3+
///
4+
/// Copyright 2020 ArangoDB GmbH, Cologne, Germany
5+
///
6+
/// Licensed under the Apache License, Version 2.0 (the "License");
7+
/// you may not use this file except in compliance with the License.
8+
/// You may obtain a copy of the License at
9+
///
10+
/// http://www.apache.org/licenses/LICENSE-2.0
11+
///
12+
/// Unless required by applicable law or agreed to in writing, software
13+
/// distributed under the License is distributed on an "AS IS" BASIS,
14+
/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
/// See the License for the specific language governing permissions and
16+
/// limitations under the License.
17+
///
18+
/// Copyright holder is ArangoDB GmbH, Cologne, Germany
19+
///
20+
////////////////////////////////////////////////////////////////////////////////
21+
22+
#include "AqlExecutorTestCase.h"
23+
24+
using namespace arangodb::tests::aql;
25+
26+
template <bool enableQueryTrace>
27+
AqlExecutorTestCase<enableQueryTrace>::AqlExecutorTestCase()
28+
: fakedQuery{_server->createFakeQuery(enableQueryTrace)} {
29+
auto engine = std::make_unique<ExecutionEngine>(*fakedQuery, SerializationFormat::SHADOWROWS);
30+
fakedQuery->setEngine(engine.release());
31+
if constexpr (enableQueryTrace) {
32+
Logger::QUERIES.setLogLevel(LogLevel::DEBUG);
33+
}
34+
}
35+
36+
template <bool enableQueryTrace>
37+
AqlExecutorTestCase<enableQueryTrace>::~AqlExecutorTestCase() {
38+
if constexpr (enableQueryTrace) {
39+
Logger::QUERIES.setLogLevel(LogLevel::INFO);
40+
}
41+
}
42+
43+
template <bool enableQueryTrace>
44+
auto AqlExecutorTestCase<enableQueryTrace>::generateNodeDummy() -> ExecutionNode* {
45+
auto dummy = std::make_unique<SingletonNode>(fakedQuery->plan(), _execNodes.size());
46+
auto res = dummy.get();
47+
_execNodes.emplace_back(std::move(dummy));
48+
return res;
49+
}
50+
template <bool enableQueryTrace>
51+
auto AqlExecutorTestCase<enableQueryTrace>::manager() const -> AqlItemBlockManager& {
52+
return fakedQuery->engine()->itemBlockManager();
53+
}
54+
55+
template class ::arangodb::tests::aql::AqlExecutorTestCase<true>;
56+
template class ::arangodb::tests::aql::AqlExecutorTestCase<false>;

‎tests/Aql/AqlExecutorTestCase.h

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
////////////////////////////////////////////////////////////////////////////////
2+
/// DISCLAIMER
3+
///
4+
/// Copyright 2020 ArangoDB GmbH, Cologne, Germany
5+
///
6+
/// Licensed under the Apache License, Version 2.0 (the "License");
7+
/// you may not use this file except in compliance with the License.
8+
/// You may obtain a copy of the License at
9+
///
10+
/// http://www.apache.org/licenses/LICENSE-2.0
11+
///
12+
/// Unless required by applicable law or agreed to in writing, software
13+
/// distributed under the License is distributed on an "AS IS" BASIS,
14+
/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
/// See the License for the specific language governing permissions and
16+
/// limitations under the License.
17+
///
18+
/// Copyright holder is ArangoDB GmbH, Cologne, Germany
19+
///
20+
////////////////////////////////////////////////////////////////////////////////
21+
22+
#ifndef TESTS_AQL_EXECUTORTESTCASE_H
23+
#define TESTS_AQL_EXECUTORTESTCASE_H
24+
25+
#include "gtest/gtest.h"
26+
27+
#include "ExecutorTestHelper.h"
28+
29+
#include "Aql/AqlItemBlockManager.h"
30+
#include "Aql/ExecutionNode.h"
31+
32+
#include "Mocks/Servers.h"
33+
34+
using namespace arangodb::aql;
35+
using namespace arangodb::tests::aql;
36+
37+
namespace arangodb {
38+
namespace tests {
39+
namespace aql {
40+
41+
/**
42+
* @brief Base class for ExecutorTests in Aql.
43+
* It will provide a test server, including
44+
* an AqlQuery, as well as the ability to generate
45+
* Dummy ExecutionNodes.
46+
*
47+
* @tparam enableQueryTrace Enable Aql Profile Trace logging
48+
*/
49+
template <bool enableQueryTrace = false>
50+
class AqlExecutorTestCase : public ::testing::Test {
51+
public:
52+
// Creating a server instance costs a lot of time, so do it only once.
53+
// Note that newer version of gtest call these SetUpTestSuite/TearDownTestSuite
54+
static void SetUpTestCase() {
55+
_server = std::make_unique<mocks::MockAqlServer>();
56+
}
57+
58+
static void TearDownTestCase() { _server.reset(); }
59+
60+
protected:
61+
AqlExecutorTestCase();
62+
virtual ~AqlExecutorTestCase();
63+
64+
/**
65+
* @brief Creates and manages a ExecutionNode.
66+
* These nodes can be used to create the Executors
67+
* Caller does not need to manage the memory.
68+
*
69+
* @return ExecutionNode* Pointer to a dummy ExecutionNode. Memory is managed, do not delete.
70+
*/
71+
auto generateNodeDummy() -> ExecutionNode*;
72+
73+
auto manager() const -> AqlItemBlockManager&;
74+
75+
template <std::size_t inputColumns = 1, std::size_t outputColumns = 1>
76+
auto makeExecutorTestHelper() -> ExecutorTestHelper<inputColumns, outputColumns> {
77+
return ExecutorTestHelper<inputColumns, outputColumns>(*fakedQuery, itemBlockManager);
78+
}
79+
80+
private:
81+
std::vector<std::unique_ptr<ExecutionNode>> _execNodes;
82+
83+
protected:
84+
// available variables
85+
static inline std::unique_ptr<mocks::MockAqlServer> _server;
86+
ResourceMonitor monitor{};
87+
AqlItemBlockManager itemBlockManager{&monitor, SerializationFormat::SHADOWROWS};
88+
std::unique_ptr<arangodb::aql::Query> fakedQuery;
89+
};
90+
91+
/**
92+
* @brief Shortcut handle for parameterized AqlExecutorTestCases with param
93+
*
94+
* @tparam T The Test Parameter used for gtest.
95+
* @tparam enableQueryTrace Enable Aql Profile Trace logging
96+
*/
97+
template <typename T, bool enableQueryTrace = false>
98+
class AqlExecutorTestCaseWithParam : public AqlExecutorTestCase<enableQueryTrace>,
99+
public ::testing::WithParamInterface<T> {};
100+
101+
} // namespace aql
102+
} // namespace tests
103+
} // namespace arangodb
104+
#endif

‎tests/Aql/CalculationExecutorTest.cpp

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@
2626
#include "gtest/gtest.h"
2727

2828
#include "Aql/AqlCall.h"
29+
#include "AqlExecutorTestCase.h"
2930
#include "AqlItemBlockHelper.h"
30-
#include "ExecutorTestHelper.h"
3131
#include "RowFetcherHelper.h"
3232

3333
#include "Aql/AqlItemBlock.h"
@@ -129,12 +129,11 @@ INSTANTIATE_TEST_CASE_P(CalculationExecutor, CalculationExecutorTest,
129129
splitStep<1>, splitStep<2>));
130130

131131
TEST_P(CalculationExecutorTest, reference_empty_input) {
132-
// auto infos = buildInfos();
133132
AqlCall call{};
134133
ExecutionStats stats{};
135134

136-
ExecutorTestHelper<2, 2>(*fakedQuery)
137-
.setExecBlock<CalculationExecutor<CalculationType::Reference>>(std::move(infos))
135+
makeExecutorTestHelper<2, 2>()
136+
.addConsumer<CalculationExecutor<CalculationType::Reference>>(std::move(infos))
138137
.setInputValue({})
139138
.setInputSplitType(getSplit())
140139
.setCall(call)
@@ -149,8 +148,8 @@ TEST_P(CalculationExecutorTest, reference_some_input) {
149148
AqlCall call{};
150149
ExecutionStats stats{};
151150

152-
ExecutorTestHelper<2, 2>(*fakedQuery)
153-
.setExecBlock<CalculationExecutor<CalculationType::Reference>>(std::move(infos))
151+
makeExecutorTestHelper<2, 2>()
152+
.addConsumer<CalculationExecutor<CalculationType::Reference>>(std::move(infos))
154153
.setInputValue(MatrixBuilder<2>{
155154
RowBuilder<2>{0, NoneEntry{}}, RowBuilder<2>{1, NoneEntry{}},
156155
RowBuilder<2>{R"("a")", NoneEntry{}}, RowBuilder<2>{2, NoneEntry{}},
@@ -174,8 +173,8 @@ TEST_P(CalculationExecutorTest, referece_some_input_skip) {
174173
call.offset = 4;
175174
ExecutionStats stats{};
176175

177-
ExecutorTestHelper<2, 2>(*fakedQuery)
178-
.setExecBlock<CalculationExecutor<CalculationType::Reference>>(std::move(infos))
176+
makeExecutorTestHelper<2, 2>()
177+
.addConsumer<CalculationExecutor<CalculationType::Reference>>(std::move(infos))
179178
.setInputValue(MatrixBuilder<2>{
180179
RowBuilder<2>{0, NoneEntry{}}, RowBuilder<2>{1, NoneEntry{}},
181180
RowBuilder<2>{R"("a")", NoneEntry{}}, RowBuilder<2>{2, NoneEntry{}},
@@ -196,8 +195,8 @@ TEST_P(CalculationExecutorTest, reference_some_input_limit) {
196195
call.hardLimit = 4;
197196
ExecutionStats stats{};
198197

199-
ExecutorTestHelper<2, 2>(*fakedQuery)
200-
.setExecBlock<CalculationExecutor<CalculationType::Reference>>(std::move(infos))
198+
makeExecutorTestHelper<2, 2>()
199+
.addConsumer<CalculationExecutor<CalculationType::Reference>>(std::move(infos))
201200
.setInputValue(MatrixBuilder<2>{
202201
RowBuilder<2>{0, NoneEntry{}}, RowBuilder<2>{1, NoneEntry{}},
203202
RowBuilder<2>{R"("a")", NoneEntry{}}, RowBuilder<2>{2, NoneEntry{}},
@@ -220,8 +219,8 @@ TEST_P(CalculationExecutorTest, reference_some_input_limit_fullcount) {
220219
call.fullCount = true;
221220
ExecutionStats stats{};
222221

223-
ExecutorTestHelper<2, 2>(*fakedQuery)
224-
.setExecBlock<CalculationExecutor<CalculationType::Reference>>(std::move(infos))
222+
makeExecutorTestHelper<2, 2>()
223+
.addConsumer<CalculationExecutor<CalculationType::Reference>>(std::move(infos))
225224
.setInputValue(MatrixBuilder<2>{
226225
RowBuilder<2>{0, NoneEntry{}}, RowBuilder<2>{1, NoneEntry{}},
227226
RowBuilder<2>{R"("a")", NoneEntry{}}, RowBuilder<2>{2, NoneEntry{}},
@@ -242,8 +241,8 @@ TEST_P(CalculationExecutorTest, condition_some_input) {
242241
AqlCall call{};
243242
ExecutionStats stats{};
244243

245-
ExecutorTestHelper<2, 2>(*fakedQuery)
246-
.setExecBlock<CalculationExecutor<CalculationType::Condition>>(std::move(infos))
244+
makeExecutorTestHelper<2, 2>()
245+
.addConsumer<CalculationExecutor<CalculationType::Condition>>(std::move(infos))
247246
.setInputValue(MatrixBuilder<2>{
248247
RowBuilder<2>{0, NoneEntry{}}, RowBuilder<2>{1, NoneEntry{}},
249248
RowBuilder<2>{R"("a")", NoneEntry{}}, RowBuilder<2>{2, NoneEntry{}},
@@ -267,8 +266,8 @@ TEST_P(CalculationExecutorTest, condition_some_input_skip) {
267266
call.offset = 4;
268267
ExecutionStats stats{};
269268

270-
ExecutorTestHelper<2, 2>(*fakedQuery)
271-
.setExecBlock<CalculationExecutor<CalculationType::Condition>>(std::move(infos))
269+
makeExecutorTestHelper<2, 2>()
270+
.addConsumer<CalculationExecutor<CalculationType::Condition>>(std::move(infos))
272271
.setInputValue(MatrixBuilder<2>{
273272
RowBuilder<2>{0, NoneEntry{}}, RowBuilder<2>{1, NoneEntry{}},
274273
RowBuilder<2>{R"("a")", NoneEntry{}}, RowBuilder<2>{2, NoneEntry{}},
@@ -289,8 +288,8 @@ TEST_P(CalculationExecutorTest, condition_some_input_limit) {
289288
call.hardLimit = 4;
290289
ExecutionStats stats{};
291290

292-
ExecutorTestHelper<2, 2>(*fakedQuery)
293-
.setExecBlock<CalculationExecutor<CalculationType::Condition>>(std::move(infos))
291+
makeExecutorTestHelper<2, 2>()
292+
.addConsumer<CalculationExecutor<CalculationType::Condition>>(std::move(infos))
294293
.setInputValue(MatrixBuilder<2>{
295294
RowBuilder<2>{0, NoneEntry{}}, RowBuilder<2>{1, NoneEntry{}},
296295
RowBuilder<2>{R"("a")", NoneEntry{}}, RowBuilder<2>{2, NoneEntry{}},
@@ -313,8 +312,8 @@ TEST_P(CalculationExecutorTest, condition_some_input_limit_fullcount) {
313312
call.fullCount = true;
314313
ExecutionStats stats{};
315314

316-
ExecutorTestHelper<2, 2>(*fakedQuery)
317-
.setExecBlock<CalculationExecutor<CalculationType::Condition>>(std::move(infos))
315+
makeExecutorTestHelper<2, 2>()
316+
.addConsumer<CalculationExecutor<CalculationType::Condition>>(std::move(infos))
318317
.setInputValue(MatrixBuilder<2>{
319318
RowBuilder<2>{0, NoneEntry{}}, RowBuilder<2>{1, NoneEntry{}},
320319
RowBuilder<2>{R"("a")", NoneEntry{}}, RowBuilder<2>{2, NoneEntry{}},
@@ -333,12 +332,11 @@ TEST_P(CalculationExecutorTest, condition_some_input_limit_fullcount) {
333332

334333
// Could be fixed and enabled if one enabled the V8 engine
335334
TEST_P(CalculationExecutorTest, DISABLED_v8condition_some_input) {
336-
// auto infos = buildInfos();
337335
AqlCall call{};
338336
ExecutionStats stats{};
339337

340-
ExecutorTestHelper<2, 2>(*fakedQuery)
341-
.setExecBlock<CalculationExecutor<CalculationType::V8Condition>>(std::move(infos))
338+
makeExecutorTestHelper<2, 2>()
339+
.addConsumer<CalculationExecutor<CalculationType::V8Condition>>(std::move(infos))
342340
.setInputValue(MatrixBuilder<2>{
343341
RowBuilder<2>{0, NoneEntry{}}, RowBuilder<2>{1, NoneEntry{}},
344342
RowBuilder<2>{R"("a")", NoneEntry{}}, RowBuilder<2>{2, NoneEntry{}},

0 commit comments

Comments
 (0)
0