-
Notifications
You must be signed in to change notification settings - Fork 105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support multiple query statements in e2e test framework #3437
Conversation
e286f8c
to
90da710
Compare
74d49c8
to
5dd5ed3
Compare
5dd5ed3
to
d0b530e
Compare
OK, | ||
HASH, | ||
TUPLES, | ||
TUPLES_CSV, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TUPLES_CSV
-> 'CSV_FILE`.
ResultType type; | ||
std::vector<std::string> expectedTuples; | ||
uint64_t numTuples = 0; | ||
std::string expectedMessage; // errorMsg || CSVFile || hashValue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can u directly reuse expectedTuples
for message too with a renaming of expectedTuples
to expectedResult
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we will just use expectedResult[0]
for all the single string results then.
(I originally wanted to use union to make things more clear but that has some constructor/destructor issues for compile)
@@ -94,6 +94,7 @@ class TestParser { | |||
void genGroupName(); | |||
void parseHeader(); | |||
void parseBody(); | |||
void extractAllResults(TestStatement* statement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extractAllResults
-> extractExpectedResults
.
test/test_runner/test_parser.cpp
Outdated
extractExpectedResult(statement); | ||
} while (nextLine()); | ||
} | ||
|
||
void TestParser::extractExpectedResult(TestStatement* statement) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TestQueryResult TestParser:: extractExpectedResultFromToken()
I would rename the function to extractExpectedResultFromToken
, and return TestQueryResult
without passing in TestStatement
, which should be handled by extractAllResults
to push in query result. This looks cleaner to me.
* preliminary implementation to support multiple query statements * Run clang-format * update one test case to use multiple query statements * fix compile error * support multiple queries to multiple results * Run clang-format * remove comment * fix msvc compile error * address comments * Run clang-format --------- Co-authored-by: CI Bot <yiyun-sj@users.noreply.github.com> (cherry picked from commit cc43956)
Testing Multiple Query Statements
Example