mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chun-Hung Hsiao <chhs...@apache.org>
Subject Re: Review Request 66569: Added a test to verify that grow and shrink cannot be combined.
Date Mon, 16 Apr 2018 22:05:19 GMT

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




src/tests/persistent_volume_tests.cpp
Lines 691-695 (patched)
<https://reviews.apache.org/r/66569/#comment282236>

    Since this test just verifies that the grow and shrink operations cannot be combined with
other operations, I was wondering if there's a way to check this without worry about the different
behaviors produced by PATH or MOUNT.
    
    Alternatively, if you take my suggestion to create another `PersistentVolumeResizeTest`
fixture in the previous patch, we can move this test into that fixture.



src/tests/persistent_volume_tests.cpp
Lines 702 (patched)
<https://reviews.apache.org/r/66569/#comment282405>

    No need for this.



src/tests/persistent_volume_tests.cpp
Lines 728 (patched)
<https://reviews.apache.org/r/66569/#comment282407>

    No need for this if the clock is already paused.



src/tests/persistent_volume_tests.cpp
Lines 732 (patched)
<https://reviews.apache.org/r/66569/#comment282227>

    Let's move this to the beginning of this test to make it clear that this test pauses the
clock.



src/tests/persistent_volume_tests.cpp
Lines 775-781 (patched)
<https://reviews.apache.org/r/66569/#comment282411>

    As discussed, I'm not sure if this is the semantics we want to enforce. If we end up rejecting
the whole `ACCEPT` call, then we could simplify the test by:
    
    1. Starting with a persistent volume, some free disk and some CPU.
    2. Open receiving an offer, accept it with reserving the CPU and growing the persistent
volume. Different type of resources are used here to make sure it's not just for preventing
pipelining.
    3. Verify that the `ACCEPT` call fails by checking that the next offer contains the original
resources.



src/tests/persistent_volume_tests.cpp
Lines 791 (patched)
<https://reviews.apache.org/r/66569/#comment282412>

    No need for this.



src/tests/persistent_volume_tests.cpp
Lines 814-861 (patched)
<https://reviews.apache.org/r/66569/#comment282413>

    How about moving the `SHRINK_VOLUME` test into another test? Let's keep the test focused.


- Chun-Hung Hsiao


On April 11, 2018, 9:19 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66569/
> -----------------------------------------------------------
> 
> (Updated April 11, 2018, 9:19 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-4965
>     https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a test to verify that grow and shrink cannot be combined.
> 
> 
> Diffs
> -----
> 
>   src/tests/persistent_volume_tests.cpp 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66569/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message