[jira] [Commented] (MESOS-4112) Clean up libprocess gtest macros
[
https://issues.apache.org/jira/browse/MESOS-4112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15223600#comment-15223600
]
Yong Tang commented on MESOS-4112:
--
Hi [~mcypark], the review requests have been updated. They are now located in:
https://reviews.apache.org/r/45357/
https://reviews.apache.org/r/45664/
https://reviews.apache.org/r/45663/
Please let me know if there are any issues.
> 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__FOR}}
> * {{AWAIT_ASSERT_}} -- default of 15 seconds
> * {{AWAIT_\_FOR}} -- alias for {{AWAIT_ASSERT__FOR}}
> * {{AWAIT_}} -- alias for {{AWAIT_ASSERT_}}
> * {{AWAIT_EXPECT__FOR}}
> * {{AWAIT_EXPECT_}} -- 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)
[jira] [Commented] (MESOS-4112) Clean up libprocess gtest macros
[
https://issues.apache.org/jira/browse/MESOS-4112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15219959#comment-15219959
]
Yong Tang commented on MESOS-4112:
--
Hi, [~mcypark], any update to the review request:
https://reviews.apache.org/r/45356/
https://reviews.apache.org/r/45357/
Comments or feedback would be highly appreciated.
> 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__FOR}}
> * {{AWAIT_ASSERT_}} -- default of 15 seconds
> * {{AWAIT_\_FOR}} -- alias for {{AWAIT_ASSERT__FOR}}
> * {{AWAIT_}} -- alias for {{AWAIT_ASSERT_}}
> * {{AWAIT_EXPECT__FOR}}
> * {{AWAIT_EXPECT_}} -- 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}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-4112) Clean up libprocess gtest macros
[
https://issues.apache.org/jira/browse/MESOS-4112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15213234#comment-15213234
]
Yong Tang commented on MESOS-4112:
--
[~mcypark], thanks for the review.
I updated the usage of macros to replace EQ(true and EQ(false with
..._TRUE/_FALSE. The review request is below:
https://reviews.apache.org/r/45356/
I also added the HTTP response related macros (counterpart of existing macros)
with the review request:
https://reviews.apache.org/r/45357/
Please let me know if there are any issues. Again, thanks for the review!
> 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__FOR}}
> * {{AWAIT_ASSERT_}} -- default of 15 seconds
> * {{AWAIT_\_FOR}} -- alias for {{AWAIT_ASSERT__FOR}}
> * {{AWAIT_}} -- alias for {{AWAIT_ASSERT_}}
> * {{AWAIT_EXPECT__FOR}}
> * {{AWAIT_EXPECT_}} -- 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}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-4112) Clean up libprocess gtest macros
[
https://issues.apache.org/jira/browse/MESOS-4112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15213140#comment-15213140
]
Michael Park commented on MESOS-4112:
-
[~yongtang] Thanks for the patch! I've committed it. Could you follow up with a
patch to use the versions that we added?
We should be able to find all of the instances of them by grepping for
{{EQ(true}} and {{EQ(false}}.
Regarding (3) and (4), I think we should do (3) but leave (4) as-is for now.
> 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__FOR}}
> * {{AWAIT_ASSERT_}} -- default of 15 seconds
> * {{AWAIT_\_FOR}} -- alias for {{AWAIT_ASSERT__FOR}}
> * {{AWAIT_}} -- alias for {{AWAIT_ASSERT_}}
> * {{AWAIT_EXPECT__FOR}}
> * {{AWAIT_EXPECT_}} -- 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}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-4112) Clean up libprocess gtest macros
[
https://issues.apache.org/jira/browse/MESOS-4112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15213134#comment-15213134
]
Michael Park commented on MESOS-4112:
-
{noformat}
commit f978d3ce97ec22fc6f4e52a41ff8e22616c76d55
Author: Yong Tang
Date: Sat Mar 26 13:37:07 2016 -0400
Cleaned up libprocess gtest macros.
1. `AWAIT_EQ_FOR` have be added for completeness.
2. Missing equivalents of `EXPECT_TRUE`/`EXPECT_FALSE` have been added:
- `AWAIT_ASSERT_TRUE_FOR`
- `AWAIT_ASSERT_TRUE`
- `AWAIT_ASSERT_FALSE_FOR`
- `AWAIT_ASSERT_FALSE`
- `AWAIT_EXPECT_TRUE_FOR`
- `AWAIT_EXPECT_FALSE_FOR`
Review: https://reviews.apache.org/r/45070/
{noformat}
> 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__FOR}}
> * {{AWAIT_ASSERT_}} -- default of 15 seconds
> * {{AWAIT_\_FOR}} -- alias for {{AWAIT_ASSERT__FOR}}
> * {{AWAIT_}} -- alias for {{AWAIT_ASSERT_}}
> * {{AWAIT_EXPECT__FOR}}
> * {{AWAIT_EXPECT_}} -- 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}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-4112) Clean up libprocess gtest macros
[
https://issues.apache.org/jira/browse/MESOS-4112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15211921#comment-15211921
]
Yong Tang commented on MESOS-4112:
--
Hi [~mcypark],
Any chance to take a look at the review request
https://reviews.apache.org/r/45070/
or any suggestions on the lib process test macros overall?
Feedbacks are greatly appreciated.
> 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__FOR}}
> * {{AWAIT_ASSERT_}} -- default of 15 seconds
> * {{AWAIT_\_FOR}} -- alias for {{AWAIT_ASSERT__FOR}}
> * {{AWAIT_}} -- alias for {{AWAIT_ASSERT_}}
> * {{AWAIT_EXPECT__FOR}}
> * {{AWAIT_EXPECT_}} -- 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}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-4112) Clean up libprocess gtest macros
[
https://issues.apache.org/jira/browse/MESOS-4112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15202511#comment-15202511
]
Yong Tang commented on MESOS-4112:
--
Hi [~mcypark], I created a review request:
https://reviews.apache.org/r/45070/
and would appreciate if you have a chance to take a look.
In this review request I get the item (1) and (2) in your list done. For item
(3) and (4) I would like your confirmation before I move forward. Let me know
what you think and I will get it done. Thanks!
> 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__FOR}}
> * {{AWAIT_ASSERT_}} -- default of 15 seconds
> * {{AWAIT_\_FOR}} -- alias for {{AWAIT_ASSERT__FOR}}
> * {{AWAIT_}} -- alias for {{AWAIT_ASSERT_}}
> * {{AWAIT_EXPECT__FOR}}
> * {{AWAIT_EXPECT_}} -- 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}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
