You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2016/02/16 23:15:10 UTC
Review Request 43613: Refactor cluster test helpers into
self-contained objects.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634
Repository: mesos
Description
-------
Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
Diffs
-----
src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
Diff: https://reviews.apache.org/r/43613/diff/
Testing
-------
Tests are run at the end of this review chain.
Thanks,
Joseph Wu
Re: Review Request 43613: Refactor cluster test helpers into
self-contained objects.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/
-----------------------------------------------------------
(Updated Feb. 19, 2016, 4:50 p.m.)
Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
Changes
-------
Addressed Alexander's comments. (Also see: https://reviews.apache.org/r/43615/diff/4-5/)
Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634
Repository: mesos
Description
-------
Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
Diffs (updated)
-----
src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
Diff: https://reviews.apache.org/r/43613/diff/
Testing
-------
Tests are run at the end of this review chain.
Thanks,
Joseph Wu
Re: Review Request 43613: Refactor cluster test helpers into
self-contained objects.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Feb. 19, 2016, 3:46 p.m., Alexander Rojas wrote:
> > src/tests/cluster.hpp, line 163
> > <https://reviews.apache.org/r/43613/diff/5/?file=1263028#file1263028line163>
> >
> > Not yours (is the only unchanged line here) but if all other pointer objects are of type `process::Owned` but here it is `std::shared_ptr`. I think we could move towards consistency here and change the type to `process::Owned`.
Unfortunately, this one is a `shared_ptr` because the underlying `Master` objects expects it as a `shared_ptr` (I don't know why we break the pattern here.)
If we want to change the pattern, we would do what the TestingMesosSchedulerDriver does :)
```
// No-op destructor as _detector lives on the stack.
detector =
std::shared_ptr<MasterDetector>(_detector, [](MasterDetector*) {});
```
But I don't think this is worth the extra complexity. (We would need to copy the shared_ptr into the new no-op destructor and make sure that all future tests understand this.)
> On Feb. 19, 2016, 3:46 p.m., Alexander Rojas wrote:
> > src/tests/cluster.cpp, lines 250-251
> > <https://reviews.apache.org/r/43613/diff/5/?file=1263029#file1263029line250>
> >
> > If we stick to `std::shared_ptr` I would suggest to change its contstruction to `std::make_shared`
I'll also change the 4 tests that pass in this shared_ptr.
> On Feb. 19, 2016, 3:46 p.m., Alexander Rojas wrote:
> > src/tests/cluster.hpp, line 96
> > <https://reviews.apache.org/r/43613/diff/5/?file=1263028#file1263028line96>
> >
> > I'm pondering if calling the factory method `start()` is the right way. People are already used to the `create()` name. In that case we can also add an `start()` public method. Not an issue but an idea.
> >
> > Same goes for slave.
I was pondering this too.
I kept the "start" because this factory method calls `process::spawn` before returning.
> On Feb. 19, 2016, 3:46 p.m., Alexander Rojas wrote:
> > src/tests/cluster.cpp, line 519
> > <https://reviews.apache.org/r/43613/diff/5/?file=1263029#file1263029line519>
> >
> > Comming from a world with exceptions, destructors are not supposed to throw, which makes me feel uneasy about an `ASSERT` here.
> >
> > But feel free to drop.
A gtest assert doesn't throw, it actually returns; which I think still makes sense inside a destructor.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/#review119989
-----------------------------------------------------------
On Feb. 19, 2016, 11:36 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43613/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2016, 11:36 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
>
>
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
>
> Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
>
>
> Diffs
> -----
>
> src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
> src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
>
> Diff: https://reviews.apache.org/r/43613/diff/
>
>
> Testing
> -------
>
> Tests are run at the end of this review chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 43613: Refactor cluster test helpers into
self-contained objects.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/#review119989
-----------------------------------------------------------
src/tests/cluster.hpp (line 82)
<https://reviews.apache.org/r/43613/#comment181349>
I'm pondering if calling the factory method `start()` is the right way. People are already used to the `create()` name. In that case we can also add an `start()` public method. Not an issue but an idea.
Same goes for slave.
src/tests/cluster.hpp (lines 105 - 106)
<https://reviews.apache.org/r/43613/#comment181348>
I think we are slowly introducing the pattern of using default deleted functions in C++, if you do
```shell
ag --cpp --ignore-dir='build' '= ?delete;' .
```
you will find a bunch of instances of it. Seems like a perfect opportunity to do:
```c++
Master(const Master&) = delete;
Master& operator=(const Master&) = delete;
```
src/tests/cluster.hpp (line 122)
<https://reviews.apache.org/r/43613/#comment181352>
Not yours (is the only unchanged line here) but if all other pointer objects are of type `process::Owned` but here it is `std::shared_ptr`. I think we could move towards consistency here and change the type to `process::Owned`.
src/tests/cluster.cpp (lines 192 - 193)
<https://reviews.apache.org/r/43613/#comment181354>
If we stick to `std::shared_ptr` I would suggest to change its contstruction to `std::make_shared`
src/tests/cluster.cpp (line 374)
<https://reviews.apache.org/r/43613/#comment181357>
Given that you are calling in this same function and there's no risk of races at this point, I will recommend using `[this]() {…`.
src/tests/cluster.cpp (line 396)
<https://reviews.apache.org/r/43613/#comment181356>
Comming from a world with exceptions, destructors are not supposed to throw, which makes me feel uneasy about an `ASSERT` here.
But feel free to drop.
- Alexander Rojas
On Feb. 19, 2016, 8:36 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43613/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2016, 8:36 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
>
>
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
>
> Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
>
>
> Diffs
> -----
>
> src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
> src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
>
> Diff: https://reviews.apache.org/r/43613/diff/
>
>
> Testing
> -------
>
> Tests are run at the end of this review chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 43613: Refactor cluster test helpers into
self-contained objects.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/
-----------------------------------------------------------
(Updated Feb. 19, 2016, 11:36 a.m.)
Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
Changes
-------
Oops, missed a white-space change requested by Bernd.
Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634
Repository: mesos
Description
-------
Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
Diffs (updated)
-----
src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
Diff: https://reviews.apache.org/r/43613/diff/
Testing
-------
Tests are run at the end of this review chain.
Thanks,
Joseph Wu
Re: Review Request 43613: Refactor cluster test helpers into
self-contained objects.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/
-----------------------------------------------------------
(Updated Feb. 19, 2016, 11:25 a.m.)
Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
Changes
-------
Variety of commenting tweaks.
Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634
Repository: mesos
Description
-------
Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
Diffs (updated)
-----
src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
Diff: https://reviews.apache.org/r/43613/diff/
Testing
-------
Tests are run at the end of this review chain.
Thanks,
Joseph Wu
Re: Review Request 43613: Refactor cluster test helpers into
self-contained objects.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Feb. 19, 2016, 3:16 a.m., Bernd Mathiske wrote:
> > src/tests/cluster.hpp, line 205
> > <https://reviews.apache.org/r/43613/diff/3/?file=1253037#file1253037line205>
> >
> > Slightly unclear which is which. Attempt to clarify:
> >
> > ... Once this method has been called, its receiver will no longer perform cleanup in its destructor.
> >
> > Vernacular alternative:
> >
> > ... When this method is called, this wrapper instance will no longer perform cleanup during its destruction.
I reworded and added a tidbit about *not* destroying containers when these two methods are called.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/#review119858
-----------------------------------------------------------
On Feb. 19, 2016, 11:25 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43613/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2016, 11:25 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
>
>
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
>
> Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
>
>
> Diffs
> -----
>
> src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
> src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
>
> Diff: https://reviews.apache.org/r/43613/diff/
>
>
> Testing
> -------
>
> Tests are run at the end of this review chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 43613: Refactor cluster test helpers into
self-contained objects.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/#review119858
-----------------------------------------------------------
First pass. Sorry, did not get through it in one seating.
src/tests/cluster.hpp (line 77)
<https://reviews.apache.org/r/43613/#comment181250>
s/ / /
Here and in other places in this patch. Note that we only use one blank after a full stop everywhere else in the code base.
src/tests/cluster.hpp (line 154)
<https://reviews.apache.org/r/43613/#comment181251>
Slightly unclear which is which. Attempt to clarify:
... Once this method has been called, its receiver will no longer perform cleanup in its destructor.
Vernacular alternative:
... When this method is called, this wrapper instance will no longer perform cleanup during its destruction.
src/tests/cluster.hpp (line 159)
<https://reviews.apache.org/r/43613/#comment181252>
See above.
src/tests/cluster.hpp (line 181)
<https://reviews.apache.org/r/43613/#comment181253>
What do you mean by "only"? Does it make more sense if you erase that word?
src/tests/cluster.hpp (line 188)
<https://reviews.apache.org/r/43613/#comment181255>
How would a pointer manage anything?
src/tests/cluster.cpp (line 101)
<https://reviews.apache.org/r/43613/#comment181257>
s/NOTE: // no need to be dramatic about this little tidbit :-)
s/lambda/extra/
src/tests/cluster.cpp (line 136)
<https://reviews.apache.org/r/43613/#comment181258>
I think you can keep the first clause on the same line, then align underneath.
- Bernd Mathiske
On Feb. 17, 2016, 12:27 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43613/
> -----------------------------------------------------------
>
> (Updated Feb. 17, 2016, 12:27 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
>
>
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
>
> Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
>
>
> Diffs
> -----
>
> src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
> src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
>
> Diff: https://reviews.apache.org/r/43613/diff/
>
>
> Testing
> -------
>
> Tests are run at the end of this review chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 43613: Refactor cluster test helpers into
self-contained objects.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/
-----------------------------------------------------------
(Updated Feb. 17, 2016, 12:27 p.m.)
Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
Changes
-------
Accidentally used a default capture by reference. Changed to default capture by value.
Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634
Repository: mesos
Description
-------
Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
Diffs (updated)
-----
src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
Diff: https://reviews.apache.org/r/43613/diff/
Testing
-------
Tests are run at the end of this review chain.
Thanks,
Joseph Wu
Re: Review Request 43613: Refactor cluster test helpers into
self-contained objects.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/
-----------------------------------------------------------
(Updated Feb. 17, 2016, 10:25 a.m.)
Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
Changes
-------
Moved `populate` private helper functions into lambda closures. Expanded on cleanup comments.
Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634
Repository: mesos
Description
-------
Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
Diffs (updated)
-----
src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
Diff: https://reviews.apache.org/r/43613/diff/
Testing
-------
Tests are run at the end of this review chain.
Thanks,
Joseph Wu
Re: Review Request 43613: Refactor cluster test helpers into
self-contained objects.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Feb. 17, 2016, 8:32 a.m., Bernd Mathiske wrote:
> > src/tests/cluster.hpp, line 91
> > <https://reviews.apache.org/r/43613/diff/1/?file=1251881#file1251881line91>
> >
> > What do you mean by "some" of its state?
In both of these objects, we don't do everything possible to cleanup. Some things might be left behind (i.e. files in the sandbox, docker stuff, etc).
I'll expand the comment to include exactly what we clean up:
* We call the destructors of each owned individual injection.
* For master, we unset an authenticator.
* For agent, we destroy containers and undo some cgroups.
> On Feb. 17, 2016, 8:32 a.m., Bernd Mathiske wrote:
> > src/tests/cluster.cpp, line 106
> > <https://reviews.apache.org/r/43613/diff/1/?file=1251882#file1251882line106>
> >
> > How about using an inline closure with return type void here? Then you don't have to declare populate in the header and you don't have to pass all these parameters, you can capture them.
> >
> > AFAIK we are merely solving a syntactical problem here: macro ASSERT_* contains "return ..." with a return type that does not match start(). Right?
Yeah, that seems neater. Changing...
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/#review119464
-----------------------------------------------------------
On Feb. 16, 2016, 2:15 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43613/
> -----------------------------------------------------------
>
> (Updated Feb. 16, 2016, 2:15 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
>
>
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
>
> Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
>
>
> Diffs
> -----
>
> src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
> src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
>
> Diff: https://reviews.apache.org/r/43613/diff/
>
>
> Testing
> -------
>
> Tests are run at the end of this review chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 43613: Refactor cluster test helpers into
self-contained objects.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43613/#review119464
-----------------------------------------------------------
src/tests/cluster.hpp (line 77)
<https://reviews.apache.org/r/43613/#comment180831>
What do you mean by "some" of its state?
src/tests/cluster.cpp (line 100)
<https://reviews.apache.org/r/43613/#comment180839>
How about using an inline closure with return type void here? Then you don't have to declare populate in the header and you don't have to pass all these parameters, you can capture them.
AFAIK we are merely solving a syntactical problem here: macro ASSERT_* contains "return ..." with a return type that does not match start(). Right?
- Bernd Mathiske
On Feb. 16, 2016, 2:15 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43613/
> -----------------------------------------------------------
>
> (Updated Feb. 16, 2016, 2:15 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem Harutyunyan.
>
>
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Major rewrite of the `tests/cluster` helpers. This strongly ties the scope of test objects to the test body.
>
> Changes the `Cluster` class into two RAII objects (`Master` and `Slave`). The `Slave` object performs cleanup originally found in `cluster::Slave::stop`. `cluster::Master::start` and `cluster::Slave::start` were changed to factory methods.
>
>
> Diffs
> -----
>
> src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85
> src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625
>
> Diff: https://reviews.apache.org/r/43613/diff/
>
>
> Testing
> -------
>
> Tests are run at the end of this review chain.
>
>
> Thanks,
>
> Joseph Wu
>
>