> On Aug. 2, 2016, 11:15 p.m., Joseph Wu wrote: > > 3rdparty/stout/tests/flags_tests.cpp, lines 231-235 > > <https://reviews.apache.org/r/50674/diff/1/?file=1459383#file1459383line231> > > > > This test will build on Windows, right? > > > > If so, you should prepend `DISABLED_` instead of `#ifdef`-ing it. Same > > with most of these tests, I believe. > > Alex Clemmer wrote: > I believe if we mark it `DISABLED_` it is disabled for all platforms. > Let's add a flag filter: `WINDOWS_DISABLED_`. > > Alex Clemmer wrote: > Oh. This won't work. `DISABLED_` is a gtest feature, not a Mesos feature, > so we have to mark any disabled tests as `DISABLED_`. Let's make a new > prefix, `DISABLED_WINDOWS_`, and adjust the test harness to strip this prefix > when the correct gtest filter is set.
Actually, that seems like it might not be the direction we want to go either. It seems that Stout and Libprocess do not have the same filtering facilities that the Agent and Master tests do, so I think we'd have to refactor that code to be in a common place, and then add code that will specifically opt non-Windows platforms in to tests marked `DISABLED_WINDOWS_`. (Or, alternatively, to make Windows platforms opt out of `WINDOWS_DISABLED_`.) Joseph, what are your thoughts here? How important is it to you to not have `#ifdef`s in this code? - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50674/#review144555 ----------------------------------------------------------- On Aug. 1, 2016, 9:27 p.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50674/ > ----------------------------------------------------------- > > (Updated Aug. 1, 2016, 9:27 p.m.) > > > Review request for mesos, Daniel Pravat and Joseph Wu. > > > Repository: mesos > > > Description > ------- > > A large number of Stout test files are currently not being built on > Windows. Many of these files contain tests for parts of Stout that have > already been ported, or require only trivial fixes to work (such as > removing `#include`s on Windows). A small minority of the tests contain > bugs that we should fix. > > This commit will add these files to the build, fix some of the > trivially-fixable tests, and disable tests that are known to fail > because of bugs, including comments explaining why and links to JIRA > issues where appropriate. > > > Diffs > ----- > > 3rdparty/stout/include/stout/mac.hpp > 91c4fdad350459b3e0bdf1744089e14ac883829a > 3rdparty/stout/include/stout/os/windows/kill.hpp > 0ddb3eeae3377688ad298a45c849c829c218890f > 3rdparty/stout/tests/CMakeLists.txt > f85dc998e8ba5f8d727933373c3e14500d0bdfae > 3rdparty/stout/tests/flags_tests.cpp > 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 > 3rdparty/stout/tests/ip_tests.cpp 4d1f1c9a4dd35da2a21bef1349a662af44bb4ba1 > 3rdparty/stout/tests/mac_tests.cpp 1ff60cf5506c1855547400aa20107ec5086d8431 > 3rdparty/stout/tests/os/rmdir_tests.cpp > ffe234baac305e26b5a29cffcdd310350d10167e > 3rdparty/stout/tests/os_tests.cpp e67444077eae55fd25945b451164b5bcc37552b0 > > Diff: https://reviews.apache.org/r/50674/diff/ > > > Testing > ------- > > > Thanks, > > Alex Clemmer > >
