mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gaston Kleiman <gas...@mesosphere.io>
Subject Re: Review Request 64879: Updated fetcher cache tests for the default executor.
Date Tue, 02 Jan 2018 19:23:51 GMT

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


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