Re: Review Request 66227: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operator API.

[Chun-Hung Hsiao <ch...@apache.org>]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66227/#review202237
-----------------------------------------------------------


Fix it, then Ship it!





src/tests/api_tests.cpp
Lines 3684 (patched)
<https://reviews.apache.org/r/66227/#comment283985>

    Suggestion: Test growing a persistent volume through the master operator API.



src/tests/api_tests.cpp
Lines 3685 (patched)
<https://reviews.apache.org/r/66227/#comment283986>

    Suggestion: This test is disabled ...
    
    Also, is it true that persistent volumes are not supported on Windows yet? If so, why is the `CreateAndDestroyVolumes` test not disabled on Windows?



src/tests/api_tests.cpp
Lines 3688 (patched)
<https://reviews.apache.org/r/66227/#comment283987>

    Since we don't specify any special flag for this test, this is not required. The default `StartMaster()` implementation will call the overridden `MasterAPITest::CreateMasterFlags()` to use the long allocation interval.



src/tests/api_tests.cpp
Lines 3700 (patched)
<https://reviews.apache.org/r/66227/#comment283988>

    s/`Static`/`static`/



src/tests/api_tests.cpp
Lines 3745 (patched)
<https://reviews.apache.org/r/66227/#comment283989>

    Missing a period at the end of the comment. See: http://mesos.apache.org/documentation/latest/c++-style-guide/#comments



src/tests/api_tests.cpp
Lines 3787-3793 (patched)
<https://reviews.apache.org/r/66227/#comment283995>

    Alternatively, we could do:
    ```
    RepeatedPtrField<Resource> agentResources = devolve<Resource>(
        v1GetAgentsResponse->get_agents().agents(0).total_resources());
    upgradeResources(&agentResources);
    ```



src/tests/api_tests.cpp
Lines 3801 (patched)
<https://reviews.apache.org/r/66227/#comment283996>

    Suggestion: Test shrinking a persistent volume through the master operator API.



src/tests/api_tests.cpp
Lines 3802 (patched)
<https://reviews.apache.org/r/66227/#comment283997>

    Suggestion: This test is disabled ...
    
    Also, is it true that persistent volumes are not supported on Windows yet? If so, why is the `CreateAndDestroyVolumes` test not disabled on Windows?



src/tests/api_tests.cpp
Lines 3805 (patched)
<https://reviews.apache.org/r/66227/#comment283999>

    This is not required.



src/tests/api_tests.cpp
Lines 3817 (patched)
<https://reviews.apache.org/r/66227/#comment283998>

    s/`Static`/`static`/



src/tests/api_tests.cpp
Lines 3902-3908 (patched)
<https://reviews.apache.org/r/66227/#comment284000>

    Alternatively, we could do:
    ```
    RepeatedPtrField<Resource> agentResources = devolve<Resource>(
        v1GetAgentsResponse->get_agents().agents(0).total_resources());
    upgradeResources(&agentResources);
    ```


- Chun-Hung Hsiao


On April 23, 2018, 8:24 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66227/
> -----------------------------------------------------------
> 
> (Updated April 23, 2018, 8:24 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-8747
>     https://issues.apache.org/jira/browse/MESOS-8747
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operator API.
> 
> 
> Diffs
> -----
> 
>   src/tests/api_tests.cpp dd8e221d8fd1b2a241505345337897e4ee4a6347 
> 
> 
> Diff: https://reviews.apache.org/r/66227/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>