Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

(Updated Oct. 27, 2015, 12:37 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp e429d84 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Vinod Kone]

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

Ship it!



src/tests/executor_http_api_tests.cpp (line 543)


s/responseSubscribe/response/

remove the suffices for response here and everywhere else below.



src/tests/executor_http_api_tests.cpp (line 562)


ditto.

s/responseUpdate/response/



src/tests/executor_http_api_tests.cpp (line 581)


ditto. just call it 'response'



src/tests/executor_http_api_tests.cpp (line 611)


s/valid//

s/executor_id/executor id/



src/tests/executor_http_api_tests.cpp (line 632)


ditto. just call it 'response'



src/tests/executor_http_api_tests.cpp (line 642)


s/valid//



src/tests/executor_http_api_tests.cpp (line 693)


ditto. just call it 'response'


- Vinod Kone


On Oct. 27, 2015, 12:37 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> ---
> 
> (Updated Oct. 27, 2015, 12:37 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp e429d84 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

(Updated Oct. 27, 2015, 1:30 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

changes after review comments


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


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp e429d84 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38618, 38577, 38844]

All tests passed.

- Mesos ReviewBot


On Oct. 27, 2015, 1:30 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> ---
> 
> (Updated Oct. 27, 2015, 1:30 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp e429d84 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

(Updated Oct. 2, 2015, 6:53 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38618, 38577, 38844]

All tests passed.

- Mesos ReviewBot


On Oct. 2, 2015, 6:53 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> ---
> 
> (Updated Oct. 2, 2015, 6:53 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Anand Mazumdar]

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

Ship it!


LGTM

- Anand Mazumdar


On Oct. 2, 2015, 6:53 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> ---
> 
> (Updated Oct. 2, 2015, 6:53 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs
-

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

(Updated Sept. 29, 2015, 8:08 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs
-

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38618, 38577, 38844]

All tests passed.

- Mesos ReviewBot


On Sept. 29, 2015, 9:27 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> ---
> 
> (Updated Sept. 29, 2015, 9:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

(Updated Sept. 29, 2015, 9:27 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Adding test for invalid status update 'TASK_STAGING'


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


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Anand Mazumdar]

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


LGTM ! Just some minor comments to clean up the tests around 
parameterizing/scoping.


src/tests/executor_http_api_tests.cpp (line 450)


s/TEST_F/TEST_P

Is there a reason why this test is not parameterized ?



src/tests/executor_http_api_tests.cpp (line 468)






src/tests/executor_http_api_tests.cpp (lines 468 - 483)


s/responseSubscribe/response

Can we use scoping here for each of the 3 different scenarios :

{
  Call call;
  blah blah;
}

{
  Call call;
  blah blah;
}
...



src/tests/executor_http_api_tests.cpp (line 515)


s/TEST_F/TEST_P and make this test parametrized. Is there a reason why this 
is not ?

s/InvalidCallUpdate/StatusUpdateCallFailedValidation to be more descriptive



src/tests/executor_http_api_tests.cpp (line 531)


// We send a valid Call::Update message with inconsistent executor_id 
between Call::executor_id and Call::Update::TaskInfo::executor_id. This should 
result in failed validation.

^^ How does this sound to you ?



src/tests/executor_http_api_tests.cpp (line 561)


Nit: // We send a valid Call::Update message with a TASK_STAGING status 
update. This should fail validation.

(Mainly because after parameterizing the test this won't always be a 
protobuf message.)



src/tests/executor_http_api_tests.cpp (lines 563 - 573)


Scope this similar to my earlier comment:

{
  blah blah;
}


- Anand Mazumdar


On Sept. 29, 2015, 9:27 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> ---
> 
> (Updated Sept. 29, 2015, 9:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

(Updated Sept. 29, 2015, 8:05 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

(Updated Sept. 29, 2015, 8:25 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

> On Sept. 29, 2015, 8:11 p.m., Anand Mazumdar wrote:
> > Can you also update the new tests to use the updated way to test recovery 
> > of Agent as done by the other tests ? ( Otherwise, these would be flaky )

Done :) Thanks


- Isabel


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


On Sept. 29, 2015, 8:25 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> ---
> 
> (Updated Sept. 29, 2015, 8:25 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>