Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47374]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On May 24, 2016, 2:02 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated May 24, 2016, 2:02 a.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> my_module_tests_CPPFLAGS = \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
>   -DBUILD_DIR=\"$(abs_top_builddir)\"  \
>   ...
>   
> my_module_tests_LDADD = libmesos_tests.la
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Joseph Wu]

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

(Updated May 23, 2016, 7:02 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
Schlicht, and Till Toenshoff.


Repository: mesos


Description
---

This gives external projects easier access to the test helpers used in
mesos tests.  

For example, a module writer may want to write a test like 
`src/tests/oversubscription_tests.cpp`.  To build and link against 
this library, the module writer would mimic the build flags for tests:
```
my_module_tests_CPPFLAGS = \
  -I$(ZOOKEEPER)/include   \
  -I$(ZOOKEEPER)/generated \
  -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
  -DBUILD_DIR=\"$(abs_top_builddir)\"  \
  ...
  
my_module_tests_LDADD = libmesos_tests.la
```


Diffs
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Joseph Wu]

> On May 23, 2016, 5:09 p.m., Adam B wrote:
> > src/Makefile.am, lines 1937-1938
> > 
> >
> > Why does libmesos_tests_la_SOURCES need to include qos_controllers code?

I don't remember the exact reason, but one of my previous iterations (not 
published) broke on this module.  It's unnecessary for the current 
implementation.


> On May 23, 2016, 5:09 p.m., Adam B wrote:
> > src/Makefile.am, lines 1991-1994
> > 
> >
> > I guess all these `tests/*.cpp` aren't needed anymore because they're 
> > covered in `libmesos_tests_la_SOURCES` which is also included in 
> > `mesos_tests_DEPENDENCIES` and `mesos_tests_LDADD`?

Yup.  These are all replaced with an `LDADD` later on.


- Joseph


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


On May 23, 2016, 7 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated May 23, 2016, 7 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> my_module_tests_CPPFLAGS = \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
>   -DBUILD_DIR=\"$(abs_top_builddir)\"  \
>   ...
>   
> my_module_tests_LDADD = libmesos_tests.la
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Joseph Wu]

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

(Updated May 23, 2016, 7 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
Schlicht, and Till Toenshoff.


Changes
---

Remove QoS controller code from common test library.


Repository: mesos


Description (updated)
---

This gives external projects easier access to the test helpers used in
mesos tests.  

For example, a module writer may want to write a test like 
`src/tests/oversubscription_tests.cpp`.  To build and link against 
this library, the module writer would mimic the build flags for tests:
```
my_module_tests_CPPFLAGS = \
  -I$(ZOOKEEPER)/include   \
  -I$(ZOOKEEPER)/generated \
  -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
  -DBUILD_DIR=\"$(abs_top_builddir)\"  \
  ...
  
my_module_tests_LDADD = libmesos_tests.la
```


Diffs (updated)
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Adam B]

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



Looks ok to me, but I'd love for @jieyu or @karya or @tillt to give it the 
final ShipIt, since they have more experience with tests in the Makefile.


src/Makefile.am (lines 1937 - 1938)


Why does libmesos_tests_la_SOURCES need to include qos_controllers code?



src/Makefile.am (lines 1991 - 1992)


I guess all these `tests/*.cpp` aren't needed anymore because they're 
covered in `libmesos_tests_la_SOURCES` which is also included in 
`mesos_tests_DEPENDENCIES` and `mesos_tests_LDADD`?


- Adam B


On May 23, 2016, 5:08 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated May 23, 2016, 5:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> my_module_CPPFLAGS =   \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
>   -DBUILD_DIR=\"$(abs_top_builddir)\"  \
>   ...
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47374]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 16, 2016, 8:02 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated May 16, 2016, 8:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Kapil Arya, and Jan 
> Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> my_module_CPPFLAGS =   \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
>   -DBUILD_DIR=\"$(abs_top_builddir)\"  \
>   ...
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Joseph Wu]

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

(Updated May 16, 2016, 1:02 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Kapil Arya, and Jan 
Schlicht.


Changes
---

Add java and python flags.


Repository: mesos


Description
---

This gives external projects easier access to the test helpers used in
mesos tests.  

For example, a module writer may want to write a test like 
`src/tests/oversubscription_tests.cpp`.  To build and link against 
this library, the module writer would mimic the build flags for tests:
```
my_module_CPPFLAGS =   \
  -I$(ZOOKEEPER)/include   \
  -I$(ZOOKEEPER)/generated \
  -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
  -DBUILD_DIR=\"$(abs_top_builddir)\"  \
  ...
```


Diffs (updated)
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Joseph Wu]

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

(Updated May 16, 2016, 11:10 a.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Kapil Arya, and Jan 
Schlicht.


Changes
---

Rebase and remove dependency.


Repository: mesos


Description
---

This gives external projects easier access to the test helpers used in
mesos tests.  

For example, a module writer may want to write a test like 
`src/tests/oversubscription_tests.cpp`.  To build and link against 
this library, the module writer would mimic the build flags for tests:
```
my_module_CPPFLAGS =   \
  -I$(ZOOKEEPER)/include   \
  -I$(ZOOKEEPER)/generated \
  -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
  -DBUILD_DIR=\"$(abs_top_builddir)\"  \
  ...
```


Diffs (updated)
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47374, 47216, 47150, 47215, 47214, 47213, 47212, 47205, 47149]

Failed command: ./support/apply-review.sh -n -r 47213

Error:
2016-05-14 03:36:17 URL:https://reviews.apache.org/r/47213/diff/raw/ 
[1540/1540] -> "47213.patch" [1]
error: 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp: does 
not exist in index

Full log: https://builds.apache.org/job/mesos-reviewbot/13061/console

- Mesos ReviewBot


On May 13, 2016, 11:31 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated May 13, 2016, 11:31 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Kapil Arya, and Jan 
> Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> my_module_CPPFLAGS =   \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
>   -DBUILD_DIR=\"$(abs_top_builddir)\"  \
>   ...
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 447bc2ab511ad173d3d911be10992be0974f4584 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>