-
Notifications
You must be signed in to change notification settings - Fork 86
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
Run tests in parallel #1657
Run tests in parallel #1657
Changes from 7 commits
3c44657
2df3459
e0122be
e10c5e1
0fccece
2de9c92
6145923
c496220
2ca1a3a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,7 +112,7 @@ function(add_kuzu_test TEST_NAME) | |
target_link_libraries(${TEST_NAME} PRIVATE test_helper test_runner graph_test) | ||
target_include_directories(${TEST_NAME} PRIVATE ${PROJECT_SOURCE_DIR}/test/include) | ||
include(GoogleTest) | ||
gtest_discover_tests(${TEST_NAME}) | ||
gtest_discover_tests(${TEST_NAME} DISCOVERY_MODE PRE_TEST) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you double check if this change is necessary, if not, revert. |
||
endfunction() | ||
|
||
add_definitions(-DKUZU_ROOT_DIRECTORY="${PROJECT_SOURCE_DIR}") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,11 +2,10 @@ | |
|
||
class CApiDatabaseTest : public EmptyDBTest { | ||
public: | ||
void SetUp() override {} | ||
void SetUp() override { EmptyDBTest::SetUp(); } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mewim Is it ok to add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I missed this. Since it is only setting a path it is fine. |
||
}; | ||
|
||
TEST_F(CApiDatabaseTest, CreationAndDestroy) { | ||
auto databasePath = TestHelper::getTmpTestDir(); | ||
auto databasePathCStr = databasePath.c_str(); | ||
auto database = kuzu_database_init(databasePathCStr, 0); | ||
ASSERT_NE(database, nullptr); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
#include "graph_test/graph_test.h" | ||
|
||
using namespace kuzu::testing; | ||
using namespace kuzu::common; | ||
|
||
void deleteMatchingDir(const std::string& dirPath, const std::string& match) { | ||
for (const auto& entry : std::filesystem::directory_iterator(dirPath)) { | ||
if (entry.path().filename().string().find(match) != std::string::npos) { | ||
FileUtils::removeDir(entry.path()); | ||
} | ||
} | ||
} | ||
|
||
int main(int argc, char** argv) { | ||
std::vector<std::string> tempDirs = { | ||
TestHelper::PARQUET_TEMP_DATASET_PATH, TestHelper::TMP_TEST_DIR}; | ||
if (argc > 1 && std::string(argv[1]) == "--gtest_list_tests") { | ||
for (const auto& tempDir : tempDirs) { | ||
// path = test/unittest_temp_ | ||
std::filesystem::path path = tempDir; | ||
// dirToCheck = test | ||
std::string dirToCheck = path.parent_path().string(); | ||
// dirPatternToRemove = unittest_temp_ | ||
std::string dirPatternToRemove = path.filename().string(); | ||
deleteMatchingDir(TestHelper::appendKuzuRootPath(dirToCheck), dirPatternToRemove); | ||
} | ||
} | ||
return 0; | ||
} |
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.
I'm not familiar with ASan. Is it ok to run multiple jobs here?
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.
I think it should be fine.
I noticed one test (
CApiConnectionTest.Interrupt
) failing in asan CI test, can you take a further look?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.
I can take a look but this error is also in other PRs.
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.
Got it. Can you open an issue on this then?