Skip to content

Commit

Permalink
Remove macros that inspect the contents of exceptions (#12076)
Browse files Browse the repository at this point in the history
We should not be encouraging users to rely specific error messages. Anywhere that is currently doing so is likely an indication that libcudf should be throwing a more specific type of exception instead of just a `cudf::logic_error`. This PR removes the testing utilities that were previously used for this purpose and reworks the relevant tests.

Related to #10200.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - David Wendt (https://github.com/davidwendt)
  - Bradley Dice (https://github.com/bdice)
  - Karthikeyan (https://github.com/karthikeyann)

URL: #12076
  • Loading branch information
vyasr authored Nov 8, 2022
1 parent 2ced214 commit b16b4ff
Show file tree
Hide file tree
Showing 13 changed files with 124 additions and 187 deletions.
10 changes: 9 additions & 1 deletion cpp/doxygen/developer_guide/DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,14 @@ Functions like merge or groupby in libcudf make no guarantees about the order of
Promising deterministic ordering is not, in general, conducive to fast parallel algorithms.
Calling code is responsible for performing sorts after the fact if sorted outputs are needed.

## libcudf does not promise specific exception messages

libcudf documents the exceptions that will be thrown by an API for different kinds of invalid inputs.
The types of those exceptions (e.g. `cudf::logic_error`) are part of the public API.
However, the explanatory string returned by the `what` method of those exceptions is not part of the API and is subject to change.
Calling code should not rely on the contents of libcudf error messages to determine the nature of the error.
For information on the types of exceptions that libcudf throws under different circumstances, see the [section on error handling](#errors).

# libcudf API and Implementation

## Streams
Expand Down Expand Up @@ -837,7 +845,7 @@ description of what has broken from the past release. Label pull requests that c
with the "non-breaking" tag.


# Error Handling
# Error Handling {#errors}

libcudf follows conventions (and provides utilities) enforcing compile-time and run-time
conditions and detecting and handling CUDA errors. Communication of errors is always via C++
Expand Down
52 changes: 0 additions & 52 deletions cpp/include/cudf_test/cudf_gtest.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,58 +110,6 @@ struct TypeList<Types<TYPES...>> {
*/
#define EXPECT_CUDA_SUCCEEDED(expr) EXPECT_EQ(cudaSuccess, expr)

/**
* @brief Utility for testing the expectation that an expression x throws the specified
* exception whose what() message ends with the msg
*
* @param x The expression to test
* @param exception The exception type to test for
* @param startswith The start of the expected message
* @param endswith The end of the expected message
*/
#define EXPECT_THROW_MESSAGE(x, exception, startswith, endswith) \
do { \
EXPECT_THROW( \
{ \
try { \
x; \
} catch (const exception& e) { \
ASSERT_NE(nullptr, e.what()); \
EXPECT_THAT(e.what(), testing::StartsWith((startswith))); \
EXPECT_THAT(e.what(), testing::EndsWith((endswith))); \
throw; \
} \
}, \
exception); \
} while (0)

/**
* @brief test macro to be expected to throw cudf::logic_error with a message
*
* @param x The statement to be tested
* @param msg The message associated with the exception
*/
#define CUDF_EXPECT_THROW_MESSAGE(x, msg) \
EXPECT_THROW_MESSAGE(x, cudf::logic_error, "cuDF failure at:", msg)

/**
* @brief test macro to be expected to throw cudf::cuda_error with a message
*
* @param x The statement to be tested
* @param msg The message associated with the exception
*/
#define CUDA_EXPECT_THROW_MESSAGE(x, msg) \
EXPECT_THROW_MESSAGE(x, cudf::cuda_error, "CUDA error encountered at:", msg)

/**
* @brief test macro to be expected to throw cudf::fatal_logic_error with a message
*
* @param x The statement to be tested
* @param msg The message associated with the exception
*/
#define FATAL_CUDA_EXPECT_THROW_MESSAGE(x, msg) \
EXPECT_THROW_MESSAGE(x, cudf::fatal_cuda_error, "Fatal CUDA error encountered at:", msg)

/**
* @brief test macro to be expected as no exception.
*
Expand Down
5 changes: 3 additions & 2 deletions cpp/tests/copying/get_value_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,9 @@ TYPED_TEST(FixedWidthGetValueTest, IndexOutOfBounds)
{
fixed_width_column_wrapper<TypeParam, int32_t> col({9, 8, 7, 6}, {0, 1, 0, 1});

CUDF_EXPECT_THROW_MESSAGE(get_element(col, -1);, "Index out of bounds");
CUDF_EXPECT_THROW_MESSAGE(get_element(col, 4);, "Index out of bounds");
// Test for out of bounds indexes in both directions.
EXPECT_THROW(get_element(col, -1), cudf::logic_error);
EXPECT_THROW(get_element(col, 4), cudf::logic_error);
}

struct StringGetValueTest : public BaseFixture {
Expand Down
27 changes: 16 additions & 11 deletions cpp/tests/copying/segmented_gather_list_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -576,26 +576,31 @@ TEST_F(SegmentedGatherTestFloat, Fails)
cudf::test::strings_column_wrapper nonlist_map1{"1", "2", "0", "1"};
LCW<cudf::string_view> nonlist_map2{{"1", "2", "0", "1"}};

CUDF_EXPECT_THROW_MESSAGE(
// Input must be a list of integer indices. It should fail for integers,
// strings, or lists containing anything other than integers.
EXPECT_THROW(
cudf::lists::detail::segmented_gather(lists_column_view{list}, lists_column_view{nonlist_map0}),
"lists_column_view only supports lists");
cudf::logic_error);

CUDF_EXPECT_THROW_MESSAGE(
EXPECT_THROW(
cudf::lists::detail::segmented_gather(lists_column_view{list}, lists_column_view{nonlist_map1}),
"lists_column_view only supports lists");
cudf::logic_error);

CUDF_EXPECT_THROW_MESSAGE(
EXPECT_THROW(
cudf::lists::detail::segmented_gather(lists_column_view{list}, lists_column_view{nonlist_map2}),
"Gather map should be list column of index type");
cudf::logic_error);

auto valids =
cudf::detail::make_counting_transform_iterator(0, [](auto i) { return i % 2 == 0; });
LCW<int8_t> nulls_map{{{3, 2, 1, 0}, {0}, {0}, {0, 1}}, valids};
CUDF_EXPECT_THROW_MESSAGE(

// Nulls are not supported in the gather map.
EXPECT_THROW(
cudf::lists::detail::segmented_gather(lists_column_view{list}, lists_column_view{nulls_map}),
"Gather map contains nulls");
cudf::logic_error);

CUDF_EXPECT_THROW_MESSAGE(cudf::lists::detail::segmented_gather(
lists_column_view{list}, lists_column_view{size_mismatch_map}),
"Gather map and list column should be same size");
// Gather map and list column sizes must be the same.
EXPECT_THROW(cudf::lists::detail::segmented_gather(lists_column_view{list},
lists_column_view{size_mismatch_map}),
cudf::logic_error);
}
21 changes: 2 additions & 19 deletions cpp/tests/error/error_handling_test.cu
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,10 @@ TEST(ExpectsTest, FalseCondition)

TEST(ExpectsTest, TrueCondition) { EXPECT_NO_THROW(CUDF_EXPECTS(true, "condition is true")); }

TEST(ExpectsTest, TryCatch)
{
CUDF_EXPECT_THROW_MESSAGE(CUDF_EXPECTS(false, "test reason"), "test reason");
}

TEST(CudaTryTest, Error)
{
CUDA_EXPECT_THROW_MESSAGE(CUDF_CUDA_TRY(cudaErrorLaunchFailure),
"cudaErrorLaunchFailure unspecified launch failure");
}
TEST(CudaTryTest, Error) { EXPECT_THROW(CUDF_CUDA_TRY(cudaErrorLaunchFailure), cudf::cuda_error); }

TEST(CudaTryTest, Success) { EXPECT_NO_THROW(CUDF_CUDA_TRY(cudaSuccess)); }

TEST(CudaTryTest, TryCatch)
{
CUDA_EXPECT_THROW_MESSAGE(CUDF_CUDA_TRY(cudaErrorMemoryAllocation),
"cudaErrorMemoryAllocation out of memory");
}

TEST(StreamCheck, success) { EXPECT_NO_THROW(CUDF_CHECK_CUDA(0)); }

namespace {
Expand Down Expand Up @@ -79,9 +64,7 @@ TEST(StreamCheck, CatchFailedKernel)
#ifndef NDEBUG
stream.synchronize();
#endif
CUDA_EXPECT_THROW_MESSAGE(CUDF_CHECK_CUDA(stream.value()),
"cudaErrorInvalidConfiguration "
"invalid configuration argument");
EXPECT_THROW(CUDF_CHECK_CUDA(stream.value()), cudf::cuda_error);
}

__global__ void kernel() { asm("trap;"); }
Expand Down
26 changes: 14 additions & 12 deletions cpp/tests/groupby/count_scan_tests.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, NVIDIA CORPORATION.
* Copyright (c) 2021-2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -53,9 +53,10 @@ TYPED_TEST(groupby_count_scan_test, basic)
result_wrapper expect_vals{0, 1, 2, 0, 1, 2, 3, 0, 1, 2};
// clang-format on

// Count groupby aggregation is only supported with null_policy::EXCLUDE
auto agg1 = cudf::make_count_aggregation<groupby_scan_aggregation>();
CUDF_EXPECT_THROW_MESSAGE(test_single_scan(keys, vals, expect_keys, expect_vals, std::move(agg1)),
"Unsupported groupby scan aggregation");
EXPECT_THROW(test_single_scan(keys, vals, expect_keys, expect_vals, std::move(agg1)),
cudf::logic_error);

auto agg2 = cudf::make_count_aggregation<groupby_scan_aggregation>(null_policy::INCLUDE);
test_single_scan(keys, vals, expect_keys, expect_vals, std::move(agg2));
Expand Down Expand Up @@ -181,13 +182,13 @@ TYPED_TEST(FixedPointTestAllReps, GroupByCountScan)
auto const expect_vals = result_wrapper{0, 1, 2, 0, 1, 2, 3, 0, 1, 2};
// clang-format on

CUDF_EXPECT_THROW_MESSAGE(
test_single_scan(keys,
vals,
expect_keys,
expect_vals,
cudf::make_count_aggregation<groupby_scan_aggregation>()),
"Unsupported groupby scan aggregation");
// Count groupby aggregation is only supported with null_policy::EXCLUDE
EXPECT_THROW(test_single_scan(keys,
vals,
expect_keys,
expect_vals,
cudf::make_count_aggregation<groupby_scan_aggregation>()),
cudf::logic_error);

auto agg2 = cudf::make_count_aggregation<groupby_scan_aggregation>(null_policy::INCLUDE);
test_single_scan(keys, vals, expect_keys, expect_vals, std::move(agg2));
Expand All @@ -209,9 +210,10 @@ TEST_F(groupby_dictionary_count_scan_test, basic)
result_wrapper expect_vals{0, 0, 0, 1, 0, 1};
// clang-format on

// Count groupby aggregation is only supported with null_policy::EXCLUDE
auto agg1 = cudf::make_count_aggregation<groupby_scan_aggregation>();
CUDF_EXPECT_THROW_MESSAGE(test_single_scan(keys, vals, expect_keys, expect_vals, std::move(agg1)),
"Unsupported groupby scan aggregation");
EXPECT_THROW(test_single_scan(keys, vals, expect_keys, expect_vals, std::move(agg1)),
cudf::logic_error);
test_single_scan(keys,
vals,
expect_keys,
Expand Down
7 changes: 3 additions & 4 deletions cpp/tests/groupby/keys_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,11 @@ TYPED_TEST(groupby_keys_test, mismatch_num_rows)
fixed_width_column_wrapper<K> keys{1, 2, 3};
fixed_width_column_wrapper<V> vals{0, 1, 2, 3, 4};

// Verify that scan throws an error when given data of mismatched sizes.
auto agg = cudf::make_count_aggregation<groupby_aggregation>();
CUDF_EXPECT_THROW_MESSAGE(test_single_agg(keys, vals, keys, vals, std::move(agg)),
"Size mismatch between request values and groupby keys.");
EXPECT_THROW(test_single_agg(keys, vals, keys, vals, std::move(agg)), cudf::logic_error);
auto agg2 = cudf::make_count_aggregation<groupby_scan_aggregation>();
CUDF_EXPECT_THROW_MESSAGE(test_single_scan(keys, vals, keys, vals, std::move(agg2)),
"Size mismatch between request values and groupby keys.");
EXPECT_THROW(test_single_scan(keys, vals, keys, vals, std::move(agg2)), cudf::logic_error);
}

template <typename T>
Expand Down
113 changes: 54 additions & 59 deletions cpp/tests/groupby/rank_scan_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -508,65 +508,60 @@ TEST_F(groupby_rank_scan_test_failures, DISABLED_test_exception_triggers)
auto const keys = input<T>{{1, 2, 3}, null_at(2)};
auto const col = input<T>{3, 3, 1};

CUDF_EXPECT_THROW_MESSAGE(
test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::DENSE),
null_policy::INCLUDE,
sorted::NO),
"Rank aggregate in groupby scan requires the keys to be presorted");

CUDF_EXPECT_THROW_MESSAGE(
test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::MIN),
null_policy::INCLUDE,
sorted::NO),
"Rank aggregate in groupby scan requires the keys to be presorted");

CUDF_EXPECT_THROW_MESSAGE(
test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::DENSE),
null_policy::EXCLUDE,
sorted::YES),
"Rank aggregate in groupby scan requires the keys to be presorted");

CUDF_EXPECT_THROW_MESSAGE(
test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::MIN),
null_policy::EXCLUDE,
sorted::YES),
"Rank aggregate in groupby scan requires the keys to be presorted");

CUDF_EXPECT_THROW_MESSAGE(
test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::DENSE),
null_policy::EXCLUDE,
sorted::NO),
"Rank aggregate in groupby scan requires the keys to be presorted");

CUDF_EXPECT_THROW_MESSAGE(
test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::MIN),
null_policy::EXCLUDE,
sorted::NO),
"Rank aggregate in groupby scan requires the keys to be presorted");
// All of these aggregations raise exceptions unless provided presorted keys
EXPECT_THROW(test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::DENSE),
null_policy::INCLUDE,
sorted::NO),
cudf::logic_error);

EXPECT_THROW(test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::MIN),
null_policy::INCLUDE,
sorted::NO),
cudf::logic_error);

EXPECT_THROW(test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::DENSE),
null_policy::EXCLUDE,
sorted::YES),
cudf::logic_error);

EXPECT_THROW(test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::MIN),
null_policy::EXCLUDE,
sorted::YES),
cudf::logic_error);

EXPECT_THROW(test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::DENSE),
null_policy::EXCLUDE,
sorted::NO),
cudf::logic_error);

EXPECT_THROW(test_single_scan(keys,
col,
keys,
col,
make_rank_aggregation<groupby_scan_aggregation>(rank_method::MIN),
null_policy::EXCLUDE,
sorted::NO),
cudf::logic_error);
}

} // namespace test
Expand Down
6 changes: 3 additions & 3 deletions cpp/tests/io/json_tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -750,9 +750,9 @@ TEST_F(JsonTest, TreeRepresentationError)
cudf::io::json::detail::get_token_stream(d_input, options, stream);

// Get the JSON's tree representation
CUDF_EXPECT_THROW_MESSAGE(
cuio_json::detail::get_tree_representation(tokens_gpu, token_indices_gpu, stream),
"JSON Parser encountered an invalid format at location 6");
// This JSON is invalid and will raise an exception.
EXPECT_THROW(cuio_json::detail::get_tree_representation(tokens_gpu, token_indices_gpu, stream),
cudf::logic_error);
}

/**
Expand Down
8 changes: 4 additions & 4 deletions cpp/tests/io/nested_json_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -588,11 +588,11 @@ TEST_P(JsonParserTest, ExpectFailMixStructAndList)
R"( [{"a":[123, {"0": 123}], "b":1.0}, {"b":1.1}, {"b":2.1}] )",
R"( [{"a":[123, "123"], "b":1.0}, {"b":1.1}, {"b":2.1}] )"};

// libcudf does not currently support a mix of lists and structs.
for (auto const& input : inputs_fail) {
CUDF_EXPECT_THROW_MESSAGE(
auto const cudf_table = json_parser(
cudf::host_span<SymbolT const>{input.data(), input.size()}, options, stream, mr),
"A mix of lists and structs within the same column is not supported");
EXPECT_THROW(auto const cudf_table = json_parser(
cudf::host_span<SymbolT const>{input.data(), input.size()}, options, stream, mr),
cudf::logic_error);
}

for (auto const& input : inputs_succeed) {
Expand Down
Loading

0 comments on commit b16b4ff

Please sign in to comment.