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