From 472ceb49e02bb7685a2f2aa48704b1e786a06773 Mon Sep 17 00:00:00 2001 From: Benjamin Winger Date: Thu, 5 Sep 2024 15:11:38 +0100 Subject: [PATCH] Enable read only DB tests on windows --- src/common/file_system/local_file_system.cpp | 3 ++- test/c_api/database_test.cpp | 4 ---- test/main/CMakeLists.txt | 1 + test/main/connection_test.cpp | 1 + test/main/system_config_test.cpp | 4 +++- tools/nodejs_api/test/test_database.js | 16 +++------------- tools/python_api/test/conftest.py | 6 +----- tools/python_api/test/test_database.py | 12 +++++------- tools/python_api/test/test_exception.py | 3 --- 9 files changed, 16 insertions(+), 34 deletions(-) diff --git a/src/common/file_system/local_file_system.cpp b/src/common/file_system/local_file_system.cpp index b2a574125e8..63432add35e 100644 --- a/src/common/file_system/local_file_system.cpp +++ b/src/common/file_system/local_file_system.cpp @@ -107,7 +107,8 @@ std::unique_ptr LocalFileSystem::openFile(const std::string& path, int LOCKFILE_FAIL_IMMEDIATELY | LOCKFILE_EXCLUSIVE_LOCK; OVERLAPPED overlapped = {0}; overlapped.Offset = 0; - BOOL rc = LockFileEx(handle, dwFlags, 0, 0, 0, &overlapped); + BOOL rc = LockFileEx(handle, dwFlags, 0 /*reserved*/, 1 /*numBytesLow*/, 0 /*numBytesHigh*/, + &overlapped); if (!rc) { throw IOException("Could not set lock on file : " + fullPath); } diff --git a/test/c_api/database_test.cpp b/test/c_api/database_test.cpp index f14f10140ee..2525f05345e 100644 --- a/test/c_api/database_test.cpp +++ b/test/c_api/database_test.cpp @@ -24,10 +24,6 @@ TEST_F(CApiDatabaseTest, CreationAndDestroy) { } TEST_F(CApiDatabaseTest, CreationReadOnly) { -// TODO: Enable this test on Windows when the read-only mode is implemented. -#ifdef _WIN32 - return; -#endif kuzu_database database; kuzu_connection connection; kuzu_query_result queryResult; diff --git a/test/main/CMakeLists.txt b/test/main/CMakeLists.txt index 944fe90cee7..e29f5efd789 100644 --- a/test/main/CMakeLists.txt +++ b/test/main/CMakeLists.txt @@ -1,5 +1,6 @@ if(MSVC) add_kuzu_api_test(main_test + system_config_test.cpp arrow_test.cpp connection_test.cpp prepare_test.cpp diff --git a/test/main/connection_test.cpp b/test/main/connection_test.cpp index 0b532acd60d..a5b1cedd183 100644 --- a/test/main/connection_test.cpp +++ b/test/main/connection_test.cpp @@ -150,6 +150,7 @@ TEST_F(ApiTest, Prepare) { } TEST_F(ApiTest, CreateTableAfterClosingDatabase) { + database.reset(); database = std::make_unique(databasePath, *systemConfig); conn = std::make_unique(database.get()); diff --git a/test/main/system_config_test.cpp b/test/main/system_config_test.cpp index 2d015f51dab..64501094037 100644 --- a/test/main/system_config_test.cpp +++ b/test/main/system_config_test.cpp @@ -5,7 +5,9 @@ using namespace kuzu::common; using namespace kuzu::testing; using namespace kuzu::main; -class SystemConfigTest : public ApiTest {}; +class SystemConfigTest : public ApiTest { + void SetUp() override { BaseGraphTest::SetUp(); } +}; void assertQuery(QueryResult& result) { auto a = result.toString(); diff --git a/tools/nodejs_api/test/test_database.js b/tools/nodejs_api/test/test_database.js index 8a494090a98..cc741672162 100644 --- a/tools/nodejs_api/test/test_database.js +++ b/tools/nodejs_api/test/test_database.js @@ -68,12 +68,6 @@ describe("Database constructor", function () { }); it("should create a database in read-only mode", async function () { - // TODO: Enable this test on Windows when the read-only mode is implemented. - if (process.platform === "win32") { - this._runnable.title += " (skipped: not implemented on Windows)"; - this.skip(); - } - const tmpDbPath = await new Promise((resolve, reject) => { tmp.dir({ unsafeCleanup: true }, (err, path, _) => { if (err) { @@ -89,7 +83,7 @@ describe("Database constructor", function () { assert.exists(testDb._database); assert.isTrue(testDb._isInitialized); assert.notExists(testDb._initPromise); - delete testDb; + await testDb.close(); const testDbReadOnly = new kuzu.Database( tmpDbPath, 1 << 28 /* 256MB */, @@ -193,7 +187,7 @@ describe("Database constructor", function () { it("should create an in-memory database when no path is provided", async function () { const testDb = new kuzu.Database(); const conn = new kuzu.Connection(testDb); - let res = await conn.query("CREATE NODE TABLE person(name STRING, age INT64, PRIMARY KEY(name));"); + let res = await conn.query("CREATE NODE TABLE person(name STRING, age INT64, PRIMARY KEY(name));"); res.close(); res = await conn.query("CREATE (:person {name: 'Alice', age: 30});"); res.close(); @@ -235,10 +229,6 @@ describe("Database constructor", function () { describe("Database close", function () { it("should allow initializing a new database after closing", async function () { - if (process.platform === "win32") { - this._runnable.title += " (skipped: not implemented on Windows)"; - this.skip(); - } const tmpDbPath = await new Promise((resolve, reject) => { tmp.dir({ unsafeCleanup: true }, (err, path, _) => { if (err) { @@ -253,7 +243,7 @@ describe("Database close", function () { assert.notEqual(subProcessResult.code, 0); assert.include( subProcessResult.stderr, - "Error: IO exception: Could not set lock on file" + "Could not set lock on file" ); await testDb.close(); subProcessResult = await openDatabaseOnSubprocess(tmpDbPath); diff --git a/tools/python_api/test/conftest.py b/tools/python_api/test/conftest.py index f74179c5c6d..792c587ca4e 100644 --- a/tools/python_api/test/conftest.py +++ b/tools/python_api/test/conftest.py @@ -165,11 +165,7 @@ def conn_db_readonly(tmp_path: Path) -> ConnDB: """Return a cached read-only connection and database.""" global _READONLY_CONN_DB_ if _READONLY_CONN_DB_ is None: - # On Windows, the read-only mode is not implemented yet. - # Therefore, we create a writable database on Windows. - # However, the caller should ensure that the database is not modified. - # TODO: Remove this workaround when the read-only mode is implemented on Windows. - _READONLY_CONN_DB_ = create_conn_db(init_db(tmp_path), read_only=sys.platform != "win32") + _READONLY_CONN_DB_ = create_conn_db(init_db(tmp_path), read_only=True) return _READONLY_CONN_DB_ diff --git a/tools/python_api/test/test_database.py b/tools/python_api/test/test_database.py index 4f368540d4a..d6e414e3091 100644 --- a/tools/python_api/test/test_database.py +++ b/tools/python_api/test/test_database.py @@ -35,13 +35,11 @@ def test_database_close(tmp_path: Path, build_dir: Path) -> None: assert db._database is not None # Open the database on a subprocess. It should raise an exception. - # TODO: Enable this test on Windows when the read-only mode is implemented. - if sys.platform != "win32": - with pytest.raises( - RuntimeError, - match=r"RuntimeError: IO exception: Could not set lock on file", - ): - open_database_on_subprocess(db_path, build_dir) + with pytest.raises( + Exception, + match=r"RuntimeError: IO exception: Could not set lock on file", + ): + open_database_on_subprocess(db_path, build_dir) db.close() assert db.is_closed diff --git a/tools/python_api/test/test_exception.py b/tools/python_api/test/test_exception.py index 79031739188..897e5e801d3 100644 --- a/tools/python_api/test/test_exception.py +++ b/tools/python_api/test/test_exception.py @@ -25,9 +25,6 @@ def test_db_path_exception() -> None: def test_read_only_exception(conn_db_readonly: ConnDB) -> None: - # TODO: Enable this test on Windows when the read-only mode is implemented. - if sys.platform == "win32": - pytest.skip("Read-only mode has not been implemented on Windows yet") _, db = conn_db_readonly path = db.database_path read_only_db = kuzu.Database(path, read_only=True)