mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 55637: CMake: Added `test` target.
Date Sat, 28 Jan 2017 00:38:06 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message