From 945884880b9d55a98d297c0f32bb52295fc3783f Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Wed, 16 Oct 2024 12:25:10 -0400 Subject: [PATCH] fix lint and flakey test --- c/driver/snowflake/snowflake_test.cc | 2 +- c/validation/adbc_validation_connection.cc | 27 +++++++++++----------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/c/driver/snowflake/snowflake_test.cc b/c/driver/snowflake/snowflake_test.cc index 90735b3ffe..67d3cbb3fd 100644 --- a/c/driver/snowflake/snowflake_test.cc +++ b/c/driver/snowflake/snowflake_test.cc @@ -133,7 +133,7 @@ class SnowflakeQuirks : public adbc_validation::DriverQuirks { case NANOARROW_TYPE_LARGE_STRING: case NANOARROW_TYPE_LIST: case NANOARROW_TYPE_LARGE_LIST: - return NANOARROW_TYPE_STRING; + return NANOARROW_TYPE_STRING; default: return ingest_type; } diff --git a/c/validation/adbc_validation_connection.cc b/c/validation/adbc_validation_connection.cc index 6ef4302137..9cef88c6d5 100644 --- a/c/validation/adbc_validation_connection.cc +++ b/c/validation/adbc_validation_connection.cc @@ -701,7 +701,6 @@ void ConnectionTest::TestMetadataGetObjectsTablesTypes() { db_schemas_index < ArrowArrayViewListChildOffset(catalog_db_schemas_list, row + 1); db_schemas_index++) { - // db_schema_tables should either be null or an empty list for (int64_t tables_index = ArrowArrayViewListChildOffset(db_schema_tables_list, db_schemas_index); @@ -743,13 +742,12 @@ void ConnectionTest::TestMetadataGetObjectsColumns() { struct TestCase { std::optional filter; - std::vector column_names; - std::vector ordinal_positions; + std::vector> columns; }; std::vector test_cases; - test_cases.push_back({std::nullopt, {"int64s", "strings"}, {1, 2}}); - test_cases.push_back({"in%", {"int64s"}, {1}}); + test_cases.push_back({std::nullopt, {{"int64s", 1}, {"strings", 2}}}); + test_cases.push_back({"in%", {{"int64s", 1}}}); const std::string catalog = quirks()->catalog(); @@ -759,13 +757,14 @@ void ConnectionTest::TestMetadataGetObjectsColumns() { SCOPED_TRACE(scope); StreamReader reader; + std::vector> columns; std::vector column_names; std::vector ordinal_positions; ASSERT_THAT( AdbcConnectionGetObjects( - &connection, ADBC_OBJECT_DEPTH_COLUMNS, catalog.c_str(), nullptr, nullptr, nullptr, - test_case.filter.has_value() ? test_case.filter->c_str() : nullptr, + &connection, ADBC_OBJECT_DEPTH_COLUMNS, catalog.c_str(), nullptr, nullptr, + nullptr, test_case.filter.has_value() ? test_case.filter->c_str() : nullptr, &reader.stream.value, &error), IsOkStatus(&error)); ASSERT_NO_FATAL_FAILURE(reader.GetSchema()); @@ -835,10 +834,9 @@ void ConnectionTest::TestMetadataGetObjectsColumns() { std::string temp(name.data, name.size_bytes); std::transform(temp.begin(), temp.end(), temp.begin(), [](unsigned char c) { return std::tolower(c); }); - column_names.push_back(std::move(temp)); - ordinal_positions.push_back( - static_cast(ArrowArrayViewGetIntUnsafe( - table_columns->children[1], columns_index))); + columns.emplace_back(std::move(temp), + static_cast(ArrowArrayViewGetIntUnsafe( + table_columns->children[1], columns_index))); } } } @@ -848,8 +846,11 @@ void ConnectionTest::TestMetadataGetObjectsColumns() { } while (reader.array->release); ASSERT_TRUE(found_expected_table) << "Did (not) find table in metadata"; - ASSERT_EQ(test_case.column_names, column_names); - ASSERT_EQ(test_case.ordinal_positions, ordinal_positions); + // metadata columns do not guarantee the order they are returned in, we can + // avoid the test being flakey by sorting the column names we found + std::sort(columns.begin(), columns.end(), + [](const auto& a, const auto& b) -> bool { return a.first < b.first; }); + ASSERT_EQ(test_case.columns, columns); } }