> On Oct. 14, 2017, 1:40 a.m., Anand Mazumdar wrote:
> > src/tests/scheduler_http_api_tests.cpp
> > Lines 737-747 (patched)
> > <https://reviews.apache.org/r/62241/diff/1/?file=1820104#file1820104line737>
> >
> >     Kill this? We don't need this to test if the downgrade worked.
> 
> Ilya Pronin wrote:
>     The crash that this test reproduces occurs when the master handles a 
> `Message` from the scheduler. We can remove the reconciliation part and the 
> crash would still happen when the scheduler driver sends a `TEARDOWN` call 
> message from `SchedulerProcess::stop()`. But I think it is better to be 
> explicit here and not leave exercising the code that we were fixing to the 
> cleanup logic.

I see. Can you modify the comment to also mention why are you exercising the 
message handling i.e., it used to crash the master previously? Otherwise, its 
not immediately obvious as to why it's being done here.


- Anand


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


On Oct. 16, 2017, 6:17 p.m., Ilya Pronin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62241/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2017, 6:17 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-7867
>     https://issues.apache.org/jira/browse/MESOS-7867
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This test verifies that we are able to upgrade from a PID based
> framework to an HTTP framework and then downgrade back without
> restarting the master.
> 
> 
> Diffs
> -----
> 
>   src/tests/scheduler_http_api_tests.cpp 
> cc03be0d2110f08023bf0ce181da37658737bd7c 
> 
> 
> Diff: https://reviews.apache.org/r/62241/diff/2/
> 
> 
> Testing
> -------
> 
> Ran `make check`. Verified that the test fails without the previous patch.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Reply via email to