Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-12 Thread Srinivas Brahmaroutu


> On July 12, 2016, 4:23 a.m., Alex Clemmer wrote:
> > src/tests/CMakeLists.txt, line 156
> > 
> >
> > Also, I think this is not the right command to run for Mesos tests -- 
> > can you confirm? The same goes for the containerizer memory tests. Running 
> > `make test` fails on my OS X 10.10 machine, with the following error:
> > 
> > ```
> > /usr/local/Cellar/cmake/3.4.0/bin/ctest --force-new-ctest-process
> > Test project /Users/alex/src/mesos/build
> > Start 1: StoutTests
> > 1/4 Test #1: StoutTests ...   Passed0.45 sec
> > Start 2: ProcessTests
> > 2/4 Test #2: ProcessTests .   Passed1.74 sec
> > Start 3: MesosTests
> > 3/4 Test #3: MesosTests ...***Failed0.31 sec
> > Usage: /Users/alex/src/mesos/build/src/tests/mesos-tests  > username>
> > 
> > Start 4: MesosContainerizerMemoryTests
> > 4/4 Test #4: MesosContainerizerMemoryTests ***Failed0.05 sec
> > Usage: 
> > /Users/alex/src/mesos/build/src/tests/containerizer/mesos-containerizer-memory_test
> >   [OPTIONS]
> > 
> > Available subcommands:
> > help
> > MemoryTestHelperMain
> > 
> > 
> > 
> > 50% tests passed, 2 tests failed out of 4
> > 
> > Total Test time (real) =   2.56 sec
> > 
> > The following tests FAILED:
> >   3 - MesosTests (Failed)
> >   4 - MesosContainerizerMemoryTests (Failed)
> > Errors while running CTest
> > ```
> > 
> > We will need to fix this before we check in.

Intention is not to run any tests in this patch. I require other dependencies 
from examples, etc to run the code without changin source code. What we are 
building is a dummy executable with only utility classes and the 'main' coming 
from 'active_user_test_helper.cpp'. That patched are lined up to make a 
meaningful 'mesos-tests' in the end. If you think we do it the other way, I 
will submit individual patches for examples modules first and then send in this 
patch.


- Srinivas


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


On July 12, 2016, 4:26 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49688/
> ---
> 
> (Updated July 12, 2016, 4:26 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cmake build for mesos tests.
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
>   src/tests/cmake/TestsConfigure.cmake PRE-CREATION 
>   src/tests/containerizer/CMakeLists.txt 
> dab8cb07f09f5554123ede4ec8b45b60abf62eee 
> 
> Diff: https://reviews.apache.org/r/49688/diff/
> 
> 
> Testing
> ---
> 
> cmake ..
> cmake check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-12 Thread Srinivas Brahmaroutu

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

(Updated July 12, 2016, 4:26 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Bugs: MESOS-5792
https://issues.apache.org/jira/browse/MESOS-5792


Repository: mesos


Description
---

Added cmake build for mesos tests.


Diffs (updated)
-

  src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
  src/tests/cmake/TestsConfigure.cmake PRE-CREATION 
  src/tests/containerizer/CMakeLists.txt 
dab8cb07f09f5554123ede4ec8b45b60abf62eee 

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


Testing
---

cmake ..
cmake check


Thanks,

Srinivas Brahmaroutu



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-11 Thread Alex Clemmer


> On July 12, 2016, 4:16 a.m., Alex Clemmer wrote:
> > src/tests/CMakeLists.txt, line 71
> > 
> >
> > Can we please use the Mesos style TODO format? Specifically:
> > 
> > * add semicolons after the `TODO(xxx):`
> > * capitalize the first letter of the message
> > * end message with a period
> > * indent to line up with the rest of the block (i.e., put 2 spaces 
> > between the start of the line and the `#` character)
> > 
> > In this case, it should look something like:
> > 
> > ```
> > TODO(srbrahma): Needs leveldb to compile.
> > ```
> > 
> > Also would be good to have a bug # that corresponds to the issue.
> 
> Alex Clemmer wrote:
> Actually, it looks like all of these `TODO`s are not quite correct -- we 
> do support leveldb build, and we don't need anything else except to build the 
> qos controller that I mentioned earlier. These can all be safely uncommented 
> as soon as you add that to the build.
> 
> Please do keep in mind the comment about the `TODO` formatting, though.
> 
> Alex Clemmer wrote:
> That said, I do believe you will have to add `state/leveldb.cpp` to the 
> build before that works.

(Also, don't forget that `ldcache_tests.cpp` is Linux-specific.)


- Alex


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


On July 11, 2016, 3:46 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49688/
> ---
> 
> (Updated July 11, 2016, 3:46 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cmake build for mesos tests.
> 
> 
> Diffs
> -
> 
>   cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
>   src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
>   src/tests/cmake/TestsConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49688/diff/
> 
> 
> Testing
> ---
> 
> cmake ..
> cmake check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-11 Thread Alex Clemmer


> On July 12, 2016, 4:16 a.m., Alex Clemmer wrote:
> > src/tests/CMakeLists.txt, line 71
> > 
> >
> > Can we please use the Mesos style TODO format? Specifically:
> > 
> > * add semicolons after the `TODO(xxx):`
> > * capitalize the first letter of the message
> > * end message with a period
> > * indent to line up with the rest of the block (i.e., put 2 spaces 
> > between the start of the line and the `#` character)
> > 
> > In this case, it should look something like:
> > 
> > ```
> > TODO(srbrahma): Needs leveldb to compile.
> > ```
> > 
> > Also would be good to have a bug # that corresponds to the issue.
> 
> Alex Clemmer wrote:
> Actually, it looks like all of these `TODO`s are not quite correct -- we 
> do support leveldb build, and we don't need anything else except to build the 
> qos controller that I mentioned earlier. These can all be safely uncommented 
> as soon as you add that to the build.
> 
> Please do keep in mind the comment about the `TODO` formatting, though.

That said, I do believe you will have to add `state/leveldb.cpp` to the build 
before that works.


- Alex


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


On July 11, 2016, 3:46 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49688/
> ---
> 
> (Updated July 11, 2016, 3:46 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cmake build for mesos tests.
> 
> 
> Diffs
> -
> 
>   cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
>   src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
>   src/tests/cmake/TestsConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49688/diff/
> 
> 
> Testing
> ---
> 
> cmake ..
> cmake check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-11 Thread Alex Clemmer


> On July 12, 2016, 4:16 a.m., Alex Clemmer wrote:
> > src/tests/CMakeLists.txt, line 71
> > 
> >
> > Can we please use the Mesos style TODO format? Specifically:
> > 
> > * add semicolons after the `TODO(xxx):`
> > * capitalize the first letter of the message
> > * end message with a period
> > * indent to line up with the rest of the block (i.e., put 2 spaces 
> > between the start of the line and the `#` character)
> > 
> > In this case, it should look something like:
> > 
> > ```
> > TODO(srbrahma): Needs leveldb to compile.
> > ```
> > 
> > Also would be good to have a bug # that corresponds to the issue.

Actually, it looks like all of these `TODO`s are not quite correct -- we do 
support leveldb build, and we don't need anything else except to build the qos 
controller that I mentioned earlier. These can all be safely uncommented as 
soon as you add that to the build.

Please do keep in mind the comment about the `TODO` formatting, though.


- Alex


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


On July 11, 2016, 3:46 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49688/
> ---
> 
> (Updated July 11, 2016, 3:46 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cmake build for mesos tests.
> 
> 
> Diffs
> -
> 
>   cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
>   src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
>   src/tests/cmake/TestsConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49688/diff/
> 
> 
> Testing
> ---
> 
> cmake ..
> cmake check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-11 Thread Alex Clemmer

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




src/tests/CMakeLists.txt (line 156)


Also, I think this is not the right command to run for Mesos tests -- can 
you confirm? The same goes for the containerizer memory tests. Running `make 
test` fails on my OS X 10.10 machine, with the following error:

```
/usr/local/Cellar/cmake/3.4.0/bin/ctest --force-new-ctest-process
Test project /Users/alex/src/mesos/build
Start 1: StoutTests
1/4 Test #1: StoutTests ...   Passed0.45 sec
Start 2: ProcessTests
2/4 Test #2: ProcessTests .   Passed1.74 sec
Start 3: MesosTests
3/4 Test #3: MesosTests ...***Failed0.31 sec
Usage: /Users/alex/src/mesos/build/src/tests/mesos-tests 

Start 4: MesosContainerizerMemoryTests
4/4 Test #4: MesosContainerizerMemoryTests ***Failed0.05 sec
Usage: 
/Users/alex/src/mesos/build/src/tests/containerizer/mesos-containerizer-memory_test
  [OPTIONS]

Available subcommands:
help
MemoryTestHelperMain

50% tests passed, 2 tests failed out of 4

Total Test time (real) =   2.56 sec

The following tests FAILED:
  3 - MesosTests (Failed)
  4 - MesosContainerizerMemoryTests (Failed)
Errors while running CTest
```

We will need to fix this before we check in.


- Alex Clemmer


On July 11, 2016, 3:46 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49688/
> ---
> 
> (Updated July 11, 2016, 3:46 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cmake build for mesos tests.
> 
> 
> Diffs
> -
> 
>   cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
>   src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
>   src/tests/cmake/TestsConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49688/diff/
> 
> 
> Testing
> ---
> 
> cmake ..
> cmake check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-11 Thread Alex Clemmer

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




cmake/MesosConfigure.cmake (line 94)


Can you please explain a bit why this moved? I'd like to keep all 
configuration files for tests in the `CMakeLists.txt` that build those tests, 
if it is at all possible.

Just a few notes on the design, in case the context is helpful for 
understanding why this is important. stout, libprocess, and libmesos each have 
a relevant `[ProjectName]Configure.cmake`, and 
`[ProjectName]TestsConfigure.cmake` scripts. The `[ProjectName]Configure.cmake` 
are intended to be complete, self-contained configuration scripts that will 
fully configure any project that wishes to consume those libraries -- all they 
need to do is to `include([ProjectName]Configure)` in the relevant 
`CMakeLists.txt`, and they're good to go.

In contrast, the `[ProjectName]TestsConfigure.cmake` files are included 
only in the `CMakeLists.txt` that declares the data needed to build the test 
target, and nowhere else.

The rationale for this really strong split between test and library 
configuration is to keep the test configuration from "bleeding" into the 
library configuration. This keeps the build extremely simple, because in 
general we can know that the test configuration is limited to the 
`CMakeLists.txt` that builds the test, which greatly limits the applicable 
domain of error in the build system. Since we can't write tests for the build 
system, this separation is really important: we need to keep things in the 
build system really, really simple if we want to be able to keep reasoning 
about the behavior of the build on multiple platforms.



cmake/MesosConfigure.cmake (lines 96 - 97)


Hmm, this seems like it should break the build. We're defining these 
scripts in `MesosConfigure.cmake` before we define `MESOS_TARGET`, yet 
`CONTAINERIZER_TEST_LIBS` lists `MESOS_TARGET` as a dependency, no? So we 
should fail to emit flags that link to libmesos.

Am I missing something obvious?



src/tests/CMakeLists.txt (line 17)


General comment: when we have an `if`, can we please indent the code in the 
conditional block?



src/tests/CMakeLists.txt (line 18)


The style guide is totally _ad hoc_, but can we please not add extraneous 
spaces here?



src/tests/CMakeLists.txt (line 21)


It looks like at least some of the following tests are missing, and I don't 
see them grep'ing the codebase. Rough guess: we should be including these in 
the relevant test subdirectories? If this is the rationale, can we please add 
them all now? If this is not the rationale, can we please leave a comment 
explaining?

```
  tests/common/command_utils_tests.cpp  \
  tests/common/http_tests.cpp   \
  tests/common/recordio_tests.cpp   \
  tests/containerizer/composing_containerizer_tests.cpp \
  tests/containerizer/docker_containerizer_tests.cpp\
  tests/containerizer/docker_spec_tests.cpp \
  tests/containerizer/docker_tests.cpp  \
  tests/containerizer/external_containerizer_test.cpp   \
  tests/containerizer/isolator_tests.cpp\
  tests/containerizer/launcher.cpp  \
  tests/containerizer/memory_test_helper.cpp\
  tests/containerizer/mesos_containerizer_tests.cpp \
  tests/containerizer/provisioner_appc_tests.cpp\
  tests/containerizer/provisioner_backend_tests.cpp \
  tests/containerizer/provisioner_docker_tests.cpp
```

Also, though not a test, the following file also seems to be not listed 
here:

```
slave/qos_controllers/load.cpp
```



src/tests/CMakeLists.txt (line 35)


I don't think we have a `HAS_JAVA` option, do we? Does it make sense to add 
an `option` in the root `CMakeLists.txt`, in the same way we have for the 
`VERBOSE`, `REBUNDLED`, and `ENABLE_LIBEVENT` options?

(If you don't know, an `option` exposes a binary choice to the CMake 
command line, so you can do something like `cmake .. -DENABLE_LIBEVENT=1`, 
which would compile Mesos with libevent.)



src/tests/CMakeLists.txt (line 36)


See "please indent the `if` block" comment above.



src/tests/CMakeLists.txt (line 43)


Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-11 Thread Srinivas Brahmaroutu

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

(Updated July 11, 2016, 3:46 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Bugs: MESOS-5792
https://issues.apache.org/jira/browse/MESOS-5792


Repository: mesos


Description
---

Added cmake build for mesos tests.


Diffs (updated)
-

  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
  src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
  src/tests/cmake/TestsConfigure.cmake PRE-CREATION 

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


Testing
---

cmake ..
cmake check


Thanks,

Srinivas Brahmaroutu



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-09 Thread Srinivas Brahmaroutu

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

(Updated July 10, 2016, 2:10 a.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Bugs: MESOS-5792
https://issues.apache.org/jira/browse/MESOS-5792


Repository: mesos


Description
---

Added cmake build for mesos tests.


Diffs (updated)
-

  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
  src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
  src/tests/cmake/TestsConfigure.cmake PRE-CREATION 

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


Testing
---

cmake ..
cmake check


Thanks,

Srinivas Brahmaroutu



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-09 Thread Srinivas Brahmaroutu

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

(Updated July 10, 2016, 1:49 a.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Bugs: MESOS-5792
https://issues.apache.org/jira/browse/MESOS-5792


Repository: mesos


Description
---

Added cmake build for mesos tests.


Diffs (updated)
-

  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
  src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
  src/tests/cmake/TestsConfigure.cmake PRE-CREATION 

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


Testing
---

cmake ..
cmake check


Thanks,

Srinivas Brahmaroutu



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-09 Thread Srinivas Brahmaroutu


> On July 6, 2016, 4:17 p.m., Joseph Wu wrote:
> > src/tests/CMakeLists.txt, line 17
> > 
> >
> > It does look like this MesosTestConfigure thing is unused, but I'll 
> > need to double-check with Alex Clemmer.

We use MesosTestsConfigure to run Container tests (we can rename accordingly?) 
and TestsConfigure will be used by top level tests.


> On July 6, 2016, 4:17 p.m., Joseph Wu wrote:
> > src/tests/CMakeLists.txt, line 57
> > 
> >
> > This might not work without `src/tests/main.cpp`

cmake does build a executable. It may not run at the moment until all dependent 
libraries are built.


> On July 6, 2016, 4:17 p.m., Joseph Wu wrote:
> > CMakeLists.txt, lines 110-115
> > 
> >
> > It seems like this custom target will build all three tests, and then 
> > run them sequentially afterwards.  It may be better to chain three of these 
> > custom targets together, so that we do the same thing as autotools:
> > 1) build stout, test 
> > 2) build libprocess, test
> > 3) build mesos, test

I took out the target for mesos at the moment. I will add this once the tests 
are functional in the top patch.


- Srinivas


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


On July 10, 2016, 1 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49688/
> ---
> 
> (Updated July 10, 2016, 1 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cmake build for mesos tests.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 31601a2280fa4a07df53e4e332a7e2fb0199079c 
>   cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
>   src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
>   src/tests/cmake/TestsConfigure.cmake PRE-CREATION 
>   src/tests/containerizer/CMakeLists.txt 
> dab8cb07f09f5554123ede4ec8b45b60abf62eee 
> 
> Diff: https://reviews.apache.org/r/49688/diff/
> 
> 
> Testing
> ---
> 
> cmake ..
> cmake check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-09 Thread Srinivas Brahmaroutu

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

(Updated July 10, 2016, 1 a.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Bugs: MESOS-5792
https://issues.apache.org/jira/browse/MESOS-5792


Repository: mesos


Description (updated)
---

Added cmake build for mesos tests.


Diffs (updated)
-

  CMakeLists.txt 31601a2280fa4a07df53e4e332a7e2fb0199079c 
  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
  src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
  src/tests/cmake/TestsConfigure.cmake PRE-CREATION 
  src/tests/containerizer/CMakeLists.txt 
dab8cb07f09f5554123ede4ec8b45b60abf62eee 

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


Testing
---

cmake ..
cmake check


Thanks,

Srinivas Brahmaroutu



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-06 Thread Joseph Wu

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




CMakeLists.txt (lines 110 - 115)


It seems like this custom target will build all three tests, and then run 
them sequentially afterwards.  It may be better to chain three of these custom 
targets together, so that we do the same thing as autotools:
1) build stout, test 
2) build libprocess, test
3) build mesos, test



src/tests/CMakeLists.txt (line 17)


It does look like this MesosTestConfigure thing is unused, but I'll need to 
double-check with Alex Clemmer.



src/tests/CMakeLists.txt (line 56)


This might not work without `src/tests/main.cpp`



src/tests/cmake/TestsConfigure.cmake (line 20)


s/mesos_tests/mesos-tests/



src/tests/cmake/TestsConfigure.cmake (lines 24 - 49)


Aren't these already defined elsewhere?


- Joseph Wu


On July 5, 2016, 10:23 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49688/
> ---
> 
> (Updated July 5, 2016, 10:23 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP. Added cmake build for mesos tests.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 31601a2280fa4a07df53e4e332a7e2fb0199079c 
>   src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
>   src/tests/cmake/TestsConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49688/diff/
> 
> 
> Testing
> ---
> 
> cmake ..
> cmake check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>



Re: Review Request 49688: Added cmake build for mesos tests.

2016-07-06 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [49688]

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 July 6, 2016, 5:23 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49688/
> ---
> 
> (Updated July 6, 2016, 5:23 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP. Added cmake build for mesos tests.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 31601a2280fa4a07df53e4e332a7e2fb0199079c 
>   src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
>   src/tests/cmake/TestsConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49688/diff/
> 
> 
> Testing
> ---
> 
> cmake ..
> cmake check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>



Review Request 49688: Added cmake build for mesos tests.

2016-07-05 Thread Srinivas Brahmaroutu

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Bugs: MESOS-5792
https://issues.apache.org/jira/browse/MESOS-5792


Repository: mesos


Description
---

WIP. Added cmake build for mesos tests.


Diffs
-

  CMakeLists.txt 31601a2280fa4a07df53e4e332a7e2fb0199079c 
  src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
  src/tests/cmake/TestsConfigure.cmake PRE-CREATION 

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


Testing
---

cmake ..
cmake check


Thanks,

Srinivas Brahmaroutu