You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jan Schlicht <ja...@mesosphere.io> on 2017/08/01 14:06:16 UTC
Re: Review Request 61272: Added a MockResourceProvider.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/
-----------------------------------------------------------
(Updated Aug. 1, 2017, 4:06 p.m.)
Review request for mesos, Benjamin Bannier and Jie Yu.
Summary (updated)
-----------------
Added a MockResourceProvider.
Repository: mesos
Description (updated)
-------
See summary.
Diffs (updated)
-----
src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
Diff: https://reviews.apache.org/r/61272/diff/2/
Changes: https://reviews.apache.org/r/61272/diff/1-2/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jie Yu <yu...@gmail.com>.
> On Aug. 1, 2017, 10:37 p.m., Jie Yu wrote:
> > src/tests/mesos.hpp
> > Lines 2287-2291 (patched)
> > <https://reviews.apache.org/r/61272/diff/2/?file=1786382#file1786382line2287>
> >
> > This is a bit counter intuitive. I was expecting that MockResourceProvider will take a real Driver:
> > ```
> > class MockResourceProvider
> > {
> > public:
> > MockResourceProvider(
> > const URL& endpoint,
> > ContentType contentType)
> > : driver(
> > Owned<EndpointDetector>(new ConstantEndpointDetector(endpoint)),
> > contentType,
> > [this]() { connected(); },
> > [this]() { disconnected(); },
> > [this](std::queue<Event> events) {
> > while (!events.empty()) {
> > Event event = std::move(events.front());
> > events.pop();
> > received(event);
> > }
> > }) {}
> >
> > MOCK_METHOD0_T(connected, void());
> > MOCK_METHOD0_T(disconnected, void());
> > ...
> >
> > private:
> > Driver driver;
> > };
> > ```
>
> Jan Schlicht wrote:
> The driver needs to be accessible in the test case to allow things like
> ```
> v1::MockResourceProvider resourceProvider;
> v1::resource_provider::TestDriver driver;
> ...
>
> Future<Event::Subscribed> subscribed;
> EXPECT_CALL(*resourceProvider, subscribed(_))
> .WillOnce(FutureArg<0>(&subscribed));
>
> mesos::v1::ResourceProviderInfo resourceProviderInfo;
> resourceProviderInfo.set_type("org.apache.mesos.rp.test");
> resourceProviderInfo.set_name("test");
>
> // Creates a 'SUBSCRIBE' message and sends it using the driver.
> subscribe(&driver, resourceProviderInfo);
>
> AWAIT_READY(subscribed);
> ```
>
> Jie Yu wrote:
> hum, why can't we use a real RP manager to send the message?
>
> Jan Schlicht wrote:
> Sorry, I don't understand. The driver sends `Call`s to the RP manager and receives `Event`s that it relays to `MockResourceProvider`. We need to separate driver and `MockResourceProvider` to be able to set expectations on our mock, before the driver starts. E.g. we'd expect `MockResourceProvider::connected` to be called when a connection to the RP manager has been established. We need to set this expectation (by creating a `Future` and `EXPECT_CALL` on the `MockResourceProvider` instance) before a driver instanciated, hence have to keep them separated.
I mean, `MockResourceProvider` should take `TestDriver` if you want to inject dependency for testing, instead of the reverse.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/#review181922
-----------------------------------------------------------
On Aug. 7, 2017, 6:15 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2017, 6:15 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a MockResourceProvider.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
>
>
> Diff: https://reviews.apache.org/r/61272/diff/5/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On Aug. 2, 2017, 12:37 a.m., Jie Yu wrote:
> > src/tests/mesos.hpp
> > Lines 2287-2291 (patched)
> > <https://reviews.apache.org/r/61272/diff/2/?file=1786382#file1786382line2287>
> >
> > This is a bit counter intuitive. I was expecting that MockResourceProvider will take a real Driver:
> > ```
> > class MockResourceProvider
> > {
> > public:
> > MockResourceProvider(
> > const URL& endpoint,
> > ContentType contentType)
> > : driver(
> > Owned<EndpointDetector>(new ConstantEndpointDetector(endpoint)),
> > contentType,
> > [this]() { connected(); },
> > [this]() { disconnected(); },
> > [this](std::queue<Event> events) {
> > while (!events.empty()) {
> > Event event = std::move(events.front());
> > events.pop();
> > received(event);
> > }
> > }) {}
> >
> > MOCK_METHOD0_T(connected, void());
> > MOCK_METHOD0_T(disconnected, void());
> > ...
> >
> > private:
> > Driver driver;
> > };
> > ```
>
> Jan Schlicht wrote:
> The driver needs to be accessible in the test case to allow things like
> ```
> v1::MockResourceProvider resourceProvider;
> v1::resource_provider::TestDriver driver;
> ...
>
> Future<Event::Subscribed> subscribed;
> EXPECT_CALL(*resourceProvider, subscribed(_))
> .WillOnce(FutureArg<0>(&subscribed));
>
> mesos::v1::ResourceProviderInfo resourceProviderInfo;
> resourceProviderInfo.set_type("org.apache.mesos.rp.test");
> resourceProviderInfo.set_name("test");
>
> // Creates a 'SUBSCRIBE' message and sends it using the driver.
> subscribe(&driver, resourceProviderInfo);
>
> AWAIT_READY(subscribed);
> ```
>
> Jie Yu wrote:
> hum, why can't we use a real RP manager to send the message?
Sorry, I don't understand. The driver sends `Call`s to the RP manager and receives `Event`s that it relays to `MockResourceProvider`. We need to separate driver and `MockResourceProvider` to be able to set expectations on our mock, before the driver starts. E.g. we'd expect `MockResourceProvider::connected` to be called when a connection to the RP manager has been established. We need to set this expectation (by creating a `Future` and `EXPECT_CALL` on the `MockResourceProvider` instance) before a driver instanciated, hence have to keep them separated.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/#review181922
-----------------------------------------------------------
On Aug. 2, 2017, 1:38 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> -----------------------------------------------------------
>
> (Updated Aug. 2, 2017, 1:38 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a MockResourceProvider.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
>
>
> Diff: https://reviews.apache.org/r/61272/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On Aug. 2, 2017, 12:37 a.m., Jie Yu wrote:
> > src/tests/mesos.hpp
> > Lines 2287-2291 (patched)
> > <https://reviews.apache.org/r/61272/diff/2/?file=1786382#file1786382line2287>
> >
> > This is a bit counter intuitive. I was expecting that MockResourceProvider will take a real Driver:
> > ```
> > class MockResourceProvider
> > {
> > public:
> > MockResourceProvider(
> > const URL& endpoint,
> > ContentType contentType)
> > : driver(
> > Owned<EndpointDetector>(new ConstantEndpointDetector(endpoint)),
> > contentType,
> > [this]() { connected(); },
> > [this]() { disconnected(); },
> > [this](std::queue<Event> events) {
> > while (!events.empty()) {
> > Event event = std::move(events.front());
> > events.pop();
> > received(event);
> > }
> > }) {}
> >
> > MOCK_METHOD0_T(connected, void());
> > MOCK_METHOD0_T(disconnected, void());
> > ...
> >
> > private:
> > Driver driver;
> > };
> > ```
The driver needs to be accessible in the test case to allow things like
```
v1::MockResourceProvider resourceProvider;
v1::resource_provider::TestDriver driver;
...
Future<Event::Subscribed> subscribed;
EXPECT_CALL(*resourceProvider, subscribed(_))
.WillOnce(FutureArg<0>(&subscribed));
mesos::v1::ResourceProviderInfo resourceProviderInfo;
resourceProviderInfo.set_type("org.apache.mesos.rp.test");
resourceProviderInfo.set_name("test");
// Creates a 'SUBSCRIBE' message and sends it using the driver.
subscribe(&driver, resourceProviderInfo);
AWAIT_READY(subscribed);
```
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/#review181922
-----------------------------------------------------------
On Aug. 2, 2017, 1:38 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> -----------------------------------------------------------
>
> (Updated Aug. 2, 2017, 1:38 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a MockResourceProvider.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
>
>
> Diff: https://reviews.apache.org/r/61272/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jie Yu <yu...@gmail.com>.
> On Aug. 1, 2017, 10:37 p.m., Jie Yu wrote:
> > src/tests/mesos.hpp
> > Lines 2287-2291 (patched)
> > <https://reviews.apache.org/r/61272/diff/2/?file=1786382#file1786382line2287>
> >
> > This is a bit counter intuitive. I was expecting that MockResourceProvider will take a real Driver:
> > ```
> > class MockResourceProvider
> > {
> > public:
> > MockResourceProvider(
> > const URL& endpoint,
> > ContentType contentType)
> > : driver(
> > Owned<EndpointDetector>(new ConstantEndpointDetector(endpoint)),
> > contentType,
> > [this]() { connected(); },
> > [this]() { disconnected(); },
> > [this](std::queue<Event> events) {
> > while (!events.empty()) {
> > Event event = std::move(events.front());
> > events.pop();
> > received(event);
> > }
> > }) {}
> >
> > MOCK_METHOD0_T(connected, void());
> > MOCK_METHOD0_T(disconnected, void());
> > ...
> >
> > private:
> > Driver driver;
> > };
> > ```
>
> Jan Schlicht wrote:
> The driver needs to be accessible in the test case to allow things like
> ```
> v1::MockResourceProvider resourceProvider;
> v1::resource_provider::TestDriver driver;
> ...
>
> Future<Event::Subscribed> subscribed;
> EXPECT_CALL(*resourceProvider, subscribed(_))
> .WillOnce(FutureArg<0>(&subscribed));
>
> mesos::v1::ResourceProviderInfo resourceProviderInfo;
> resourceProviderInfo.set_type("org.apache.mesos.rp.test");
> resourceProviderInfo.set_name("test");
>
> // Creates a 'SUBSCRIBE' message and sends it using the driver.
> subscribe(&driver, resourceProviderInfo);
>
> AWAIT_READY(subscribed);
> ```
hum, why can't we use a real RP manager to send the message?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/#review181922
-----------------------------------------------------------
On Aug. 2, 2017, 11:38 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> -----------------------------------------------------------
>
> (Updated Aug. 2, 2017, 11:38 a.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a MockResourceProvider.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
>
>
> Diff: https://reviews.apache.org/r/61272/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/#review181922
-----------------------------------------------------------
src/tests/mesos.hpp
Lines 2287-2291 (patched)
<https://reviews.apache.org/r/61272/#comment257734>
This is a bit counter intuitive. I was expecting that MockResourceProvider will take a real Driver:
```
class MockResourceProvider
{
public:
MockResourceProvider(
const URL& endpoint,
ContentType contentType)
: driver(
Owned<EndpointDetector>(new ConstantEndpointDetector(endpoint)),
contentType,
[this]() { connected(); },
[this]() { disconnected(); },
[this](std::queue<Event> events) {
while (!events.empty()) {
Event event = std::move(events.front());
events.pop();
received(event);
}
}) {}
MOCK_METHOD0_T(connected, void());
MOCK_METHOD0_T(disconnected, void());
...
private:
Driver driver;
};
```
- Jie Yu
On Aug. 1, 2017, 2:06 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2017, 2:06 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
>
>
> Diff: https://reviews.apache.org/r/61272/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On Aug. 3, 2017, noon, Benjamin Bannier wrote:
> > src/tests/mesos.hpp
> > Lines 2301 (patched)
> > <https://reviews.apache.org/r/61272/diff/3/?file=1788109#file1788109line2301>
> >
> > How about passing this as an argument instead? It semantically belongs to the remote description we already pass (`url`, `contentType`).
Makes sense. By making the crendential an additional template parameter (and setting it in the v1 namespace), this would also allow us to use a `v2::DEFAULT_CREDENTIAL` or something else in the future.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/#review182088
-----------------------------------------------------------
On Aug. 2, 2017, 1:38 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> -----------------------------------------------------------
>
> (Updated Aug. 2, 2017, 1:38 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a MockResourceProvider.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
>
>
> Diff: https://reviews.apache.org/r/61272/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/#review182088
-----------------------------------------------------------
src/tests/mesos.hpp
Lines 2258-2259 (patched)
<https://reviews.apache.org/r/61272/#comment257954>
This is not needed as these functions will be declared automatically.
src/tests/mesos.hpp
Lines 2291 (patched)
<https://reviews.apache.org/r/61272/#comment257955>
We want to construct our actual base class object of type `Driver` here, e.g.,
: Driver( ...
src/tests/mesos.hpp
Lines 2301 (patched)
<https://reviews.apache.org/r/61272/#comment257951>
How about passing this as an argument instead? It semantically belongs to the remote description we already pass (`url`, `contentType`).
src/tests/mesos.hpp
Lines 2308 (patched)
<https://reviews.apache.org/r/61272/#comment257953>
It seems weird to `move` here. `std::queue::front` returns a reference, and we need to make a copy anyway.
- Benjamin Bannier
On Aug. 2, 2017, 1:38 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> -----------------------------------------------------------
>
> (Updated Aug. 2, 2017, 1:38 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a MockResourceProvider.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
>
>
> Diff: https://reviews.apache.org/r/61272/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/
-----------------------------------------------------------
(Updated Aug. 18, 2017, 12:24 p.m.)
Review request for mesos, Benjamin Bannier and Jie Yu.
Changes
-------
Rebased.
Bugs: MESOS-7469
https://issues.apache.org/jira/browse/MESOS-7469
Repository: mesos
Description
-------
Added a MockResourceProvider.
Diffs (updated)
-----
src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
Diff: https://reviews.apache.org/r/61272/diff/7/
Changes: https://reviews.apache.org/r/61272/diff/6-7/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/#review183099
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On Aug. 15, 2017, 12:05 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> -----------------------------------------------------------
>
> (Updated Aug. 15, 2017, 12:05 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a MockResourceProvider.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
>
>
> Diff: https://reviews.apache.org/r/61272/diff/6/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/
-----------------------------------------------------------
(Updated Aug. 15, 2017, 2:05 p.m.)
Review request for mesos, Benjamin Bannier and Jie Yu.
Changes
-------
Changed `MockResourceProvider` to provide a `Driver`.
Bugs: MESOS-7469
https://issues.apache.org/jira/browse/MESOS-7469
Repository: mesos
Description
-------
Added a MockResourceProvider.
Diffs (updated)
-----
src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
Diff: https://reviews.apache.org/r/61272/diff/6/
Changes: https://reviews.apache.org/r/61272/diff/5-6/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/
-----------------------------------------------------------
(Updated Aug. 7, 2017, 8:15 p.m.)
Review request for mesos, Benjamin Bannier and Jie Yu.
Changes
-------
Rebased to support `EndpointDetector`.
Bugs: MESOS-7469
https://issues.apache.org/jira/browse/MESOS-7469
Repository: mesos
Description
-------
Added a MockResourceProvider.
Diffs (updated)
-----
src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
Diff: https://reviews.apache.org/r/61272/diff/5/
Changes: https://reviews.apache.org/r/61272/diff/4-5/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/
-----------------------------------------------------------
(Updated Aug. 3, 2017, 4:28 p.m.)
Review request for mesos, Benjamin Bannier and Jie Yu.
Changes
-------
Addressed issues.
Bugs: MESOS-7469
https://issues.apache.org/jira/browse/MESOS-7469
Repository: mesos
Description
-------
Added a MockResourceProvider.
Diffs (updated)
-----
src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
Diff: https://reviews.apache.org/r/61272/diff/4/
Changes: https://reviews.apache.org/r/61272/diff/3-4/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/
-----------------------------------------------------------
(Updated Aug. 2, 2017, 1:38 p.m.)
Review request for mesos, Benjamin Bannier and Jie Yu.
Changes
-------
Use a URL instead of a UPID.
Bugs: MESOS-7469
https://issues.apache.org/jira/browse/MESOS-7469
Repository: mesos
Description (updated)
-------
Added a MockResourceProvider.
Diffs (updated)
-----
src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
Diff: https://reviews.apache.org/r/61272/diff/3/
Changes: https://reviews.apache.org/r/61272/diff/2-3/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jie Yu <yu...@gmail.com>.
> On Aug. 1, 2017, 10:40 p.m., Jie Yu wrote:
> > src/tests/mesos.hpp
> > Lines 2255 (patched)
> > <https://reviews.apache.org/r/61272/diff/2/?file=1786382#file1786382line2255>
> >
> > I don't think we need a v0 MockResourceProvider. Let's just use v1
>
> Jan Schlicht wrote:
> This isn't a v0 MockResouceProvider, but a template that is (at the moment) only used for v1, but could be reused for v2...
> This is a common pattern in `mesos.hpp` introduced by `MockHTTPScheduler` and `MockHTTPExecutor` that I'm following here.
good point on v2.. i havent thought about that.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/#review181923
-----------------------------------------------------------
On Aug. 2, 2017, 11:38 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> -----------------------------------------------------------
>
> (Updated Aug. 2, 2017, 11:38 a.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a MockResourceProvider.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
>
>
> Diff: https://reviews.apache.org/r/61272/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On Aug. 2, 2017, 12:40 a.m., Jie Yu wrote:
> > src/tests/mesos.hpp
> > Lines 2255 (patched)
> > <https://reviews.apache.org/r/61272/diff/2/?file=1786382#file1786382line2255>
> >
> > I don't think we need a v0 MockResourceProvider. Let's just use v1
This isn't a v0 MockResouceProvider, but a template that is (at the moment) only used for v1, but could be reused for v2...
This is a common pattern in `mesos.hpp` introduced by `MockHTTPScheduler` and `MockHTTPExecutor` that I'm following here.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/#review181923
-----------------------------------------------------------
On Aug. 2, 2017, 1:38 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> -----------------------------------------------------------
>
> (Updated Aug. 2, 2017, 1:38 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a MockResourceProvider.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
>
>
> Diff: https://reviews.apache.org/r/61272/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 61272: Added a MockResourceProvider.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61272/#review181923
-----------------------------------------------------------
src/tests/mesos.hpp
Lines 2255 (patched)
<https://reviews.apache.org/r/61272/#comment257737>
I don't think we need a v0 MockResourceProvider. Let's just use v1
- Jie Yu
On Aug. 1, 2017, 2:06 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2017, 2:06 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45
>
>
> Diff: https://reviews.apache.org/r/61272/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>