Details
-
Task
-
Status: Resolved
-
Major
-
Resolution: Fixed
-
None
-
None
-
Mesosphere Sprint 32
-
2
Description
This ticket is regarding the libprocess gtest helpers in 3rdparty/libprocess/include/process/gtest.hpp.
The pattern in this file seems to be a set of macros:
- AWAIT_ASSERT_<STATE>_FOR
- AWAIT_ASSERT_<STATE> – default of 15 seconds
- AWAIT_<STATE>_FOR – alias for AWAIT_ASSERT_<STATE>_FOR
- AWAIT_<STATE> – alias for AWAIT_ASSERT_<STATE>
- AWAIT_EXPECT_<STATE>_FOR
- AWAIT_EXPECT_<STATE> – default of 15 seconds
(1) AWAIT_EQ_FOR should be added for completeness.
(2) In gtest, we've got EXPECT_EQ as well as the bool-specific versions: EXPECT_TRUE and EXPECT_FALSE.
We should adopt this pattern in these helpers as well. Keeping the pattern above in mind, the following are missing:
- AWAIT_ASSERT_TRUE_FOR
- AWAIT_ASSERT_TRUE
- AWAIT_ASSERT_FALSE_FOR
- AWAIT_ASSERT_FALSE
- AWAIT_EXPECT_TRUE_FOR
- AWAIT_EXPECT_FALSE_FOR
(3) There are HTTP response related macros at the bottom of the file, e.g. AWAIT_EXPECT_RESPONSE_STATUS_EQ, however these are missing their ASSERT counterparts.
(4) The reason for (3) presumably is because we reach for EXPECT over ASSERT in general due to the test suite crashing behavior of ASSERT. If this is the case, it would be worthwhile considering whether macros such as AWAIT_READY should alias AWAIT_EXPECT_READY rather than AWAIT_ASSERT_READY.
(5) There are a few more missing macros, given AWAIT_EQ_FOR and AWAIT_EQ which aliases to AWAIT_ASSERT_EQ_FOR and AWAIT_ASSERT_EQ respectively, we should also add AWAIT_TRUE_FOR, AWAIT_TRUE, AWAIT_FALSE_FOR, and AWAIT_FALSE as well.