mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilbert Song <songzihao1...@gmail.com>
Subject Re: Review Request 57059: Updated default executor tests.
Date Tue, 07 Mar 2017 23:58:34 GMT

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




src/tests/default_executor_tests.cpp
Lines 153-160 (original), 142-149 (patched)
<https://reviews.apache.org/r/57059/#comment240414>

    Could we use `v1::scheduler::SendSubscribe()` for these part?
    
    We can do it once receive `connected()` at line  #119:
    ```
      Future<Nothing> connected;
      EXPECT_CALL(*scheduler, connected(_))
        .WillOnce(DoAll(v1::scheduler::SendSubscribe(frameworkInfo),
                        FutureSatisfy(&connected)));
    ```
    
    ditto to the others.



src/tests/default_executor_tests.cpp
Lines 166-167 (original), 154-161 (patched)
<https://reviews.apache.org/r/57059/#comment240416>

    We could use the `v1::createExecutorInfo()` helper here:
    
    ```
      v1::FrameworkID frameworkId(subscribed->framework_id());
      v1::ExecutorInfo executorInfo = v1::createExecutorInfo(
          "test_default_executor",
          None(),
          "cpus:0.1;mem:32;disk:32",
          v1::ExecutorInfo::DEFAULT);
    
      // Update `executorInfo` with the subscribed `frameworkId`.
      executorInfo.mutable_framework_id()->CopyFrom(frameworkId);
    ```
    
    Ideally, we should make `FrameworkId` configurable in the helper `createExecutorInfo()`.



src/tests/default_executor_tests.cpp
Lines 185-203 (original), 179-197 (patched)
<https://reviews.apache.org/r/57059/#comment240417>

    use `v1::createCallAccept()`?



src/tests/default_executor_tests.cpp
Line 207 (original), 201 (patched)
<https://reviews.apache.org/r/57059/#comment240418>

    Not yours, but should we add check for `TASK_FINISHED`?


- Gilbert Song


On March 6, 2017, 5:17 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57059/
> -----------------------------------------------------------
> 
> (Updated March 6, 2017, 5:17 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Reorganized so that objects are defined closer to their usage.
> 
> 
> Diffs
> -----
> 
>   src/tests/default_executor_tests.cpp e4d43c8ad447577a9c5c7951207596bda1070856 
> 
> 
> Diff: https://reviews.apache.org/r/57059/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


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