Skip to content

Commit f28ed1f

Browse files
alinaliBQalinaliBQ
committed
Port test fixture changes
1 parent 7d61b61 commit f28ed1f

File tree

4 files changed

+51
-17
lines changed

4 files changed

+51
-17
lines changed

‎cpp/src/arrow/flight/sql/odbc/odbc_api.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
#include "arrow/flight/sql/odbc/odbc_impl/spi/connection.h"
3232
#include "arrow/util/logging.h"
3333

34-
#if defined _WIN32 || defined _WIN64
34+
#if defined _WIN32
3535
// For displaying DSN Window
3636
# include "arrow/flight/sql/odbc/odbc_impl/system_dsn.h"
3737
#endif
@@ -436,7 +436,7 @@ SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
436436

437437
// GH-46448 TODO: Implement SQL_DRIVER_COMPLETE_REQUIRED in SQLDriverConnect according
438438
// to the spec
439-
#if defined _WIN32 || defined _WIN64
439+
#if defined _WIN32
440440
// Load the DSN window according to driver_completion
441441
if (driver_completion == SQL_DRIVER_PROMPT) {
442442
// Load DSN window before first attempt to connect

‎cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,8 @@ TYPED_TEST(ConnectionTest, TestSQLGetEnvAttrOutputNTS) {
162162

163163
TYPED_TEST(ConnectionTest, DISABLED_TestSQLGetEnvAttrGetLength) {
164164
// Test is disabled because call to SQLGetEnvAttr is handled by the driver manager on
165-
// Windows. This test case can be potentially used on macOS/Linux
165+
// Windows. Windows driver manager ignores the length pointer.
166+
// This test case can be potentially used on macOS/Linux
166167
SQLINTEGER length;
167168
ASSERT_EQ(SQL_SUCCESS,
168169
SQLGetEnvAttr(this->env, SQL_ATTR_ODBC_VERSION, nullptr, 0, &length));
@@ -172,7 +173,8 @@ TYPED_TEST(ConnectionTest, DISABLED_TestSQLGetEnvAttrGetLength) {
172173

173174
TYPED_TEST(ConnectionTest, DISABLED_TestSQLGetEnvAttrNullValuePointer) {
174175
// Test is disabled because call to SQLGetEnvAttr is handled by the driver manager on
175-
// Windows. This test case can be potentially used on macOS/Linux
176+
// Windows. The Windows driver manager doesn't error out when null pointer is passed.
177+
// This test case can be potentially used on macOS/Linux
176178
ASSERT_EQ(SQL_ERROR,
177179
SQLGetEnvAttr(this->env, SQL_ATTR_ODBC_VERSION, nullptr, 0, nullptr));
178180
}
@@ -229,7 +231,8 @@ TYPED_TEST(ConnectionHandleTest, TestSQLDriverConnect) {
229231
ASSERT_EQ(SQL_SUCCESS,
230232
SQLDriverConnect(this->conn, NULL, &connect_str0[0],
231233
static_cast<SQLSMALLINT>(connect_str0.size()), out_str,
232-
kOdbcBufferSize, &out_str_len, SQL_DRIVER_NOPROMPT));
234+
kOdbcBufferSize, &out_str_len, SQL_DRIVER_NOPROMPT))
235+
<< GetOdbcErrorMessage(SQL_HANDLE_DBC, this->conn);
233236

234237
// Check that out_str has same content as connect_str
235238
std::string out_connection_string = ODBC::SqlWcharToString(out_str, out_str_len);
@@ -241,10 +244,11 @@ TYPED_TEST(ConnectionHandleTest, TestSQLDriverConnect) {
241244
ASSERT_TRUE(CompareConnPropertyMap(out_properties, in_properties));
242245

243246
// Disconnect from ODBC
244-
ASSERT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn));
247+
ASSERT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn))
248+
<< GetOdbcErrorMessage(SQL_HANDLE_DBC, this->conn);
245249
}
246250

247-
#if defined _WIN32 || defined _WIN64
251+
#if defined _WIN32
248252
TYPED_TEST(ConnectionHandleTest, TestSQLDriverConnectDsn) {
249253
// Connect string
250254
std::string connect_str = this->GetConnectionString();
@@ -270,13 +274,15 @@ TYPED_TEST(ConnectionHandleTest, TestSQLDriverConnectDsn) {
270274
ASSERT_EQ(SQL_SUCCESS,
271275
SQLDriverConnect(this->conn, NULL, &connect_str0[0],
272276
static_cast<SQLSMALLINT>(connect_str0.size()), out_str,
273-
kOdbcBufferSize, &out_str_len, SQL_DRIVER_NOPROMPT));
277+
kOdbcBufferSize, &out_str_len, SQL_DRIVER_NOPROMPT))
278+
<< GetOdbcErrorMessage(SQL_HANDLE_DBC, this->conn);
274279

275280
// Remove DSN
276281
ASSERT_TRUE(UnregisterDsn(wdsn));
277282

278283
// Disconnect from ODBC
279-
ASSERT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn));
284+
ASSERT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn))
285+
<< GetOdbcErrorMessage(SQL_HANDLE_DBC, this->conn);
280286
}
281287

282288
TYPED_TEST(ConnectionHandleTest, TestSQLConnect) {
@@ -300,13 +306,15 @@ TYPED_TEST(ConnectionHandleTest, TestSQLConnect) {
300306
ASSERT_EQ(SQL_SUCCESS,
301307
SQLConnect(this->conn, dsn0.data(), static_cast<SQLSMALLINT>(dsn0.size()),
302308
uid0.data(), static_cast<SQLSMALLINT>(uid0.size()), pwd0.data(),
303-
static_cast<SQLSMALLINT>(pwd0.size())));
309+
static_cast<SQLSMALLINT>(pwd0.size())))
310+
<< GetOdbcErrorMessage(SQL_HANDLE_DBC, this->conn);
304311

305312
// Remove DSN
306313
ASSERT_TRUE(UnregisterDsn(wdsn));
307314

308315
// Disconnect from ODBC
309-
ASSERT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn));
316+
ASSERT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn))
317+
<< GetOdbcErrorMessage(SQL_HANDLE_DBC, this->conn);
310318
}
311319

312320
TEST_F(ConnectionRemoteTest, TestSQLConnectInputUidPwd) {
@@ -339,13 +347,15 @@ TEST_F(ConnectionRemoteTest, TestSQLConnectInputUidPwd) {
339347
ASSERT_EQ(SQL_SUCCESS,
340348
SQLConnect(this->conn, dsn0.data(), static_cast<SQLSMALLINT>(dsn0.size()),
341349
uid0.data(), static_cast<SQLSMALLINT>(uid0.size()), pwd0.data(),
342-
static_cast<SQLSMALLINT>(pwd0.size())));
350+
static_cast<SQLSMALLINT>(pwd0.size())))
351+
<< GetOdbcErrorMessage(SQL_HANDLE_DBC, conn);
343352

344353
// Remove DSN
345354
ASSERT_TRUE(UnregisterDsn(wdsn));
346355

347356
// Disconnect from ODBC
348-
ASSERT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn));
357+
ASSERT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn))
358+
<< GetOdbcErrorMessage(SQL_HANDLE_DBC, conn);
349359
}
350360

351361
TEST_F(ConnectionRemoteTest, TestSQLConnectInvalidUid) {
@@ -411,13 +421,15 @@ TEST_F(ConnectionRemoteTest, TestSQLConnectDSNPrecedence) {
411421
ASSERT_EQ(SQL_SUCCESS,
412422
SQLConnect(this->conn, dsn0.data(), static_cast<SQLSMALLINT>(dsn0.size()),
413423
uid0.data(), static_cast<SQLSMALLINT>(uid0.size()), pwd0.data(),
414-
static_cast<SQLSMALLINT>(pwd0.size())));
424+
static_cast<SQLSMALLINT>(pwd0.size())))
425+
<< GetOdbcErrorMessage(SQL_HANDLE_DBC, conn);
415426

416427
// Remove DSN
417428
ASSERT_TRUE(UnregisterDsn(wdsn));
418429

419430
// Disconnect from ODBC
420-
ASSERT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn));
431+
ASSERT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn))
432+
<< GetOdbcErrorMessage(SQL_HANDLE_DBC, conn);
421433
}
422434

423435
#endif
@@ -456,5 +468,4 @@ TYPED_TEST(ConnectionHandleTest, TestSQLDisconnectWithoutConnection) {
456468
TYPED_TEST(ConnectionTest, TestConnect) {
457469
// Verifies connect and disconnect works on its own
458470
}
459-
460471
} // namespace arrow::flight::sql::odbc

‎cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,17 @@ std::wstring ODBCRemoteTestBase::GetQueryAllDataTypes() {
150150

151151
void ODBCRemoteTestBase::SetUp() {
152152
if (arrow::internal::GetEnvVar(kTestConnectStr.data()).ValueOr("").empty()) {
153+
skipping_test_ = true;
153154
GTEST_SKIP() << "Skipping test: kTestConnectStr not set";
154155
}
155156
}
156157

157158
void FlightSQLODBCRemoteTestBase::SetUp() {
158159
ODBCRemoteTestBase::SetUp();
160+
if (skipping_test_) {
161+
return;
162+
}
163+
159164
this->Connect();
160165
connected_ = true;
161166
}
@@ -169,16 +174,30 @@ void FlightSQLODBCRemoteTestBase::TearDown() {
169174

170175
void FlightSQLOdbcV2RemoteTestBase::SetUp() {
171176
ODBCRemoteTestBase::SetUp();
177+
if (skipping_test_) {
178+
return;
179+
}
180+
172181
this->Connect(SQL_OV_ODBC2);
173182
connected_ = true;
174183
}
175184

176185
void FlightSQLOdbcHandleRemoteTestBase::SetUp() {
177186
ODBCRemoteTestBase::SetUp();
187+
if (skipping_test_) {
188+
return;
189+
}
190+
178191
this->AllocEnvConnHandles();
192+
allocated_ = true;
179193
}
180194

181-
void FlightSQLOdbcHandleRemoteTestBase::TearDown() { this->FreeEnvConnHandles(); }
195+
void FlightSQLOdbcHandleRemoteTestBase::TearDown() {
196+
if (allocated_) {
197+
this->FreeEnvConnHandles();
198+
allocated_ = false;
199+
}
200+
}
182201

183202
std::string FindTokenInCallHeaders(const CallHeaders& incoming_headers) {
184203
// Lambda function to compare characters without case sensitivity.

‎cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ class ODBCRemoteTestBase : public ::testing::Test {
7979

8080
protected:
8181
void SetUp() override;
82+
83+
bool skipping_test_ = false;
8284
};
8385

8486
/// \brief Base test fixture for running tests against a remote server.
@@ -107,6 +109,8 @@ class FlightSQLOdbcHandleRemoteTestBase : public FlightSQLODBCRemoteTestBase {
107109
protected:
108110
void SetUp() override;
109111
void TearDown() override;
112+
113+
bool allocated_ = false;
110114
};
111115

112116
static constexpr std::string_view kAuthorizationHeader = "authorization";

0 commit comments

Comments
 (0)