> On Nov. 27, 2018, 9:16 p.m., Benjamin Mahler wrote:
> > src/tests/master_tests.cpp
> > Lines 3850 (patched)
> > <https://reviews.apache.org/r/69452/diff/1/?file=2110508#file2110508line3850>
> >
> >     Maybe a TODO to simplify this test by having a test scheduler that 
> > knows how to launch a mock executor?

I'm not sure if this is a good idea, because the default (composing or mesos 
containerizer) would launch the executor in another linux process, which would 
be impossible for mocking, unless we have another binary that just forward all 
the requests back to the mock executor in the test process. Using the test 
containerizer seems more simpler.


> On Nov. 27, 2018, 9:16 p.m., Benjamin Mahler wrote:
> > src/tests/master_tests.cpp
> > Lines 3861 (patched)
> > <https://reviews.apache.org/r/69452/diff/1/?file=2110508#file2110508line3861>
> >
> >     executorConnected?

This holds the `executor::Mesos*` object that we'll use later, and the 
`...Library` name is consistent to other tests where we name the object.


> On Nov. 27, 2018, 9:16 p.m., Benjamin Mahler wrote:
> > src/tests/master_tests.cpp
> > Lines 3974-3976 (patched)
> > <https://reviews.apache.org/r/69452/diff/1/?file=2110508#file2110508line3974>
> >
> >     Maybe settle at the end anyway, so that we start calling destructors 
> > after the message finishes processing? Right now it will start after the 
> > message starts processing.

Do you mean pausing the clock and do a `Clock::settle()` before the test 
finishes? Yeah right now the teardown will start after the message starts 
processing, but the teardown process would wait for the master process to 
terminate.


> On Nov. 27, 2018, 9:16 p.m., Benjamin Mahler wrote:
> > src/tests/mesos.hpp
> > Line 907 (original), 919 (patched)
> > <https://reviews.apache.org/r/69452/diff/1/?file=2110509#file2110509line919>
> >
> >     just curious, what happened here? seems like this should be a different 
> > patch?

Sure let me split it into another patch. It's just that the common helper uses 
the `slave_id` accesser, which is not universal to v0/v1.


- Chun-Hung


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


On Nov. 27, 2018, 4:58 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69452/
> -----------------------------------------------------------
> 
> (Updated Nov. 27, 2018, 4:58 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Till Toenshoff.
> 
> 
> Bugs: MESOS-9419
>     https://issues.apache.org/jira/browse/MESOS-9419
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds the `ExecutorMessageToRecoveredHttpFramework` test to
> ensure that a master will not crash when forwarding an executor message
> to a recovered framework to prevent any future regression of MESOS-9419.
> 
> 
> Diffs
> -----
> 
>   src/tests/master_tests.cpp 651bb9ba9298fc3d7179ed86487aa55be519ce12 
>   src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e 
> 
> 
> Diff: https://reviews.apache.org/r/69452/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> This test will fail without r/69451.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Reply via email to