Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-25 Thread Ben Mahler 


> On June 25, 2015, 12:38 a.m., Vinod Kone wrote:
> > src/tests/master_contender_detector_tests.cpp, line 877
> > 
> >
> > s/EXPECT/ASSERT/
> > 
> > as this is the last expectation in the test.
> 
> Marco Massenzio wrote:
> LoL - I was doing some AngularJS unit testing the night before, the 
> `expect` must have gotten stuck in my head!

Well, we use EXPECT when the test can continue if the expectation fails, ASSERT 
when the test cannot proceed further: http://stackoverflow.com/a/2565309

But it's not that critical, pretty sure most tests are inconsistent about this.


- Ben


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


On June 25, 2015, 7:53 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 25, 2015, 7:53 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-25 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [35815]

All tests passed.

- Mesos ReviewBot


On June 25, 2015, 7:53 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 25, 2015, 7:53 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-25 Thread Marco Massenzio 

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

(Updated June 25, 2015, 7:53 a.m.)


Review request for mesos, Niklas Nielsen and Vinod Kone.


Changes
---

Vinod's comments.


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


Repository: mesos


Description
---

As of Mesos 0.24, we will support JSON format
for the MasterInfo information written out to
ZooKeeper for the Leader election process.

This tests that the JSON can be correctly parsed
and the correct MasterInfo information retrieved.

Jira: MESOS-2898


Diffs (updated)
-

  src/tests/master_contender_detector_tests.cpp 
1b30eeb51520da7a5834768ac88a88a8a84fd976 

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


Testing
---

make check


Thanks,

Marco Massenzio

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-25 Thread Marco Massenzio 


> On June 25, 2015, 12:38 a.m., Vinod Kone wrote:
> > src/tests/master_contender_detector_tests.cpp, line 877
> > 
> >
> > s/EXPECT/ASSERT/
> > 
> > as this is the last expectation in the test.

LoL - I was doing some AngularJS unit testing the night before, the `expect` 
must have gotten stuck in my head!


- Marco


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


On June 24, 2015, 6:01 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 24, 2015, 6:01 p.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-24 Thread Vinod Kone 

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

Ship it!


Nice test!


src/tests/master_contender_detector_tests.cpp (line 848)


s/know/known/



src/tests/master_contender_detector_tests.cpp (lines 856 - 858)


We generally avoid putting LOG messages in tests. I would recommend to kill 
this.



src/tests/master_contender_detector_tests.cpp (line 864)


s/writing/write/



src/tests/master_contender_detector_tests.cpp (line 877)


s/EXPECT/ASSERT/

as this is the last expectation in the test.


- Vinod Kone


On June 24, 2015, 6:01 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 24, 2015, 6:01 p.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-24 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [35815]

All tests passed.

- Mesos ReviewBot


On June 24, 2015, 6:01 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 24, 2015, 6:01 p.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-24 Thread Marco Massenzio 

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

(Updated June 24, 2015, 6:01 p.m.)


Review request for mesos, Niklas Nielsen and Vinod Kone.


Changes
---

Fixed leaks & indent


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


Repository: mesos


Description
---

As of Mesos 0.24, we will support JSON format
for the MasterInfo information written out to
ZooKeeper for the Leader election process.

This tests that the JSON can be correctly parsed
and the correct MasterInfo information retrieved.

Jira: MESOS-2898


Diffs (updated)
-

  src/tests/master_contender_detector_tests.cpp 
1b30eeb51520da7a5834768ac88a88a8a84fd976 

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


Testing
---

make check


Thanks,

Marco Massenzio

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-24 Thread Marco Massenzio 


> On June 24, 2015, 2:31 a.m., Anand Mazumdar wrote:
> > src/tests/master_contender_detector_tests.cpp, line 865
> > 
> >
> > Leaking here too. This can be wrapped in Owned<>

Thanks for catching this.
Clearly been doing too much Python/Java for too long...


- Marco


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


On June 24, 2015, 1:15 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 24, 2015, 1:15 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-24 Thread Marco Massenzio 


> On June 24, 2015, 2:21 a.m., haosdent huang wrote:
> > src/tests/master_contender_detector_tests.cpp, line 844
> > 
> >
> > Oh, you are follow the test case before.

this doesn't make it right :)
Fixed (both places)


- Marco


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


On June 24, 2015, 1:15 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 24, 2015, 1:15 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-23 Thread Anand Mazumdar 

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



src/tests/master_contender_detector_tests.cpp (line 861)


We are leaking here ? Using Group group(...); should suffice here.



src/tests/master_contender_detector_tests.cpp (line 865)


Leaking here too. This can be wrapped in Owned<>


- Anand Mazumdar


On June 24, 2015, 1:15 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 24, 2015, 1:15 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-23 Thread haosdent huang 

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

Ship it!


Ship It!

- haosdent huang


On June 24, 2015, 1:15 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 24, 2015, 1:15 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-23 Thread haosdent huang 

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



src/tests/master_contender_detector_tests.cpp (line 844)


Oh, you are follow the test case before.


- haosdent huang


On June 24, 2015, 1:15 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 24, 2015, 1:15 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-23 Thread haosdent huang 

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



src/tests/master_contender_detector_tests.cpp (line 40)


Should change to  here?



src/tests/master_contender_detector_tests.cpp (line 844)


Seems the indent is not correct here.


- haosdent huang


On June 24, 2015, 1:15 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 24, 2015, 1:15 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-23 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [35815]

All tests passed.

- Mesos ReviewBot


On June 24, 2015, 1:15 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35815/
> ---
> 
> (Updated June 24, 2015, 1:15 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2898
> https://issues.apache.org/jira/browse/MESOS-2898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As of Mesos 0.24, we will support JSON format
> for the MasterInfo information written out to
> ZooKeeper for the Leader election process.
> 
> This tests that the JSON can be correctly parsed
> and the correct MasterInfo information retrieved.
> 
> Jira: MESOS-2898
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 1b30eeb51520da7a5834768ac88a88a8a84fd976 
> 
> Diff: https://reviews.apache.org/r/35815/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-23 Thread Marco Massenzio 

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

(Updated June 24, 2015, 1:15 a.m.)


Review request for mesos, Niklas Nielsen and Vinod Kone.


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


Repository: mesos


Description
---

As of Mesos 0.24, we will support JSON format
for the MasterInfo information written out to
ZooKeeper for the Leader election process.

This tests that the JSON can be correctly parsed
and the correct MasterInfo information retrieved.

Jira: MESOS-2898


Diffs (updated)
-

  src/tests/master_contender_detector_tests.cpp 
1b30eeb51520da7a5834768ac88a88a8a84fd976 

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


Testing
---

make check


Thanks,

Marco Massenzio