mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Clemmer <clemmer.alexan...@gmail.com>
Subject Re: Review Request 50674: Stout: Enabled tests that pass on Windows.
Date Tue, 11 Oct 2016 23:49:40 GMT


> 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.
> 
> Alex Clemmer wrote:
>     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?

Fixed per our discussion.


- Alex


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


On Oct. 11, 2016, 11:49 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50674/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2016, 11:49 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/gtest.hpp fca304a0be6ccfdabb351d43ee670435978c1f0f 
>   3rdparty/stout/include/stout/mac.hpp 91c4fdad350459b3e0bdf1744089e14ac883829a 
>   3rdparty/stout/tests/CMakeLists.txt 49971c7ccff319c96ed1f48cb2c9665695090688 
>   3rdparty/stout/tests/flags_tests.cpp 94ba915c40836e476cf6097274a85c55acd4d73b 
>   3rdparty/stout/tests/ip_tests.cpp 59e69a51e41b4773cb7c5a5de9f70d4810fd0294 
>   3rdparty/stout/tests/mac_tests.cpp ebd50a00585ef37ea2fa244d48bdd90036040258 
>   3rdparty/stout/tests/os/rmdir_tests.cpp ffe234baac305e26b5a29cffcdd310350d10167e 
>   3rdparty/stout/tests/os_tests.cpp 6a7b836f7102d9e014eaf9dbd47e33b987becb33 
> 
> Diff: https://reviews.apache.org/r/50674/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


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