Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2016-01-13 Thread Avinash sridharan

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

(Updated Jan. 13, 2016, 3:43 p.m.)


Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
Conway.


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


Repository: mesos


Description
---

Adding the test framework submitted by Mandeep (@mchadha) 
https://reviews.apache.org/r/39056/


Diffs
-

  src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 

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


Testing
---

Ran make check after adding Mandeep's test case to the GTEST framework.


Thanks,

Avinash sridharan



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-12-08 Thread Avinash sridharan


> On Dec. 2, 2015, 6:32 p.m., Bernd Mathiske wrote:
> > src/tests/reservation_tests.cpp, line 248
> > 
> >
> > More customary indentation:
> > 
> >  EXPECT_CALL(sched, resourceOffers(, _))
> >.WillOnce(FutureArg<1>());
> 
> Avinash sridharan wrote:
> Was following this particular style guide:
> http://mesos.apache.org/documentation/latest/c++-style-guide/
> 
> The "Function Definition/Invocation" section wants us to follow styles 
> 1,4, 5 and sometimes 3 and 'never' 2 hence the change?

Anand had a similar comment. I was trying to follow the style guide, but I 
guess there are already inconsistencies in the existing code and enforcing the 
style guide will create an outlier that might make it worse. So just sticking 
with whatever is followed in the current file.


> On Dec. 2, 2015, 6:32 p.m., Bernd Mathiske wrote:
> > src/tests/reservation_tests.cpp, line 266
> > 
> >
> > Missing blanks after "//".

Remove this comment since Anand felt it was redundant.


- Avinash


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


On Dec. 2, 2015, 10:38 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Dec. 2, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-12-02 Thread Bernd Mathiske


> On Nov. 30, 2015, 6:15 a.m., Bernd Mathiske wrote:
> > src/tests/reservation_tests.cpp, line 248
> > 
> >
> > s/RESERVE/UNRESERVE/ ?
> > 
> > What is the intention here? Please explain.
> 
> Avinash sridharan wrote:
> Can you clarify this comment? The idea here seems to be to generate 
> multiple CPU resource requests. Hence the RESERVE?

The misleading comment near this is gone and a new one is there that addresses 
the issue. Thx


- Bernd


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


On Dec. 2, 2015, 12:27 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Dec. 2, 2015, 12:27 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-12-02 Thread Bernd Mathiske

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



src/tests/reservation_tests.cpp (line 237)


More customary indentation:

 EXPECT_CALL(sched, resourceOffers(, _))
   .WillOnce(FutureArg<1>());



src/tests/reservation_tests.cpp (line 243)


Please put blanks after "//" on theleft.

Typos: 
- calculate
- comparison

Never use apostrophy abbreviations where not necessary:
s/isn't/is not/

Missing period at sentence end after MESOS-3552.



src/tests/reservation_tests.cpp (line 245)


Where exactly is "over here"? Does the wait statement contain floating 
point comparisons?



src/tests/reservation_tests.cpp (line 254)


Missing blanks after "//".


- Bernd Mathiske


On Dec. 2, 2015, 12:27 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Dec. 2, 2015, 12:27 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-12-02 Thread Avinash sridharan


> On Dec. 2, 2015, 6:32 p.m., Bernd Mathiske wrote:
> > src/tests/reservation_tests.cpp, line 248
> > 
> >
> > More customary indentation:
> > 
> >  EXPECT_CALL(sched, resourceOffers(, _))
> >.WillOnce(FutureArg<1>());

Was following this particular style guide:
http://mesos.apache.org/documentation/latest/c++-style-guide/

The "Function Definition/Invocation" section wants us to follow styles 1,4, 5 
and sometimes 3 and 'never' 2 hence the change?


> On Dec. 2, 2015, 6:32 p.m., Bernd Mathiske wrote:
> > src/tests/reservation_tests.cpp, line 257
> > 
> >
> > Where exactly is "over here"? Does the wait statement contain floating 
> > point comparisons?

over here is actually the agent. Would result a crash in the agent. Will update 
the comment accordingly


- Avinash


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


On Dec. 2, 2015, 8:27 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Dec. 2, 2015, 8:27 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-12-02 Thread Anand Mazumdar

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


Some minor comments modulo comments from Bernd earlier.


src/tests/reservation_tests.cpp (line 205)


Is this comment needed ?



src/tests/reservation_tests.cpp (line 222)


Can we just do:

`EXPECT_CALL(sched, resourceOffers(, _))
 .WillOnce(FutureArg<1>());`
 
This is more in sync with the style followed in the rest of the tests. Can 
we modify the other ocurrences in this test too ?



src/tests/reservation_tests.cpp (line 254)


We generally avoid redundant/self-explanatory comments. Is this really 
needed at all ? What is it trying to convey ?



src/tests/reservation_tests.cpp (line 261)


Not yours : Can we insert an additional line here. We leave 2 lines after 
each global function/test.


- Anand Mazumdar


On Dec. 2, 2015, 8:27 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Dec. 2, 2015, 8:27 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-12-02 Thread Avinash sridharan

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

(Updated Dec. 2, 2015, 8:23 a.m.)


Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.


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


Repository: mesos


Description
---

Adding the test framework submitted by Mandeep (@mchadha) 
https://reviews.apache.org/r/39056/


Diffs (updated)
-

  src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 

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


Testing
---

Ran make check after adding Mandeep's test case to the GTEST framework.


Thanks,

Avinash sridharan



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-12-02 Thread Avinash sridharan


> On Nov. 30, 2015, 2:15 p.m., Bernd Mathiske wrote:
> > src/tests/reservation_tests.cpp, line 176
> > 
> >
> > Also see the indentations in various other statements involving 
> > argument passing, etc. Best to sit down with one of us at the same screen 
> > and then clean up all these indentations in one swoop. Respectively see: 
> > http://mesos.apache.org/documentation/latest/c++-style-guide/

Tried following the style guide as closely as possible. If we still see some 
missing style errors will sit with one of the core folks here and fix it.


> On Nov. 30, 2015, 2:15 p.m., Bernd Mathiske wrote:
> > src/tests/reservation_tests.cpp, line 248
> > 
> >
> > s/RESERVE/UNRESERVE/ ?
> > 
> > What is the intention here? Please explain.

Can you clarify this comment? The idea here seems to be to generate multiple 
CPU resource requests. Hence the RESERVE?


- Avinash


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


On Dec. 2, 2015, 8:27 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Dec. 2, 2015, 8:27 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-12-02 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [40730, 40732, 40731]

Failed command: ./support/apply-review.sh -n -r 40731

Error:
 2015-12-02 08:48:03 URL:https://reviews.apache.org/r/40731/diff/raw/ 
[7250/7250] -> "40731.patch" [1]
error: patch failed: src/tests/reservation_tests.cpp:161
error: src/tests/reservation_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Dec. 2, 2015, 8:27 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Dec. 2, 2015, 8:27 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-12-02 Thread Avinash sridharan


> On Dec. 2, 2015, 8:02 p.m., Anand Mazumdar wrote:
> > src/tests/reservation_tests.cpp, line 213
> > 
> >
> > Is this comment needed ?

I think this a copy-paste issue. This must have come from the previous test 
case (TEST_F(ReservationTest, ReserveThenUnreserve)) that was copied over. Will 
remove it.


> On Dec. 2, 2015, 8:02 p.m., Anand Mazumdar wrote:
> > src/tests/reservation_tests.cpp, line 231
> > 
> >
> > Can we just do:
> > 
> > `EXPECT_CALL(sched, resourceOffers(, _))
> >  .WillOnce(FutureArg<1>());`
> >  
> > This is more in sync with the style followed in the rest of the tests. 
> > Can we modify the other ocurrences in this test too ?

The above exceeds the 80 column limit. I was following the style guide here:
http://mesos.apache.org/documentation/latest/c++-style-guide/

The "Function Definition/Invocation" section wants us to follow styles 1,4, 5 
and sometimes 3 and 'never' 2 hence the change?


> On Dec. 2, 2015, 8:02 p.m., Anand Mazumdar wrote:
> > src/tests/reservation_tests.cpp, line 266
> > 
> >
> > We generally avoid redundant/self-explanatory comments. Is this really 
> > needed at all ? What is it trying to convey ?

The problem is that the check failure would lead to a crash in the agent, and 
wouldn't show up in the mockscheduler. Not sure if we can re-write the test 
case somehow to reflect it. Hence added an explicit comment to explain the 
behavior to the reader.


> On Dec. 2, 2015, 8:02 p.m., Anand Mazumdar wrote:
> > src/tests/reservation_tests.cpp, line 273
> > 
> >
> > Not yours : Can we insert an additional line here. We leave 2 lines 
> > after each global function/test.

ok


- Avinash


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


On Dec. 2, 2015, 8:27 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Dec. 2, 2015, 8:27 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-12-02 Thread Avinash sridharan

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

(Updated Dec. 2, 2015, 10:38 p.m.)


Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
Conway.


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


Repository: mesos


Description
---

Adding the test framework submitted by Mandeep (@mchadha) 
https://reviews.apache.org/r/39056/


Diffs (updated)
-

  src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 

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


Testing
---

Ran make check after adding Mandeep's test case to the GTEST framework.


Thanks,

Avinash sridharan



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-12-02 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [40730, 40732, 40731]

Failed command: ./support/apply-review.sh -n -r 40731

Error:
 2015-12-03 00:14:47 URL:https://reviews.apache.org/r/40731/diff/raw/ 
[7279/7279] -> "40731.patch" [1]
error: patch failed: src/tests/reservation_tests.cpp:161
error: src/tests/reservation_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Dec. 2, 2015, 10:38 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Dec. 2, 2015, 10:38 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-11-30 Thread Bernd Mathiske

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



src/tests/reservation_tests.cpp (line 168)


s/double/a double

But the whole sentence is warped. Suggestion:

"This involves a value for CPUs of type double, which triggers a 
problematic floating point comparison."



src/tests/reservation_tests.cpp (line 172)


Please write a comment in the code below that explains where the comparison 
occurs.



src/tests/reservation_tests.cpp (line 173)


s/with/with an/



src/tests/reservation_tests.cpp (line 176)


Also see the indentations in various other statements involving argument 
passing, etc. Best to sit down with one of us at the same screen and then clean 
up all these indentations in one swoop. Respectively see: 
http://mesos.apache.org/documentation/latest/c++-style-guide/



src/tests/reservation_tests.cpp (line 198)


This can be made less ambigous:

"We apply a filter that causes these resources not to be filtered (default 
would be 5 seconds).



src/tests/reservation_tests.cpp (line 208)


s/We use this to/

Redundant.



src/tests/reservation_tests.cpp (line 219)


This comment pertains to line 225, not 220.



src/tests/reservation_tests.cpp (line 227)


If you use "next" here and below for two different offers, it gets 
confusing. Better "second", "third".



src/tests/reservation_tests.cpp (line 235)


This comment seems to belong to line 241.



src/tests/reservation_tests.cpp (line 239)


Better to use freah variables like offer 2, offer3. It will be easier to 
grasp where in the algorithm we are then.



src/tests/reservation_tests.cpp (line 248)


s/RESERVE/UNRESERVE/ ?

What is the intention here? Please explain.



src/tests/reservation_tests.cpp (line 251)


This comment pertains to line 257.

s/reserved/unreserved/ ?


- Bernd Mathiske


On Nov. 30, 2015, 5:39 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Nov. 30, 2015, 5:39 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 15d180f92ec0aea99e6f3a7d0b505c62bd207b61 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-11-30 Thread Avinash sridharan

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

(Updated Nov. 30, 2015, 5:39 a.m.)


Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.


Changes
---

Fixed the link to the JIRA issue.


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


Repository: mesos


Description
---

Adding the test framework submitted by Mandeep (@mchadha) 
https://reviews.apache.org/r/39056/


Diffs
-

  src/tests/reservation_tests.cpp 15d180f92ec0aea99e6f3a7d0b505c62bd207b61 

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


Testing
---

Ran make check after adding Mandeep's test case to the GTEST framework.


Thanks,

Avinash sridharan



Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

2015-11-26 Thread Neil Conway

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



src/tests/reservation_tests.cpp (line 163)


In Mesos, we use two blank lines to separate function definitions.



src/tests/reservation_tests.cpp (line 176)


The whitespace is wrong here: Mesos uses an indentation offset of 2 spaces, 
not 8. For more on the style requirements, see 
https://mesos.apache.org/documentation/latest/c++-style-guide/


- Neil Conway


On Nov. 26, 2015, 6:52 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Nov. 26, 2015, 6:52 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.
> 
> 
> Bugs: 3552
> https://issues.apache.org/jira/browse/3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 15d180f92ec0aea99e6f3a7d0b505c62bd207b61 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>