You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alex Clemmer <cl...@gmail.com> on 2017/01/17 19:33:45 UTC

Review Request 55637: CMake: Added `test` target.

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

Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


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


Repository: mesos


Description
-------

This commit will introduce a `test` target to the CMake build system.
The semantics of this target are very similar to the autotools build
system, in that they build the tests, but do not run them.

Accomplishing this is somewhat complex in CMake, because CMake expects
to control the `test` target itself. We work around this by (1)
silencing the warning that `test` is a reserved target, (2) removing the
call to `enable_testing` that sets up CTest, and (3) adding our own
`test` target. As an additional insurance policy, we error out if any of
the `BUILD_TESTING` flags are defined, which would indicate that
`include(CTest)` was called.


Diffs
-----

  CMakeLists.txt c6b1eb6a7c7e6ed3650847acbec0ddd3101bb96f 
  cmake/MesosConfigure.cmake 6a9ed9dc02f5c9a6d1fce0866f19ffeafec35cdc 
  src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5 

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


Testing
-------


Thanks,

Alex Clemmer


Re: Review Request 55637: CMake: Added `test` target.

Posted by Ilya Pronin <ip...@twopensource.com>.

> On Jan. 17, 2017, 8:46 p.m., Ilya Pronin wrote:
> > I think the name "test" can confuse people because it's usually expected to be used for CTest. Maybe it would better be leave old {{tests}} and {{check}} targets? https://cmake.org/Wiki/CMakeEmulateMakeCheck
> 
> Alex Clemmer wrote:
>     That's a great question. I'm not a CMake native, so it's not clear to me how weird this would be to do. The idea was to expose a `test` target to achieve something resembling script parity. Is this idea not compelling to you?
> 
> Ilya Pronin wrote:
>     Very compelling. I just meant that `make test` is usually expected to not only build tests but also run them, i.e. act the same way as autotools generated `make check`. E.g. debhelper's `dh_auto_test` looks for `test` or `check` target in a Makefile. So I suggested that target that just builds tests should be called `tests`.
> 
> Alex Clemmer wrote:
>     Ah, ok, let me just re-state what I think is true to make sure we're on the page. Right now the autotools build has the semantics that `make check` will build + run tests, and `make test` will simply build them. So the original idea here was that `make test` achieve script parity for our own tooling. And it sounds like what you're saying is that this should actually be the goal -- the goal should be that we do something more congruent with the normal use of the CMake semantics of `make test` and then have another target (which could be called, _e.g._, `make tests`, with an `s`) that implements these semantics.
>     
>     Is that all approximately correct?
>     
>     If so, let's wait and see what `kaysoky` says. He has a better understanding for the impact of changing the semantics of the `make test` target. IMHO this is a sensible suggestion, but it's important to get input for how much work this creates for the downstream consumers of the `make test` target.

Yes, except for 1 thing: current autotools target that builds tests is called `tests` :) https://github.com/apache/mesos/blob/master/src/Makefile.am#L2490


- Ilya


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


On Jan. 17, 2017, 7:33 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55637/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 7:33 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3697
>     https://issues.apache.org/jira/browse/MESOS-3697
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit will introduce a `test` target to the CMake build system.
> The semantics of this target are very similar to the autotools build
> system, in that they build the tests, but do not run them.
> 
> Accomplishing this is somewhat complex in CMake, because CMake expects
> to control the `test` target itself. We work around this by (1)
> silencing the warning that `test` is a reserved target, (2) removing the
> call to `enable_testing` that sets up CTest, and (3) adding our own
> `test` target. As an additional insurance policy, we error out if any of
> the `BUILD_TESTING` flags are defined, which would indicate that
> `include(CTest)` was called.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c6b1eb6a7c7e6ed3650847acbec0ddd3101bb96f 
>   cmake/MesosConfigure.cmake 6a9ed9dc02f5c9a6d1fce0866f19ffeafec35cdc 
>   src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5 
> 
> Diff: https://reviews.apache.org/r/55637/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 55637: CMake: Added `test` target.

Posted by Alex Clemmer <cl...@gmail.com>.

> On Jan. 17, 2017, 8:46 p.m., Ilya Pronin wrote:
> > I think the name "test" can confuse people because it's usually expected to be used for CTest. Maybe it would better be leave old {{tests}} and {{check}} targets? https://cmake.org/Wiki/CMakeEmulateMakeCheck

That's a great question. I'm not a CMake native, so it's not clear to me how weird this would be to do. The idea was to expose a `test` target to achieve something resembling script parity. Is this idea not compelling to you?


- Alex


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


On Jan. 17, 2017, 7:33 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55637/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 7:33 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3697
>     https://issues.apache.org/jira/browse/MESOS-3697
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit will introduce a `test` target to the CMake build system.
> The semantics of this target are very similar to the autotools build
> system, in that they build the tests, but do not run them.
> 
> Accomplishing this is somewhat complex in CMake, because CMake expects
> to control the `test` target itself. We work around this by (1)
> silencing the warning that `test` is a reserved target, (2) removing the
> call to `enable_testing` that sets up CTest, and (3) adding our own
> `test` target. As an additional insurance policy, we error out if any of
> the `BUILD_TESTING` flags are defined, which would indicate that
> `include(CTest)` was called.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c6b1eb6a7c7e6ed3650847acbec0ddd3101bb96f 
>   cmake/MesosConfigure.cmake 6a9ed9dc02f5c9a6d1fce0866f19ffeafec35cdc 
>   src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5 
> 
> Diff: https://reviews.apache.org/r/55637/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 55637: CMake: Added `test` target.

Posted by Alex Clemmer <cl...@gmail.com>.

> On Jan. 17, 2017, 8:46 p.m., Ilya Pronin wrote:
> > I think the name "test" can confuse people because it's usually expected to be used for CTest. Maybe it would better be leave old {{tests}} and {{check}} targets? https://cmake.org/Wiki/CMakeEmulateMakeCheck
> 
> Alex Clemmer wrote:
>     That's a great question. I'm not a CMake native, so it's not clear to me how weird this would be to do. The idea was to expose a `test` target to achieve something resembling script parity. Is this idea not compelling to you?
> 
> Ilya Pronin wrote:
>     Very compelling. I just meant that `make test` is usually expected to not only build tests but also run them, i.e. act the same way as autotools generated `make check`. E.g. debhelper's `dh_auto_test` looks for `test` or `check` target in a Makefile. So I suggested that target that just builds tests should be called `tests`.

Ah, ok, let me just re-state what I think is true to make sure we're on the page. Right now the autotools build has the semantics that `make check` will build + run tests, and `make test` will simply build them. So the original idea here was that `make test` achieve script parity for our own tooling. And it sounds like what you're saying is that this should actually be the goal -- the goal should be that we do something more congruent with the normal use of the CMake semantics of `make test` and then have another target (which could be called, _e.g._, `make tests`, with an `s`) that implements these semantics.

Is that all approximately correct?

If so, let's wait and see what `kaysoky` says. He has a better understanding for the impact of changing the semantics of the `make test` target. IMHO this is a sensible suggestion, but it's important to get input for how much work this creates for the downstream consumers of the `make test` target.


- Alex


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


On Jan. 17, 2017, 7:33 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55637/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 7:33 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3697
>     https://issues.apache.org/jira/browse/MESOS-3697
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit will introduce a `test` target to the CMake build system.
> The semantics of this target are very similar to the autotools build
> system, in that they build the tests, but do not run them.
> 
> Accomplishing this is somewhat complex in CMake, because CMake expects
> to control the `test` target itself. We work around this by (1)
> silencing the warning that `test` is a reserved target, (2) removing the
> call to `enable_testing` that sets up CTest, and (3) adding our own
> `test` target. As an additional insurance policy, we error out if any of
> the `BUILD_TESTING` flags are defined, which would indicate that
> `include(CTest)` was called.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c6b1eb6a7c7e6ed3650847acbec0ddd3101bb96f 
>   cmake/MesosConfigure.cmake 6a9ed9dc02f5c9a6d1fce0866f19ffeafec35cdc 
>   src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5 
> 
> Diff: https://reviews.apache.org/r/55637/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 55637: CMake: Added `test` target.

Posted by Ilya Pronin <ip...@twopensource.com>.

> On Jan. 17, 2017, 8:46 p.m., Ilya Pronin wrote:
> > I think the name "test" can confuse people because it's usually expected to be used for CTest. Maybe it would better be leave old {{tests}} and {{check}} targets? https://cmake.org/Wiki/CMakeEmulateMakeCheck
> 
> Alex Clemmer wrote:
>     That's a great question. I'm not a CMake native, so it's not clear to me how weird this would be to do. The idea was to expose a `test` target to achieve something resembling script parity. Is this idea not compelling to you?

Very compelling. I just meant that `make test` is usually expected to not only build tests but also run them, i.e. act the same way as autotools generated `make check`. E.g. debhelper's `dh_auto_test` looks for `test` or `check` target in a Makefile. So I suggested that target that just builds tests should be called `tests`.


- Ilya


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


On Jan. 17, 2017, 7:33 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55637/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 7:33 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3697
>     https://issues.apache.org/jira/browse/MESOS-3697
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit will introduce a `test` target to the CMake build system.
> The semantics of this target are very similar to the autotools build
> system, in that they build the tests, but do not run them.
> 
> Accomplishing this is somewhat complex in CMake, because CMake expects
> to control the `test` target itself. We work around this by (1)
> silencing the warning that `test` is a reserved target, (2) removing the
> call to `enable_testing` that sets up CTest, and (3) adding our own
> `test` target. As an additional insurance policy, we error out if any of
> the `BUILD_TESTING` flags are defined, which would indicate that
> `include(CTest)` was called.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c6b1eb6a7c7e6ed3650847acbec0ddd3101bb96f 
>   cmake/MesosConfigure.cmake 6a9ed9dc02f5c9a6d1fce0866f19ffeafec35cdc 
>   src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5 
> 
> Diff: https://reviews.apache.org/r/55637/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 55637: CMake: Added `test` target.

Posted by Alex Clemmer <cl...@gmail.com>.

> On Jan. 17, 2017, 8:46 p.m., Ilya Pronin wrote:
> > I think the name "test" can confuse people because it's usually expected to be used for CTest. Maybe it would better be leave old {{tests}} and {{check}} targets? https://cmake.org/Wiki/CMakeEmulateMakeCheck
> 
> Alex Clemmer wrote:
>     That's a great question. I'm not a CMake native, so it's not clear to me how weird this would be to do. The idea was to expose a `test` target to achieve something resembling script parity. Is this idea not compelling to you?
> 
> Ilya Pronin wrote:
>     Very compelling. I just meant that `make test` is usually expected to not only build tests but also run them, i.e. act the same way as autotools generated `make check`. E.g. debhelper's `dh_auto_test` looks for `test` or `check` target in a Makefile. So I suggested that target that just builds tests should be called `tests`.
> 
> Alex Clemmer wrote:
>     Ah, ok, let me just re-state what I think is true to make sure we're on the page. Right now the autotools build has the semantics that `make check` will build + run tests, and `make test` will simply build them. So the original idea here was that `make test` achieve script parity for our own tooling. And it sounds like what you're saying is that this should actually be the goal -- the goal should be that we do something more congruent with the normal use of the CMake semantics of `make test` and then have another target (which could be called, _e.g._, `make tests`, with an `s`) that implements these semantics.
>     
>     Is that all approximately correct?
>     
>     If so, let's wait and see what `kaysoky` says. He has a better understanding for the impact of changing the semantics of the `make test` target. IMHO this is a sensible suggestion, but it's important to get input for how much work this creates for the downstream consumers of the `make test` target.
> 
> Ilya Pronin wrote:
>     Yes, except for 1 thing: current autotools target that builds tests is called `tests` :) https://github.com/apache/mesos/blob/master/src/Makefile.am#L2490

Oh, oh, oh. Got it. In my defense, I don't know what the hell I'm doing. :)

Let's discard the reviews that disable CTest and re-write this to define the target as `tests`.


- Alex


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


On Jan. 17, 2017, 7:33 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55637/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 7:33 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3697
>     https://issues.apache.org/jira/browse/MESOS-3697
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit will introduce a `test` target to the CMake build system.
> The semantics of this target are very similar to the autotools build
> system, in that they build the tests, but do not run them.
> 
> Accomplishing this is somewhat complex in CMake, because CMake expects
> to control the `test` target itself. We work around this by (1)
> silencing the warning that `test` is a reserved target, (2) removing the
> call to `enable_testing` that sets up CTest, and (3) adding our own
> `test` target. As an additional insurance policy, we error out if any of
> the `BUILD_TESTING` flags are defined, which would indicate that
> `include(CTest)` was called.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c6b1eb6a7c7e6ed3650847acbec0ddd3101bb96f 
>   cmake/MesosConfigure.cmake 6a9ed9dc02f5c9a6d1fce0866f19ffeafec35cdc 
>   src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5 
> 
> Diff: https://reviews.apache.org/r/55637/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 55637: CMake: Added `test` target.

Posted by Ilya Pronin <ip...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55637/#review161940
-----------------------------------------------------------



I think the name "test" can confuse people because it's usually expected to be used for CTest. Maybe it would better be leave old {{tests}} and {{check}} targets? https://cmake.org/Wiki/CMakeEmulateMakeCheck

- Ilya Pronin


On Jan. 17, 2017, 7:33 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55637/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 7:33 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3697
>     https://issues.apache.org/jira/browse/MESOS-3697
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit will introduce a `test` target to the CMake build system.
> The semantics of this target are very similar to the autotools build
> system, in that they build the tests, but do not run them.
> 
> Accomplishing this is somewhat complex in CMake, because CMake expects
> to control the `test` target itself. We work around this by (1)
> silencing the warning that `test` is a reserved target, (2) removing the
> call to `enable_testing` that sets up CTest, and (3) adding our own
> `test` target. As an additional insurance policy, we error out if any of
> the `BUILD_TESTING` flags are defined, which would indicate that
> `include(CTest)` was called.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c6b1eb6a7c7e6ed3650847acbec0ddd3101bb96f 
>   cmake/MesosConfigure.cmake 6a9ed9dc02f5c9a6d1fce0866f19ffeafec35cdc 
>   src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5 
> 
> Diff: https://reviews.apache.org/r/55637/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 55637: CMake: Added `test` target.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55637/#review163366
-----------------------------------------------------------


Ship it!




LGTM, except your review description is out of date.

I'll replace part of it with:
```
    This commit will introduce a "tests" target to the CMake build system.
    The semantics of this target are very similar to the autotools build
    target "test" (notice the extra "s" in the CMake target), in that
    they build the tests, but do not run them.
    
    We use "tests" in CMake because CMake expects to control the "test"
    target itself.
```

- Joseph Wu


On Jan. 18, 2017, 3:34 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55637/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2017, 3:34 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3697
>     https://issues.apache.org/jira/browse/MESOS-3697
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit will introduce a `test` target to the CMake build system.
> The semantics of this target are very similar to the autotools build
> system, in that they build the tests, but do not run them.
> 
> Accomplishing this is somewhat complex in CMake, because CMake expects
> to control the `test` target itself. We work around this by (1)
> silencing the warning that `test` is a reserved target, (2) removing the
> call to `enable_testing` that sets up CTest, and (3) adding our own
> `test` target. As an additional insurance policy, we error out if any of
> the `BUILD_TESTING` flags are defined, which would indicate that
> `include(CTest)` was called.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c6b1eb6a7c7e6ed3650847acbec0ddd3101bb96f 
> 
> Diff: https://reviews.apache.org/r/55637/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 55637: CMake: Added `test` target.

Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55637/
-----------------------------------------------------------

(Updated Jan. 18, 2017, 11:34 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


Changes
-------

Addressed Ilya's very good point.


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


Repository: mesos


Description
-------

This commit will introduce a `test` target to the CMake build system.
The semantics of this target are very similar to the autotools build
system, in that they build the tests, but do not run them.

Accomplishing this is somewhat complex in CMake, because CMake expects
to control the `test` target itself. We work around this by (1)
silencing the warning that `test` is a reserved target, (2) removing the
call to `enable_testing` that sets up CTest, and (3) adding our own
`test` target. As an additional insurance policy, we error out if any of
the `BUILD_TESTING` flags are defined, which would indicate that
`include(CTest)` was called.


Diffs (updated)
-----

  CMakeLists.txt c6b1eb6a7c7e6ed3650847acbec0ddd3101bb96f 

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


Testing
-------


Thanks,

Alex Clemmer


Re: Review Request 55637: CMake: Added `test` target.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55637/#review162032
-----------------------------------------------------------



Patch looks great!

Reviews applied: [55022, 55023, 55024, 55030, 55037, 55038, 55039, 55040, 55161, 55162, 55311, 55312, 55313, 55314, 55327, 55328, 55543, 55544, 55546, 55547, 55548, 55549, 55550, 55599, 55600, 55601, 55602, 55604, 55607, 55632, 55635, 55636, 55637]

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 Jan. 17, 2017, 7:33 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55637/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 7:33 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3697
>     https://issues.apache.org/jira/browse/MESOS-3697
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit will introduce a `test` target to the CMake build system.
> The semantics of this target are very similar to the autotools build
> system, in that they build the tests, but do not run them.
> 
> Accomplishing this is somewhat complex in CMake, because CMake expects
> to control the `test` target itself. We work around this by (1)
> silencing the warning that `test` is a reserved target, (2) removing the
> call to `enable_testing` that sets up CTest, and (3) adding our own
> `test` target. As an additional insurance policy, we error out if any of
> the `BUILD_TESTING` flags are defined, which would indicate that
> `include(CTest)` was called.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c6b1eb6a7c7e6ed3650847acbec0ddd3101bb96f 
>   cmake/MesosConfigure.cmake 6a9ed9dc02f5c9a6d1fce0866f19ffeafec35cdc 
>   src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5 
> 
> Diff: https://reviews.apache.org/r/55637/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>