Re: Review Request 36314: Added test for passing total slave's resources in ResourceUsage.

2015-07-14 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [36204, 36314]

All tests passed.

- Mesos ReviewBot


On July 14, 2015, 11:09 a.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36314/
> ---
> 
> (Updated July 14, 2015, 11:09 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow up tests for https://reviews.apache.org/r/36204/
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 
> 
> Diff: https://reviews.apache.org/r/36314/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 36314: Added test for passing total slave's resources in ResourceUsage.

2015-07-14 Thread Bartek Plotka 

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

(Updated July 14, 2015, 11:09 a.m.)


Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
Kone.


Changes
---

Issues addressed.


Repository: mesos


Description
---

Follow up tests for https://reviews.apache.org/r/36204/


Diffs (updated)
-

  src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 

Diff: https://reviews.apache.org/r/36314/diff/


Testing
---

make check.


Thanks,

Bartek Plotka

Re: Review Request 36314: Added test for passing total slave's resources in ResourceUsage.

2015-07-09 Thread Jie Yu 

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

Ship it!


LGTM! Some nits.


src/tests/slave_tests.cpp (line 2258)


I don't think you need to use the MockExecutor here.



src/tests/slave_tests.cpp (line 2262)


No need to use the temp variable. I would suggest the following:

```
slave::Flags flags = CreateSlaveFlage();
flags.resources = "cpus:2;mem:1024;disk:1024;ports:[31000-32000]";

...

AWAIT_READY(usage);

EXPECT_EQ(Resources(usage.get().total()),
  Resources::parse(flags.resources).get());
```



src/tests/slave_tests.cpp (line 2280)


This check seems to be redundent. I would suggest killing it.



src/tests/slave_tests.cpp (line 2301)


Same. No need for MockExecutor.



src/tests/slave_tests.cpp (lines 2305 - 2307)


No need for this temp variable.


- Jie Yu


On July 9, 2015, 1:09 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36314/
> ---
> 
> (Updated July 9, 2015, 1:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow up tests for https://reviews.apache.org/r/36204/
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 
> 
> Diff: https://reviews.apache.org/r/36314/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 36314: Added test for passing total slave's resources in ResourceUsage.

2015-07-09 Thread Bartek Plotka 

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

(Updated July 9, 2015, 1:09 p.m.)


Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
Kone.


Changes
---

Attaching this request to resolved Mesos issue is not a good idea (:


Repository: mesos


Description
---

Follow up tests for https://reviews.apache.org/r/36204/


Diffs
-

  src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 

Diff: https://reviews.apache.org/r/36314/diff/


Testing
---

make check.


Thanks,

Bartek Plotka

Re: Review Request 36314: Added test for passing total slave's resources in ResourceUsage.

2015-07-08 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [36204, 36314]

All tests passed.

- Mesos ReviewBot


On July 8, 2015, 4:16 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36314/
> ---
> 
> (Updated July 8, 2015, 4:16 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2933
> https://issues.apache.org/jira/browse/MESOS-2933
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow up tests for https://reviews.apache.org/r/36204/
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 
> 
> Diff: https://reviews.apache.org/r/36314/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Review Request 36314: Added test for passing total slave's resources in ResourceUsage.

2015-07-08 Thread Bartek Plotka 

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

Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
Kone.


Bugs: MESOS-2933
https://issues.apache.org/jira/browse/MESOS-2933


Repository: mesos


Description
---

Follow up tests for https://reviews.apache.org/r/36204/


Diffs (updated)
-

  src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 

Diff: https://reviews.apache.org/r/36314/diff/


Testing
---

make check.


Thanks,

Bartek Plotka