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


Overall structure and flow of the test looks good.  A few nits:


src/tests/containerizer/isolator_tests.cpp (line 856)
<https://reviews.apache.org/r/42098/#comment174837>

    Nit: s/> >/>>/



src/tests/containerizer/isolator_tests.cpp (line 868)
<https://reviews.apache.org/r/42098/#comment174838>

    Nit: s/> >/>>/



src/tests/containerizer/isolator_tests.cpp (line 878)
<https://reviews.apache.org/r/42098/#comment174839>

    Nit: It might be better to expect exactly 1 offer.



src/tests/containerizer/isolator_tests.cpp (line 880)
<https://reviews.apache.org/r/42098/#comment174840>

    Add a comment here that the test will explicitly kill this task later.



src/tests/containerizer/isolator_tests.cpp (lines 901 - 903)
<https://reviews.apache.org/r/42098/#comment174842>

    `path::join` should add these slashes for you.  Is that not the case?



src/tests/containerizer/isolator_tests.cpp (line 909)
<https://reviews.apache.org/r/42098/#comment174863>

    Nit: This would be a good place to use `path::join` too.
    (And you're missing a space after the second `+` :)



src/tests/containerizer/isolator_tests.cpp (line 912)
<https://reviews.apache.org/r/42098/#comment174865>

    Is there a more meaningful check you can do here?
    
    Like a `strings::contains(...)` where you check for some Mesos-related 
string?



src/tests/containerizer/isolator_tests.cpp (line 920)
<https://reviews.apache.org/r/42098/#comment174870>

    Nit: s/'gc.schedule'/`gc->schedule`/


- Joseph Wu


On Jan. 12, 2016, 10:32 a.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42098/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2016, 10:32 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added unit test-case for CgroupsNetClsIsolatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 91178b69ccbf5b6cbf64421e5602e6d554fc34ca 
> 
> Diff: https://reviews.apache.org/r/42098/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Reply via email to