mesos-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Greg Mann (JIRA)" <>
Subject [jira] [Commented] (MESOS-6181) The logic for BadACLNoPrincipal and BadACLDropCreateAndDestroy is not correct
Date Mon, 26 Sep 2016 20:57:20 GMT


Greg Mann commented on MESOS-6181:

[~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:
>             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

View raw message