OptionCategory: Add missing implementations, uncovered tests. #12011
Conversation
Enum OptionCategory was introduced in cvc5#11931, which did not include implementations for `std::to_string(cvc5::modes::OptionCategory)` and `cvc5_modes_option_category_to_str(Cvc5OptionCategory)`. This adds implementations for these functions and further adds uncovered tests for C, Python and Java (fixes coverage build in nightlies).
| for (int32_t c = static_cast<int32_t>(CVC5_OPTION_CATEGORY_REGULAR); | ||
| c < static_cast<int32_t>(CVC5_OPTION_CATEGORY_LAST); | ||
| ++c) | ||
| { | ||
| std::string str = cvc5_modes_option_category_to_string( | ||
| static_cast<Cvc5OptionCategory>(c)); | ||
| ASSERT_NE(str, "?"); | ||
| } |
There was a problem hiding this comment.
I understand this is an easy way to test that the function "works" for all the values, and it's also future-proof in the sense that it doesn't require updating if more enum values are added later. However, it would be more robust to print all enum values as a string and check against an expected string with ASSERT_EQ, as we do for the other enum types. Please consider this option.
There was a problem hiding this comment.
We don't test for all enum values but just a single value for other enums. I don't have a particular preference. We could actually refactor the tests for the other enums in modes to the same. Note that in the uncovered test we test for a single specific enum value.
There was a problem hiding this comment.
You are right, it's a single value for the other enums. I suppose my point was that testing for "not equal to ?" is a weaker check than explicitly verifying equality with an expected value. That’s the part I didn't really like. On the other hand, I liked that you tested all possible values. Since this isn't a critical issue, we can think it over a bit more and handle the refactor later.
Right, I'd rather do this in an extra PR.
Right, @netolcc06 is supposed to take care of this. |
Enum OptionCategory was introduced in #11931, which did not include implementations for
std::to_string(cvc5::modes::OptionCategory)andcvc5_modes_option_category_to_str(Cvc5OptionCategory).This adds implementations for these functions and further adds uncovered tests for C, Python and Java (fixes coverage build in nightlies).