mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jiang Yan Xu" <...@jxu.me>
Subject Re: Review Request 37735: Refactored filesystem isolator tests to allow multiple rootfses.
Date Wed, 26 Aug 2015 18:10:26 GMT

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

Ship it!



src/tests/containerizer/filesystem_isolator_tests.cpp (line 76)
<https://reviews.apache.org/r/37735/#comment151822>

    Unfortunately I realized this after [this](https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L1138)
is checked in, but appc is not (or, no longer is, I don't know) stylized as "AppC" so here
we can just use `TestAppcProvisioner` to avoid the somewhat weird looking CamelCasing.
    
    Here and elsewhere.
    
    Also s/provision/provisions/



src/tests/containerizer/filesystem_isolator_tests.cpp (line 86)
<https://reviews.apache.org/r/37735/#comment151827>

    s/"rootfs"/"rootfses"/ because this is the root dir for multiple rootfses.



src/tests/containerizer/filesystem_isolator_tests.cpp (lines 152 - 158)
<https://reviews.apache.org/r/37735/#comment152090>

    Rename CREATE_VOLUME to CREATE_VOLUME_FROM_HOST instead?
    
    CREATE_VOLUME only has two usage instances outside this test so it should be trivial.
    
    A TODO is fine.



src/tests/containerizer/filesystem_isolator_tests.cpp (line 528)
<https://reviews.apache.org/r/37735/#comment152087>

    "test_image" is used twice in this test without a common variable. It's not immediately
clear that the two instances should match.
    
    Of course the reader is expected to know this, but the more explicit the better, right?


- Jiang Yan Xu


On Aug. 24, 2015, 4 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37735/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 4 p.m.)
> 
> 
> Review request for mesos, Timothy Chen, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Refactored filesystem isolator tests to allow multiple rootfses.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 7003b03f1da2fee53592bc23799f59eabcd913a2

>   src/tests/containerizer/provisioner.hpp c4ba46794fe5d7875fda11155367f521c34ea339 
> 
> Diff: https://reviews.apache.org/r/37735/diff/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


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