[
https://issues.apache.org/jira/browse/MESOS-4112?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Michael Park updated MESOS-4112:
--------------------------------
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.
was:
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.
> Clean up libprocess gtest macros
> --------------------------------
>
> Key: MESOS-4112
> URL: https://issues.apache.org/jira/browse/MESOS-4112
> Project: Mesos
> Issue Type: Task
> Components: libprocess, test
> Reporter: Michael Park
> Assignee: Yong Tang
>
> 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.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
