Port OrderedHashMap tests to doctest #41117
Conversation
tests/test_ordered_hash_map.h
Outdated
| } | ||
|
|
||
| bool test_iteration(OrderedHashMap<int, int> &map) { |
There was a problem hiding this comment.
This function make sense only when used by TEST_CASE("[OrderedHashMap] Iteration") {. I wonder if you should put directly into that function.
tests/test_ordered_hash_map.h
Outdated
| CHECK(test_iteration(map)); | ||
| } | ||
|
|
||
| bool test_const_iteration(const OrderedHashMap<int, int> &map) { |
There was a problem hiding this comment.
As the above function, I wonder if make more sense put this into the TEST_CASE("[OrderedHashMap] Const iteration") { directly.
There was a problem hiding this comment.
Look at ClassDB tests: #40980, there are many existing method calls like that. It's ok to call into methods within test cases. But I think it would be better if assertions are written closer to actual checks.
There was a problem hiding this comment.
Moved code from each method into respective test case 👍
tests/test_ordered_hash_map.h
Outdated
| if (expected[idx] != Pair<int, int>(E.key(), E.value())) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Something like that:
| if (expected[idx] != Pair<int, int>(E.key(), E.value())) { | |
| return false; | |
| } | |
| CHECK(expected[idx] != Pair<int, int>(E.key(), E.value())); |
The method could be converted to void instead of returning bool.
You can also use logging methods like CAPTURE, INFO and MESSAGE, to inspect those indices for instance. See doctest parameterized tests.
There was a problem hiding this comment.
Yep, it could probably be simplified that way. I'll see what I can do, thanks!
|
Thanks! |
No description provided.