> On April 11, 2017, 8:45 p.m., Neil Conway wrote:
> > I'm curious what the goal of these assertions is: to fail the test (at runtime)
if the current version of GTest is not thread-safe, right? It seems weird to be doing that
at runtime, since whether GTest is threadsafe or not seems to be a compile-time property.
> >
> > Would it make more sense to just avoid compiling these tests if `GTEST_IS_THREADSAFE`
is not defined?
Yes, we could do something like
```
#if GTEST_IS_THREADSAFE
#define TEST_THREADSAFE(test_case_name, test_name) \
TEST(test_case_name, test_name)
#else
#define TEST_THREADSAFE(test_case_name, test_name) \
TEST(test_case_name, DISABLED_##test_name)
#endif
```
similar to the `TEST_TEMP_DISABLED_ON_WINDOWS` macros, or use a `THREADSAFE_` prefix in the
test name and add a filter for these tests. I prefer the second solution, because it allows
a test case to have mutiple prefixes and will implement that.
> On April 11, 2017, 8:45 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp
> > Line 294 (original), 294 (patched)
> > <https://reviews.apache.org/r/58347/diff/1/?file=1687944#file1687944line294>
> >
> > Seems like the reference to `GTEST_IS_THREADSAFE` should be updated, here and
elsewhere.
>
> Joseph Wu wrote:
> One thing to note: Upgrading gtest to 1.8 will bring thread-safety support to Windows
( https://reviews.apache.org/r/58349/ ), so the macro will actually be defined.
Good point. I'll re-enable these tests.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58347/#review171598
-----------------------------------------------------------
On April 11, 2017, 2:27 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58347/
> -----------------------------------------------------------
>
> (Updated April 11, 2017, 2:27 p.m.)
>
>
> Review request for mesos, Neil Conway and Till Toenshoff.
>
>
> Bugs: MESOS-7193
> https://issues.apache.org/jira/browse/MESOS-7193
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Using 'GTEST_IS_THREADSAFE' in asserts to fail tests that need
> thread-safety early is problematic, because it may be undefined.
> Instead, a new assertion is introduced that evaluates
> 'GTEST_IS_THREADSAFE' and succeeds or fails accordingly.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/gtest.hpp 27077ac9047447fc4c52cc76ab26420e5bc79418
> 3rdparty/libprocess/src/tests/future_tests.cpp f21361ed1e354778bcd0357afb71300f05d3ecfd
> 3rdparty/libprocess/src/tests/io_tests.cpp 302fadc2a1894d3fd7c4f4975af3f2cc6c3a22de
> 3rdparty/libprocess/src/tests/limiter_tests.cpp b80b1da214f97b50aa7b61b79bbf683fd01116aa
> 3rdparty/libprocess/src/tests/metrics_tests.cpp d7fdb06060b273e16be27a263b5ee268842aa25c
> 3rdparty/libprocess/src/tests/process_tests.cpp bf90c7e78fd50ad7e16cc0a69a248ba71e2a7115
> 3rdparty/libprocess/src/tests/reap_tests.cpp 30518dee6c2fb904a607c7a457a5ec7366aab818
>
>
> Diff: https://reviews.apache.org/r/58347/diff/1/
>
>
> Testing
> -------
>
> libprocess-tests (using automake and CMake with macOS, Linux, Windows)
>
>
> Thanks,
>
> Jan Schlicht
>
>
|