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




src/tests/fetcher_cache_tests.cpp
Lines 391-393 (original), 393-399 (patched)
<https://reviews.apache.org/r/64879/#comment273465>

    Can't we just do `return os::access(path, X_OK);` here?



src/tests/fetcher_cache_tests.cpp
Line 487 (original), 504 (patched)
<https://reviews.apache.org/r/64879/#comment273466>

    I think the variable should be named `task_`:
    
    > [...]
    > prepend constructor and function arguments with a leading underscore to 
avoid ambiguity and / or shadowing
    > [...]
    > Prefer trailing underscores for use as member fields (but not required). 
Some trailing underscores are used to distinguish between similar variables in 
the same scope (think prime symbols), but this should be avoided as much as 
possible, including removing existing instances in the code base.



src/tests/fetcher_cache_tests.cpp
Line 503 (original), 523 (patched)
<https://reviews.apache.org/r/64879/#comment273551>

    you're overwriting `_task.runDirectory` here, is that intentional?


- Gaston Kleiman


On Dec. 30, 2017, 12:06 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64879/
> -----------------------------------------------------------
> 
> (Updated Dec. 30, 2017, 12:06 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, and Jie Yu.
> 
> 
> Bugs: MESOS-8366
>     https://issues.apache.org/jira/browse/MESOS-8366
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated the task launch helper to obtain the ContainerID of the
> launched task, which can be used to correctly construct the sandbox
> path for both the command and default executors.
> 
> Updated the test expectations in cases where the default executor
> container will invoke the fetcher more times than the test is
> expecting.
> 
> Updated various test helper functions to propagate more detailed
> error results that make test failure easier to debug.
> 
> 
> Diffs
> -----
> 
>   src/tests/fetcher_cache_tests.cpp 7db7b7dcef27b1686ccae5a7408ff2811a3b9255 
> 
> 
> Diff: https://reviews.apache.org/r/64879/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Reply via email to