From b5166470a1d59045b5c6588891d79d6d5672764b Mon Sep 17 00:00:00 2001 From: vuule Date: Wed, 2 Aug 2023 16:34:43 -0700 Subject: [PATCH 1/2] don't use CUDF_EXPECTS in tests --- cpp/tests/copying/concatenate_tests.cpp | 2 +- cpp/tests/copying/split_tests.cpp | 11 ++++---- cpp/tests/groupby/tdigest_tests.cu | 8 +++--- cpp/tests/interop/from_arrow_test.cpp | 9 +++---- cpp/tests/interop/to_arrow_test.cpp | 10 +++---- cpp/tests/io/arrow_io_source_test.cpp | 2 +- cpp/tests/io/csv_test.cpp | 3 +-- cpp/tests/io/json_test.cpp | 2 +- cpp/tests/io/parquet_test.cpp | 33 +++++++++++------------ cpp/tests/transform/row_bit_count_test.cu | 4 +-- cpp/tests/transpose/transpose_test.cpp | 6 ++--- 11 files changed, 40 insertions(+), 50 deletions(-) diff --git a/cpp/tests/copying/concatenate_tests.cpp b/cpp/tests/copying/concatenate_tests.cpp index 99ef1df1c2b..1b511fdf67c 100644 --- a/cpp/tests/copying/concatenate_tests.cpp +++ b/cpp/tests/copying/concatenate_tests.cpp @@ -842,7 +842,7 @@ TEST_F(StructsColumnTest, ConcatenateEmptyStructs) // concatenate auto result = cudf::concatenate(std::vector({*first, *second, *third, *fourth})); - CUDF_EXPECTS(result->size() == expected->size(), "column size changed after concat"); + ASSERT_EQ(result->size(), expected->size()); CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(*result, *expected); } diff --git a/cpp/tests/copying/split_tests.cpp b/cpp/tests/copying/split_tests.cpp index c9a53d6ebe0..c9ff72c2a39 100644 --- a/cpp/tests/copying/split_tests.cpp +++ b/cpp/tests/copying/split_tests.cpp @@ -1229,7 +1229,7 @@ void split_nested_list_of_structs(SplitFunc Split, CompareFunc Compare, bool spl // these outputs correctly, this should be safe. auto result = Split(*outer_list, splits); auto expected = cudf::split(static_cast(*outer_list), splits); - CUDF_EXPECTS(result.size() == expected.size(), "Split result size mismatch"); + ASSERT_EQ(result.size(), expected.size()); for (std::size_t index = 0; index < result.size(); index++) { Compare(expected[index], result[index]); @@ -1591,8 +1591,7 @@ TEST_F(ContiguousSplitUntypedTest, ValidityRepartition) cudf::table_view t({*col}); auto result = cudf::contiguous_split(t, {num_rows / 2}); auto expected = cudf::split(t, {num_rows / 2}); - CUDF_EXPECTS(result.size() == expected.size(), - "Mismatch in split results in ValidityRepartition test"); + ASSERT_EQ(result.size(), expected.size()); for (size_t idx = 0; idx < result.size(); idx++) { CUDF_TEST_EXPECT_TABLES_EQUAL(result[idx].table, expected[idx]); @@ -1696,14 +1695,14 @@ TEST_F(ContiguousSplitStringTableTest, EmptyInputColumn) { std::vector splits; auto result = cudf::contiguous_split(src_table, splits); - CUDF_EXPECTS(result.size() == 1, "Incorrect returned contiguous_split result size!"); + ASSERT_EQ(result.size(), 1); CUDF_TEST_EXPECT_TABLES_EQUIVALENT(src_table, result[0].table); } { auto result = do_chunked_pack(src_table); - CUDF_EXPECTS(result.size() == 1, "Incorrect returned contiguous_split result size!"); + ASSERT_EQ(result.size(), 1); CUDF_TEST_EXPECT_TABLES_EQUIVALENT(src_table, result[0].table); } @@ -1711,7 +1710,7 @@ TEST_F(ContiguousSplitStringTableTest, EmptyInputColumn) { std::vector splits{0, 0, 0, 0}; auto result = cudf::contiguous_split(src_table, splits); - CUDF_EXPECTS(result.size() == 5, "Incorrect returned contiguous_split result size!"); + ASSERT_EQ(result.size(), 5); for (size_t idx = 0; idx < result.size(); idx++) { CUDF_TEST_EXPECT_TABLES_EQUIVALENT(src_table, result[idx].table); diff --git a/cpp/tests/groupby/tdigest_tests.cu b/cpp/tests/groupby/tdigest_tests.cu index 1d2835675f9..97edc1c45a7 100644 --- a/cpp/tests/groupby/tdigest_tests.cu +++ b/cpp/tests/groupby/tdigest_tests.cu @@ -265,7 +265,7 @@ TEST_F(TDigestMergeTest, Grouped) { auto values = cudf::test::generate_standardized_percentile_distribution( cudf::data_type{cudf::type_id::FLOAT64}); - CUDF_EXPECTS(values->size() == 750000, "Unexpected distribution size"); + ASSERT_EQ(values->size(), 750000); // all in the same group auto keys = cudf::make_fixed_width_column( cudf::data_type{cudf::type_id::INT32}, values->size(), cudf::mask_state::UNALLOCATED); @@ -321,7 +321,7 @@ TEST_F(TDigestMergeTest, Grouped) requests.push_back({*merge_input, std::move(aggregations)}); auto result = gb.aggregate(requests); - CUDF_EXPECTS(result.second[0].results[0]->size() == 2, "Unexpected tdigest merge result size"); + ASSERT_EQ(result.second[0].results[0]->size(), 2); cudf::tdigest::tdigest_column_view tdv(*result.second[0].results[0]); // verify centroids @@ -376,7 +376,7 @@ TEST_F(TDigestMergeTest, Grouped) requests.push_back({*merge_input, std::move(aggregations)}); auto result = gb.aggregate(requests); - CUDF_EXPECTS(result.second[0].results[0]->size() == 2, "Unexpected tdigest merge result size"); + ASSERT_EQ(result.second[0].results[0]->size(), 2); cudf::tdigest::tdigest_column_view tdv(*result.second[0].results[0]); // verify centroids @@ -423,7 +423,7 @@ TEST_F(TDigestMergeTest, Grouped) requests.push_back({*merge_input, std::move(aggregations)}); auto result = gb.aggregate(requests); - CUDF_EXPECTS(result.second[0].results[0]->size() == 2, "Unexpected tdigest merge result size"); + ASSERT_EQ(result.second[0].results[0]->size(), 2); cudf::tdigest::tdigest_column_view tdv(*result.second[0].results[0]); // verify centroids diff --git a/cpp/tests/interop/from_arrow_test.cpp b/cpp/tests/interop/from_arrow_test.cpp index 12bc031d56f..9a5cc3733af 100644 --- a/cpp/tests/interop/from_arrow_test.cpp +++ b/cpp/tests/interop/from_arrow_test.cpp @@ -86,8 +86,8 @@ TEST_F(FromArrowTest, DateTimeTable) std::shared_ptr arr; arrow::TimestampBuilder timestamp_builder(arrow::timestamp(arrow::TimeUnit::type::MILLI), arrow::default_memory_pool()); - CUDF_EXPECTS(timestamp_builder.AppendValues(data).ok(), "Failed to append values"); - CUDF_EXPECTS(timestamp_builder.Finish(&arr).ok(), "Failed to build array"); + ASSERT_TRUE(timestamp_builder.AppendValues(data).ok()); + ASSERT_TRUE(timestamp_builder.Finish(&arr).ok()); std::vector> schema_vector({arrow::field("a", arr->type())}); auto schema = std::make_shared(schema_vector); @@ -119,9 +119,8 @@ TYPED_TEST(FromArrowTestDurationsTest, DurationTable) default: CUDF_FAIL("Unsupported duration unit in arrow"); } arrow::DurationBuilder duration_builder(duration(arrow_unit), arrow::default_memory_pool()); - CUDF_EXPECTS(duration_builder.AppendValues(std::vector{1, 2, 3, 4, 5, 6}).ok(), - "Failed to append values"); - CUDF_EXPECTS(duration_builder.Finish(&arr).ok(), "Failed to build array"); + ASSERT_TRUE(duration_builder.AppendValues(std::vector{1, 2, 3, 4, 5, 6}).ok()); + ASSERT_TRUE(duration_builder.Finish(&arr).ok()); std::vector> schema_vector({arrow::field("a", arr->type())}); auto schema = std::make_shared(schema_vector); diff --git a/cpp/tests/interop/to_arrow_test.cpp b/cpp/tests/interop/to_arrow_test.cpp index 6fc9f47f1f8..97d80984272 100644 --- a/cpp/tests/interop/to_arrow_test.cpp +++ b/cpp/tests/interop/to_arrow_test.cpp @@ -188,9 +188,8 @@ TEST_F(ToArrowTest, DateTimeTable) std::shared_ptr arr; arrow::TimestampBuilder timestamp_builder(timestamp(arrow::TimeUnit::type::MILLI), arrow::default_memory_pool()); - CUDF_EXPECTS(timestamp_builder.AppendValues(std::vector{1, 2, 3, 4, 5, 6}).ok(), - "Failed to append values"); - CUDF_EXPECTS(timestamp_builder.Finish(&arr).ok(), "Failed to build array"); + ASSERT_TRUE(timestamp_builder.AppendValues(std::vector{1, 2, 3, 4, 5, 6}).ok()); + ASSERT_TRUE(timestamp_builder.Finish(&arr).ok()); std::vector> schema_vector({arrow::field("a", arr->type())}); auto schema = std::make_shared(schema_vector); @@ -222,9 +221,8 @@ TYPED_TEST(ToArrowTestDurationsTest, DurationTable) default: CUDF_FAIL("Unsupported duration unit in arrow"); } arrow::DurationBuilder duration_builder(duration(arrow_unit), arrow::default_memory_pool()); - CUDF_EXPECTS(duration_builder.AppendValues(std::vector{1, 2, 3, 4, 5, 6}).ok(), - "Failed to append values"); - CUDF_EXPECTS(duration_builder.Finish(&arr).ok(), "Failed to build array"); + ASSERT_TRUE(duration_builder.AppendValues(std::vector{1, 2, 3, 4, 5, 6}).ok()); + ASSERT_TRUE(duration_builder.Finish(&arr).ok()); std::vector> schema_vector({arrow::field("a", arr->type())}); auto schema = std::make_shared(schema_vector); diff --git a/cpp/tests/io/arrow_io_source_test.cpp b/cpp/tests/io/arrow_io_source_test.cpp index fb9e20843ed..c8a4c80934b 100644 --- a/cpp/tests/io/arrow_io_source_test.cpp +++ b/cpp/tests/io/arrow_io_source_test.cpp @@ -97,7 +97,7 @@ TEST_F(ArrowIOTest, S3FileSystem) close_s3_func = reinterpret_cast( dlsym(whole_app, "_ZN5arrow2fs17EnsureS3FinalizedEv")); - if (close_s3_func) { CUDF_EXPECTS(close_s3_func().ok(), "Failed to finalize s3 filesystem"); } + if (close_s3_func) { ASSERT_TRUE(close_s3_func().ok()); } dlclose(whole_app); } } diff --git a/cpp/tests/io/csv_test.cpp b/cpp/tests/io/csv_test.cpp index 9da97c00712..2b501f45b47 100644 --- a/cpp/tests/io/csv_test.cpp +++ b/cpp/tests/io/csv_test.cpp @@ -169,8 +169,7 @@ void check_float_column(cudf::column_view const& col_lhs, CUDF_TEST_EXPECT_COLUMN_PROPERTIES_EQUIVALENT(col_lhs, (wrapper{data.begin(), data.end(), validity})); - CUDF_EXPECTS(col_lhs.null_count() == 0 and col_rhs.null_count() == 0, - "All elements should be valid"); + EXPECT_TRUE(col_lhs.null_count() == 0 and col_rhs.null_count() == 0); EXPECT_THAT(cudf::test::to_host(col_lhs).first, ::testing::Pointwise(FloatNearPointwise(tol), data)); } diff --git a/cpp/tests/io/json_test.cpp b/cpp/tests/io/json_test.cpp index 97d5846294a..5a30be755d3 100644 --- a/cpp/tests/io/json_test.cpp +++ b/cpp/tests/io/json_test.cpp @@ -149,7 +149,7 @@ void check_float_column(cudf::column_view const& col, valid_t const& validity) { CUDF_TEST_EXPECT_COLUMN_PROPERTIES_EQUAL(col, (wrapper{data.begin(), data.end(), validity})); - CUDF_EXPECTS(col.null_count() == 0, "All elements should be valid"); + EXPECT_EQ(col.null_count(), 0); EXPECT_THAT(cudf::test::to_host(col).first, ::testing::Pointwise(FloatNearPointwise(1e-6), data)); } diff --git a/cpp/tests/io/parquet_test.cpp b/cpp/tests/io/parquet_test.cpp index 4e28f536728..3955aadc45d 100644 --- a/cpp/tests/io/parquet_test.cpp +++ b/cpp/tests/io/parquet_test.cpp @@ -213,12 +213,10 @@ void read_footer(std::unique_ptr const& source, auto const ender = reinterpret_cast(ender_buffer->data()); // checks for valid header, footer, and file length - CUDF_EXPECTS(len > header_len + ender_len, "Incorrect data source"); - CUDF_EXPECTS(header->magic == cudf::io::parquet::parquet_magic && - ender->magic == cudf::io::parquet::parquet_magic, - "Corrupted header or footer"); - CUDF_EXPECTS(ender->footer_len != 0 && ender->footer_len <= (len - header_len - ender_len), - "Incorrect footer length"); + ASSERT_GT(len, header_len + ender_len); + ASSERT_TRUE(header->magic == cudf::io::parquet::parquet_magic && + ender->magic == cudf::io::parquet::parquet_magic); + ASSERT_TRUE(ender->footer_len != 0 && ender->footer_len <= (len - header_len - ender_len)); // parquet files end with 4-byte footer_length and 4-byte magic == "PAR1" // seek backwards from the end of the file (footer_length + 8 bytes of ender) @@ -228,7 +226,7 @@ void read_footer(std::unique_ptr const& source, // returns true on success bool res = cp.read(file_meta_data); - CUDF_EXPECTS(res, "Cannot parse file metadata"); + ASSERT_TRUE(res); } // returns the number of bits used for dictionary encoding data at the given page location. @@ -1622,7 +1620,7 @@ TEST_F(ParquetChunkedWriterTest, LargeTables) cudf::io::chunked_parquet_writer_options args = cudf::io::chunked_parquet_writer_options::builder(cudf::io::sink_info{filepath}); auto md = cudf::io::parquet_chunked_writer(args).write(*table1).write(*table2).close(); - CUDF_EXPECTS(!md, "The return value should be null."); + ASSERT_EQ(md, nullptr); cudf::io::parquet_reader_options read_opts = cudf::io::parquet_reader_options::builder(cudf::io::source_info{filepath}); @@ -1653,7 +1651,7 @@ TEST_F(ParquetChunkedWriterTest, ManyTables) writer.write(tbl); }); auto md = writer.close({"dummy/path"}); - CUDF_EXPECTS(md, "The returned metadata should not be null."); + ASSERT_NE(md, nullptr); cudf::io::parquet_reader_options read_opts = cudf::io::parquet_reader_options::builder(cudf::io::source_info{filepath}); @@ -3660,10 +3658,10 @@ TEST_F(ParquetWriterTest, CheckPageRows) cudf::io::parquet::FileMetaData fmd; read_footer(source, &fmd); - CUDF_EXPECTS(fmd.row_groups.size() > 0, "No row groups found"); - CUDF_EXPECTS(fmd.row_groups[0].columns.size() == 1, "Invalid number of columns"); + ASSERT_GT(fmd.row_groups.size(), 0); + ASSERT_EQ(fmd.row_groups[0].columns.size(), 1); auto const& first_chunk = fmd.row_groups[0].columns[0].meta_data; - CUDF_EXPECTS(first_chunk.data_page_offset > 0, "Invalid location for first data page"); + ASSERT_GT(first_chunk.data_page_offset, 0); // read first data page header. sizeof(PageHeader) is not exact, but the thrift encoded // version should be smaller than size of the struct. @@ -3696,10 +3694,10 @@ TEST_F(ParquetWriterTest, CheckPageRowsAdjusted) cudf::io::parquet::FileMetaData fmd; read_footer(source, &fmd); - CUDF_EXPECTS(fmd.row_groups.size() > 0, "No row groups found"); - CUDF_EXPECTS(fmd.row_groups[0].columns.size() == 1, "Invalid number of columns"); + ASSERT_GT(fmd.row_groups.size(), 0); + ASSERT_EQ(fmd.row_groups[0].columns.size(), 1); auto const& first_chunk = fmd.row_groups[0].columns[0].meta_data; - CUDF_EXPECTS(first_chunk.data_page_offset > 0, "Invalid location for first data page"); + ASSERT_GT(first_chunk.data_page_offset, 0); // read first data page header. sizeof(PageHeader) is not exact, but the thrift encoded // version should be smaller than size of the struct. @@ -3967,11 +3965,10 @@ TYPED_TEST(ParquetWriterComparableTypeTest, ThreeColumnSorted) cudf::io::parquet::FileMetaData fmd; read_footer(source, &fmd); - CUDF_EXPECTS(fmd.row_groups.size() > 0, "No row groups found"); + ASSERT_GT(fmd.row_groups.size(), 0); auto const& columns = fmd.row_groups[0].columns; - CUDF_EXPECTS(columns.size() == static_cast(expected.num_columns()), - "Invalid number of columns"); + ASSERT_EQ(columns.size(), static_cast(expected.num_columns())); // now check that the boundary order for chunk 1 is ascending, // chunk 2 is descending, and chunk 3 is unordered diff --git a/cpp/tests/transform/row_bit_count_test.cu b/cpp/tests/transform/row_bit_count_test.cu index 095495456e9..87c275749be 100644 --- a/cpp/tests/transform/row_bit_count_test.cu +++ b/cpp/tests/transform/row_bit_count_test.cu @@ -751,7 +751,7 @@ TEST_F(RowBitCount, EmptyTable) { cudf::table_view empty; auto result = cudf::row_bit_count(empty); - CUDF_EXPECTS(result != nullptr && result->size() == 0, "Expected an empty column"); + ASSERT_TRUE(result != nullptr && result->size() == 0); } { @@ -760,6 +760,6 @@ TEST_F(RowBitCount, EmptyTable) cudf::table_view empty({*strings, *ints}); auto result = cudf::row_bit_count(empty); - CUDF_EXPECTS(result != nullptr && result->size() == 0, "Expected an empty column"); + ASSERT_TRUE(result != nullptr && result->size() == 0); } } diff --git a/cpp/tests/transpose/transpose_test.cpp b/cpp/tests/transpose/transpose_test.cpp index 93cc4aaa100..973d26dffb4 100644 --- a/cpp/tests/transpose/transpose_test.cpp +++ b/cpp/tests/transpose/transpose_test.cpp @@ -146,12 +146,10 @@ void run_test(size_t ncols, size_t nrows, bool add_nulls) auto result = transpose(input_view); auto result_view = std::get<1>(result); - CUDF_EXPECTS(result_view.num_columns() == expected_view.num_columns(), - "Expected same number of columns"); + ASSERT_EQ(result_view.num_columns(), expected_view.num_columns()); for (cudf::size_type i = 0; i < result_view.num_columns(); ++i) { CUDF_TEST_EXPECT_COLUMNS_EQUAL(result_view.column(i), expected_view.column(i)); - CUDF_EXPECTS(result_view.column(i).null_count() == expected_nulls[i], - "Expected correct null count"); + ASSERT_EQ(result_view.column(i).null_count(), expected_nulls[i]); } } From fa4ca1f63dda33ef0f3a6fcde2bd0885b968f82b Mon Sep 17 00:00:00 2001 From: vuule Date: Thu, 3 Aug 2023 13:37:09 -0700 Subject: [PATCH 2/2] expect, not assert --- cpp/tests/io/arrow_io_source_test.cpp | 2 +- cpp/tests/transform/row_bit_count_test.cu | 4 ++-- cpp/tests/transpose/transpose_test.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/tests/io/arrow_io_source_test.cpp b/cpp/tests/io/arrow_io_source_test.cpp index c8a4c80934b..ed297d2da42 100644 --- a/cpp/tests/io/arrow_io_source_test.cpp +++ b/cpp/tests/io/arrow_io_source_test.cpp @@ -97,7 +97,7 @@ TEST_F(ArrowIOTest, S3FileSystem) close_s3_func = reinterpret_cast( dlsym(whole_app, "_ZN5arrow2fs17EnsureS3FinalizedEv")); - if (close_s3_func) { ASSERT_TRUE(close_s3_func().ok()); } + if (close_s3_func) { EXPECT_TRUE(close_s3_func().ok()); } dlclose(whole_app); } } diff --git a/cpp/tests/transform/row_bit_count_test.cu b/cpp/tests/transform/row_bit_count_test.cu index a7438bc13af..236407e62f3 100644 --- a/cpp/tests/transform/row_bit_count_test.cu +++ b/cpp/tests/transform/row_bit_count_test.cu @@ -750,7 +750,7 @@ TEST_F(RowBitCount, EmptyTable) { cudf::table_view empty; auto result = cudf::row_bit_count(empty); - ASSERT_TRUE(result != nullptr && result->size() == 0); + EXPECT_TRUE(result != nullptr && result->size() == 0); } { @@ -759,6 +759,6 @@ TEST_F(RowBitCount, EmptyTable) cudf::table_view empty({*strings, *ints}); auto result = cudf::row_bit_count(empty); - ASSERT_TRUE(result != nullptr && result->size() == 0); + EXPECT_TRUE(result != nullptr && result->size() == 0); } } diff --git a/cpp/tests/transpose/transpose_test.cpp b/cpp/tests/transpose/transpose_test.cpp index 973d26dffb4..cf46dd74138 100644 --- a/cpp/tests/transpose/transpose_test.cpp +++ b/cpp/tests/transpose/transpose_test.cpp @@ -149,7 +149,7 @@ void run_test(size_t ncols, size_t nrows, bool add_nulls) ASSERT_EQ(result_view.num_columns(), expected_view.num_columns()); for (cudf::size_type i = 0; i < result_view.num_columns(); ++i) { CUDF_TEST_EXPECT_COLUMNS_EQUAL(result_view.column(i), expected_view.column(i)); - ASSERT_EQ(result_view.column(i).null_count(), expected_nulls[i]); + EXPECT_EQ(result_view.column(i).null_count(), expected_nulls[i]); } }