Re: Review Request 50706: Added test cases for PARTITION_AWARE behavior.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 6, 2016, 3:24 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50706/
> ---
> 
> (Updated Sept. 6, 2016, 3:24 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4049
> https://issues.apache.org/jira/browse/MESOS-4049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for PARTITION_AWARE behavior.
> 
> 
> Diffs
> -
> 
>   src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 
> 
> Diff: https://reviews.apache.org/r/50706/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 50706: Added test cases for PARTITION_AWARE behavior.

[Neil Conway]

> On Sept. 6, 2016, 2:14 p.m., Vinod Kone wrote:
> > src/tests/partition_tests.cpp, line 710
> > 
> >
> > hmm. didn't realize that the TODO in `Master::reconcile` regarding 
> > assumption is actually wrong! tasks could be unknown to the master but 
> > known to the agent. not sure why we made that assumption.
> 
> Neil Conway wrote:
> I believe that the comment in `Master::reconcile` assumes that the master 
> hasn't failed over (see the `NOTE` at the top of the function).
> 
> Vinod Kone wrote:
> yes, but this test exercises the scenario where the master doesn't 
> failover but still has no idea about the tasks on a re-registering agent!

This test does involve (simulated) master failover, though, which is why the 
master doesn't know about the tasks on the reregistering agent.


- Neil


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


On Sept. 6, 2016, 3:24 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50706/
> ---
> 
> (Updated Sept. 6, 2016, 3:24 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4049
> https://issues.apache.org/jira/browse/MESOS-4049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for PARTITION_AWARE behavior.
> 
> 
> Diffs
> -
> 
>   src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 
> 
> Diff: https://reviews.apache.org/r/50706/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 50706: Added test cases for PARTITION_AWARE behavior.

[Vinod Kone]

> On Sept. 6, 2016, 2:14 p.m., Vinod Kone wrote:
> > src/tests/partition_tests.cpp, line 710
> > 
> >
> > hmm. didn't realize that the TODO in `Master::reconcile` regarding 
> > assumption is actually wrong! tasks could be unknown to the master but 
> > known to the agent. not sure why we made that assumption.
> 
> Neil Conway wrote:
> I believe that the comment in `Master::reconcile` assumes that the master 
> hasn't failed over (see the `NOTE` at the top of the function).

yes, but this test exercises the scenario where the master doesn't failover but 
still has no idea about the tasks on a re-registering agent!


- Vinod


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


On Sept. 6, 2016, 3:24 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50706/
> ---
> 
> (Updated Sept. 6, 2016, 3:24 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4049
> https://issues.apache.org/jira/browse/MESOS-4049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for PARTITION_AWARE behavior.
> 
> 
> Diffs
> -
> 
>   src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 
> 
> Diff: https://reviews.apache.org/r/50706/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 50706: Added test cases for PARTITION_AWARE behavior.

[Neil Conway]

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

(Updated Sept. 6, 2016, 3:24 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Address review comments.


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


Repository: mesos


Description
---

Added test cases for PARTITION_AWARE behavior.


Diffs (updated)
-

  src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 50706: Added test cases for PARTITION_AWARE behavior.

[Neil Conway]

> On Sept. 6, 2016, 2:14 p.m., Vinod Kone wrote:
> > src/tests/partition_tests.cpp, line 723
> > 
> >
> > `SlaveObserver`

Fixed the other places where this occurs as well.


> On Sept. 6, 2016, 2:14 p.m., Vinod Kone wrote:
> > src/tests/partition_tests.cpp, line 710
> > 
> >
> > hmm. didn't realize that the TODO in `Master::reconcile` regarding 
> > assumption is actually wrong! tasks could be unknown to the master but 
> > known to the agent. not sure why we made that assumption.

I believe that the comment in `Master::reconcile` assumes that the master 
hasn't failed over (see the `NOTE` at the top of the function).


- Neil


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


On Aug. 29, 2016, 9:49 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50706/
> ---
> 
> (Updated Aug. 29, 2016, 9:49 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4049
> https://issues.apache.org/jira/browse/MESOS-4049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for PARTITION_AWARE behavior.
> 
> 
> Diffs
> -
> 
>   src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 
> 
> Diff: https://reviews.apache.org/r/50706/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 50706: Added test cases for PARTITION_AWARE behavior.

[Vinod Kone]

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


Fix it, then Ship it!





src/tests/partition_tests.cpp (line 462)


re-register



src/tests/partition_tests.cpp (line 466)


re-registration



src/tests/partition_tests.cpp (line 613)


new line after this comment since it applies two blocks below; or just move 
the "sched2..." part to #620.



src/tests/partition_tests.cpp (lines 647 - 651)


instead of having expectations on the FrameworkRegisteredMessage, why not 
leverage registered callback?

Future registered1;
Future registered2;
EXPECT_CALL(sched1, registered(, _, _))
  .WillOnce(FutureSatisfy());
EXPECT_CALL(sched1, registered(, _, _))
  .WillOnce(FutureSatisfy());



src/tests/partition_tests.cpp (line 710)


hmm. didn't realize that the TODO in `Master::reconcile` regarding 
assumption is actually wrong! tasks could be unknown to the master but known to 
the agent. not sure why we made that assumption.



src/tests/partition_tests.cpp (line 723)


`SlaveObserver`



src/tests/partition_tests.cpp (line 867)


re-registers



src/tests/partition_tests.cpp (line 887)


re-registration



src/tests/partition_tests.cpp (line 934)


re-registers


- Vinod Kone


On Aug. 29, 2016, 9:49 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50706/
> ---
> 
> (Updated Aug. 29, 2016, 9:49 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4049
> https://issues.apache.org/jira/browse/MESOS-4049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for PARTITION_AWARE behavior.
> 
> 
> Diffs
> -
> 
>   src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 
> 
> Diff: https://reviews.apache.org/r/50706/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 50706: Added test cases for PARTITION_AWARE behavior.

[Neil Conway]

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

(Updated Aug. 29, 2016, 9:49 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Simplify test code.


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


Repository: mesos


Description
---

Added test cases for PARTITION_AWARE behavior.


Diffs (updated)
-

  src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 

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


Testing
---

make check


Thanks,

Neil Conway