Skip to content
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

Merged
merged 10 commits into from
May 6, 2024

Conversation

yiyun-sj
Copy link
Contributor

@yiyun-sj yiyun-sj commented May 2, 2024

Testing Multiple Query Statements

  • This implementation supports multiple query statements to multiple results

Example

// this now works
-STATEMENT INSTALL httpfs; // assumed non-error statement
           LOAD EXTENSION httpfs; // assumed non-error statement
           LOAD FROM 'http://extension.kuzudb.com/dataset/test/city.csv' return *;
---- ok
---- ok
---- 3
Guelph|75000
Kitchener|200000
Waterloo|150000

@yiyun-sj yiyun-sj requested a review from ray6080 May 2, 2024 20:45
@yiyun-sj yiyun-sj linked an issue May 2, 2024 that may be closed by this pull request
@ray6080 ray6080 linked an issue May 3, 2024 that may be closed by this pull request
@yiyun-sj yiyun-sj force-pushed the support-multiple-query-statements branch from e286f8c to 90da710 Compare May 3, 2024 19:34
@yiyun-sj yiyun-sj marked this pull request as ready for review May 3, 2024 21:44
@yiyun-sj yiyun-sj force-pushed the support-multiple-query-statements branch from 74d49c8 to 5dd5ed3 Compare May 6, 2024 15:48
@yiyun-sj yiyun-sj force-pushed the support-multiple-query-statements branch from 5dd5ed3 to d0b530e Compare May 6, 2024 16:01
OK,
HASH,
TUPLES,
TUPLES_CSV,
Copy link
Contributor

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
Copy link
Contributor

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?

Copy link
Contributor Author

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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extractAllResults -> extractExpectedResults.

extractExpectedResult(statement);
} while (nextLine());
}

void TestParser::extractExpectedResult(TestStatement* statement) {
Copy link
Contributor

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.

@yiyun-sj yiyun-sj merged commit cc43956 into master May 6, 2024
@yiyun-sj yiyun-sj deleted the support-multiple-query-statements branch May 6, 2024 18:49
ted-wq-x pushed a commit to ted-wq-x/kuzu that referenced this pull request Nov 14, 2024
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support multiple query statements in e2e test framework Testing framework improvement
2 participants