> On Dec. 17, 2016, 2:28 a.m., Jiang Yan Xu wrote:
> > src/Makefile.am, line 2263
> > <https://reviews.apache.org/r/51880/diff/13/?file=1581277#file1581277line2263>
> >
> >     How about `constainerizer_tests.cpp` so it's easy to establish the 
> > mapping from <component> to <component>_tests.cpp.
> >     
> >     `common_containerizer_tests.cpp` reads like there's a containerizer 
> > called common containerizer.

All these tests are within the path `src/tests/containerizer`, so naming it 
`containerizer_tests.cpp` is probably not great as well. Agreed that 
`common_containerizer_tests.cpp` is not the right name as well. Should we name 
it `src/tests/containerizer/resources_containerizer_tests.cpp` instead?


> On Dec. 17, 2016, 2:28 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/common_containerizer_tests.cpp, line 46
> > <https://reviews.apache.org/r/51880/diff/13/?file=1581278#file1581278line46>
> >
> >     Here we are really just testing `resources()`. How about naming it 
> > `ContainerizerResourcesTests`.
> >     
> >     In the future if more logic goes into the `Containerizer` code, we can 
> > add sepearate test fixtures. (Probably more likely for the resource 
> > detection code to move out first.)

Named it `ResourcesContainerizerTest` and `DiskResourcesContainerizerTest` to 
follow the existing pattern.


> On Dec. 17, 2016, 2:28 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/common_containerizer_tests.cpp, lines 64-89
> > <https://reviews.apache.org/r/51880/diff/13/?file=1581278#file1581278line64>
> >
> >     Why are they not SetUp and TearDown? These special methods are executed 
> > even when tests fail which is what we want. If the thinking is that these 
> > mounts don't need to be in tests that don't involve disks with `DiskInfo`, 
> > then they can be a separate test, e.g., `ConainerizerDiskResourcesTests`.

Ok. Separated into 2 separate classes, viz. `ResourcesContainerizerTest` and 
`DiskResourcesContainerizerTest`, and moved setting up of mounts in `SetUp()` 
and unmounting in `TearDown()`.


- Anindya


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


On Dec. 20, 2016, 2:04 a.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51880/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2016, 2:04 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6062
>     https://issues.apache.org/jira/browse/MESOS-6062
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added unit tests to determine disk size for MOUNT or PATH disks.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 0f62ec70816e8b48e19d35036285656a6e7cd02b 
>   src/tests/containerizer/resources_containerizer_tests.cpp PRE-CREATION 
>   src/tests/resources_tests.cpp 8dfb1be35d9f9c6ff69139d055c6b3d3ec475e68 
> 
> Diff: https://reviews.apache.org/r/51880/diff/
> 
> 
> Testing
> -------
> 
> All tests including the additional tests in this RR passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Reply via email to