You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2015/08/25 01:00:45 UTC
Review Request 37735: Refactored filesystem isolator tests to allow
multiple rootfses.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37735/
-----------------------------------------------------------
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
Re: Review Request 37735: Refactored filesystem isolator tests to
allow multiple rootfses.
Posted by Jie Yu <yu...@gmail.com>.
> On Aug. 26, 2015, 6:10 p.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/filesystem_isolator_tests.cpp, lines 152-158
> > <https://reviews.apache.org/r/37735/diff/1/?file=1048780#file1048780line152>
> >
> > 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.
Added a TODO,
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37735/#review96447
-----------------------------------------------------------
On Aug. 24, 2015, 11 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, 11 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
>
>
Re: Review Request 37735: Refactored filesystem isolator tests to
allow multiple rootfses.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
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
>
>