[
https://issues.apache.org/jira/browse/MESOS-6181?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15524145#comment-15524145
]
Greg Mann edited comment on MESOS-6181 at 9/26/16 8:57 PM:
-----------------------------------------------------------
[~gyliu], sorry for my delayed reply! Regarding the use of '{' and '}' to
enclose sections of the test code, I like to do this purely for readability.
i.e., instead of re-assigning the value of {{volume}} several times in a test,
it can be nice to declare the variable each time, enclosing it in a scope to
make it obvious where that volume is used, and where it goes out of scope. If
we use a single variable and re-assign its value multiple times in the test
without using scopes, I think it's more difficult to tell what the variable
contains at a given point in time.
However, this particular case isn't a very compelling use of this pattern, I'd
say. Unfortunately, we can't enclose the second volume-creation in a scope,
since we reference that volume at the end of the test when we call
{{EXPECT_TRUE(Resources(offer.resources()).contains(volume));}}.
Nonetheless, I believe the logic in the test is correct. Regarding your point
#1, the driver doesn't decline a non-existent offer, since the {{offer}}
variable is declared outside of the '{}' scope, it can be assigned a new value
within the scope and that value will still be accessible after the scope is
exited. Regarding point #2, the tests do actually test to make sure the final
offer contains the volume, using
{{EXPECT_TRUE(Resources(offer.resources()).contains(volume));}}.
What do you think?
was (Author: greggomann):
[~gyliu], sorry for my delayed reply! Regarding the use of `{` and `}` to
enclose sections of the test code, I like to do this purely for readability.
i.e., instead of re-assigning the value of {{volume}} several times in a test,
it can be nice to declare the variable each time, enclosing it in a scope to
make it obvious where that volume is used, and where it goes out of scope. If
we use a single variable and re-assign its value multiple times in the test
without using scopes, I think it's more difficult to tell what the variable
contains at a given point in time.
However, this particular case isn't a very compelling use of this pattern, I'd
say. Unfortunately, we can't enclose the second volume-creation in a scope,
since we reference that volume at the end of the test when we call
{{EXPECT_TRUE(Resources(offer.resources()).contains(volume));}}.
Nonetheless, I believe the logic in the test is correct. Regarding your point
#1, the driver doesn't decline a non-existent offer, since the {{offer}}
variable is declared outside of the `{}` scope, it can be assigned a new value
within the scope and that value will still be accessible after the scope is
exited. Regarding point #2, the tests do actually test to make sure the final
offer contains the volume, using
{{EXPECT_TRUE(Resources(offer.resources()).contains(volume));}}.
What do you think?
> The logic for BadACLNoPrincipal and BadACLDropCreateAndDestroy is not correct
> -----------------------------------------------------------------------------
>
> Key: MESOS-6181
> URL: https://issues.apache.org/jira/browse/MESOS-6181
> Project: Mesos
> Issue Type: Bug
> Reporter: Guangya Liu
> Assignee: Guangya Liu
>
> Two issues for those two test cases:
> 1) No need to add `{}` in the test case as there is no need to add `{}`,
> adding the `{}` will cause the driver decline a non exist offer.
> 2) If destroy volume failed, we should get the last offer to make sure that
> the last offer also contain the volume resource.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
