Re: Review Request 37735: Refactored filesystem isolator tests to allow multiple rootfses.

[Jie Yu]

> On Aug. 26, 2015, 6:10 p.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/filesystem_isolator_tests.cpp, lines 152-158
> > 
> >
> > 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.

[Jiang Yan Xu]

---
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)


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)


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



src/tests/containerizer/filesystem_isolator_tests.cpp (lines 152 - 158)


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)


"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
> 
>

Review Request 37735: Refactored filesystem isolator tests to allow multiple rootfses.

[Jie Yu]

---
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