mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anindya Sinha <anindya_si...@apple.com>
Subject Re: Review Request 51880: Added unit tests to determine disk size for MOUNT or PATH disks.
Date Tue, 20 Dec 2016 02:04:51 GMT


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


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