Stout: Enabled tests that pass on Windows. 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. Review: https://reviews.apache.org/r/50674/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2e137e8a Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2e137e8a Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2e137e8a Branch: refs/heads/master Commit: 2e137e8a23bb0544449c0596d63fa32221868d61 Parents: 99dfdf4 Author: Alex Clemmer <clemmer.alexan...@gmail.com> Authored: Fri Oct 14 13:34:10 2016 -0700 Committer: Joseph Wu <josep...@apache.org> Committed: Fri Oct 14 16:46:54 2016 -0700 ---------------------------------------------------------------------- 3rdparty/stout/include/stout/gtest.hpp | 31 ++++++++++++ 3rdparty/stout/include/stout/mac.hpp | 6 +++ 3rdparty/stout/tests/CMakeLists.txt | 12 ++--- 3rdparty/stout/tests/flags_tests.cpp | 19 ++++++-- 3rdparty/stout/tests/ip_tests.cpp | 4 +- 3rdparty/stout/tests/mac_tests.cpp | 4 +- 3rdparty/stout/tests/os/process_tests.cpp | 4 +- 3rdparty/stout/tests/os/rmdir_tests.cpp | 8 ++- 3rdparty/stout/tests/os_tests.cpp | 67 ++++++++++++++++++++------ 9 files changed, 122 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/2e137e8a/3rdparty/stout/include/stout/gtest.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/gtest.hpp b/3rdparty/stout/include/stout/gtest.hpp index fca304a..4d8106d 100644 --- a/3rdparty/stout/include/stout/gtest.hpp +++ b/3rdparty/stout/include/stout/gtest.hpp @@ -176,6 +176,37 @@ template <typename T1, typename T2> EXPECT_TRUE(actual.isNone()) +// Creates a gtest `TEST` that is disabled on Windows. +// TODO(hausdorff): Remove after temporarily-disabled tests are fixed on +// Windows. See MESOS-6392. +#ifndef __WINDOWS__ +#define TEST_TEMP_DISABLED_ON_WINDOWS(test_case_name, test_name) \ + TEST(test_case_name, test_name) +#else +#define TEST_TEMP_DISABLED_ON_WINDOWS(test_case_name, test_name) \ + TEST(test_case_name, DISABLED_##test_name) +#endif // __WINDOWS__ + + +// Creates a gtest `TEST_F` that is disabled on Windows. +// TODO(hausdorff): Remove after temporarily-disabled tests are fixed on +// Windows. See MESOS-6392. +#ifndef __WINDOWS__ +#define TEST_F_TEMP_DISABLED_ON_WINDOWS(test_case_name, test_name) \ + TEST_F(test_case_name, test_name) +#else +#define TEST_F_TEMP_DISABLED_ON_WINDOWS(test_case_name, test_name) \ + TEST_F(test_case_name, DISABLED_##test_name) +#endif // __WINDOWS__ + + +#ifndef __WINDOWS__ +#define SLEEP_COMMAND(x) "sleep " #x +#else +#define SLEEP_COMMAND(x) "timeout " #x +#endif // __WINDOWS__ + + inline ::testing::AssertionResult AssertExited( const char* actualExpr, const int actual) http://git-wip-us.apache.org/repos/asf/mesos/blob/2e137e8a/3rdparty/stout/include/stout/mac.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/mac.hpp b/3rdparty/stout/include/stout/mac.hpp index 91c4fda..7ed5f38 100644 --- a/3rdparty/stout/include/stout/mac.hpp +++ b/3rdparty/stout/include/stout/mac.hpp @@ -20,14 +20,18 @@ #include <stdio.h> #include <string.h> +#ifndef __WINDOWS__ #include <arpa/inet.h> +#endif // __WINDOWS__ #ifdef __linux__ #include <linux/if.h> #include <linux/if_packet.h> #endif +#ifndef __WINDOWS__ #include <net/ethernet.h> +#endif // __WINDOWS__ #ifdef __APPLE__ #include <net/if.h> @@ -35,7 +39,9 @@ #include <net/if_types.h> #endif +#ifndef __WINDOWS__ #include <sys/socket.h> +#endif // __WINDOWS__ #include <sys/types.h> #include <iostream> http://git-wip-us.apache.org/repos/asf/mesos/blob/2e137e8a/3rdparty/stout/tests/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/3rdparty/stout/tests/CMakeLists.txt b/3rdparty/stout/tests/CMakeLists.txt index 49971c7..e52aa62 100644 --- a/3rdparty/stout/tests/CMakeLists.txt +++ b/3rdparty/stout/tests/CMakeLists.txt @@ -25,6 +25,7 @@ set(STOUT_TESTS_SRC duration_tests.cpp dynamiclibrary_tests.cpp error_tests.cpp + flags_tests.cpp gzip_tests.cpp hashmap_tests.cpp hashset_tests.cpp @@ -33,11 +34,13 @@ set(STOUT_TESTS_SRC json_tests.cpp jsonify_tests.cpp linkedhashmap_tests.cpp + mac_tests.cpp main.cpp multimap_tests.cpp none_tests.cpp numify_tests.cpp option_tests.cpp + os_tests.cpp os/env_tests.cpp os/filesystem_tests.cpp os/process_tests.cpp @@ -47,9 +50,12 @@ set(STOUT_TESTS_SRC os/systems_tests.cpp protobuf_tests.pb.h protobuf_tests.proto + recordio_tests.cpp result_tests.cpp some_tests.cpp strings_tests.cpp + subcommand_tests.cpp + try_tests.cpp uuid_tests.cpp version_tests.cpp ) @@ -57,16 +63,10 @@ set(STOUT_TESTS_SRC if (NOT WIN32) set(STOUT_TESTS_SRC ${STOUT_TESTS_SRC} - flags_tests.cpp - mac_tests.cpp - os_tests.cpp path_tests.cpp protobuf_tests.cpp protobuf_tests.pb.cc - recordio_tests.cpp - subcommand_tests.cpp svn_tests.cpp - try_tests.cpp os/sendfile_tests.cpp os/signals_tests.cpp ) http://git-wip-us.apache.org/repos/asf/mesos/blob/2e137e8a/3rdparty/stout/tests/flags_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/tests/flags_tests.cpp b/3rdparty/stout/tests/flags_tests.cpp index 94ba915..201b16a 100644 --- a/3rdparty/stout/tests/flags_tests.cpp +++ b/3rdparty/stout/tests/flags_tests.cpp @@ -28,6 +28,8 @@ #include <stout/some.hpp> #include <stout/utils.hpp> +#include <stout/os/write.hpp> + #include <stout/tests/utils.hpp> using flags::Flag; @@ -226,7 +228,10 @@ TEST(FlagsTest, Flags) } -TEST(FlagsTest, LoadFromEnvironment) +// TODO(hausdorff): Enable this test on Windows. Currently setting an +// environment variable to the blank string will cause the environment variable +// to be deleted on Windows. See MESOS-5880. +TEST_TEMP_DISABLED_ON_WINDOWS(FlagsTest, LoadFromEnvironment) { TestFlags flags; @@ -509,7 +514,9 @@ TEST(FlagsTest, DeprecationWarning) } -TEST(FlagsTest, DuplicatesFromEnvironment) +// TODO(hausdorff): Enable this test on Windows. Currently `flags::parse` +// assumes filesystems are rooted at '/'. See MESOS-5937. +TEST_TEMP_DISABLED_ON_WINDOWS(FlagsTest, DuplicatesFromEnvironment) { TestFlags flags; @@ -609,7 +616,7 @@ TEST(FlagsTest, Errors) { TestFlags flags; - int argc = 2; + const int argc = 2; char* argv[argc]; argv[0] = (char*) "/path/to/program"; @@ -701,7 +708,7 @@ TEST(FlagsTest, MissingRequiredFlag) "required_flag", "This flag is required and has no default value."); - int argc = 2; + const int argc = 2; char* argv[argc]; argv[0] = (char*) "/path/to/program"; @@ -933,7 +940,9 @@ TEST(FlagsTest, JSON) class FlagsFileTest : public TemporaryDirectoryTest {}; -TEST_F(FlagsFileTest, JSONFile) +// TODO(hausdorff): Enable this test on Windows. Currently `flags::parse` +// assumes filesystems are rooted at '/'. See MESOS-5937. +TEST_F_TEMP_DISABLED_ON_WINDOWS(FlagsFileTest, JSONFile) { Flags<TestFlags> flags; http://git-wip-us.apache.org/repos/asf/mesos/blob/2e137e8a/3rdparty/stout/tests/ip_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/tests/ip_tests.cpp b/3rdparty/stout/tests/ip_tests.cpp index 59e69a5..b5a206f 100644 --- a/3rdparty/stout/tests/ip_tests.cpp +++ b/3rdparty/stout/tests/ip_tests.cpp @@ -31,7 +31,9 @@ using std::string; using std::vector; -TEST(NetTest, LinkDevice) +// TODO(hausdorff): Look into enabling this test on Windows. Currently `links` +// is not implemented on Windows. See MESOS-5938. +TEST_TEMP_DISABLED_ON_WINDOWS(NetTest, LinkDevice) { Try<set<string>> links = net::links(); ASSERT_SOME(links); http://git-wip-us.apache.org/repos/asf/mesos/blob/2e137e8a/3rdparty/stout/tests/mac_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/tests/mac_tests.cpp b/3rdparty/stout/tests/mac_tests.cpp index ebd50a0..d4560e7 100644 --- a/3rdparty/stout/tests/mac_tests.cpp +++ b/3rdparty/stout/tests/mac_tests.cpp @@ -30,7 +30,9 @@ using std::string; using std::vector; -TEST(NetTest, Mac) +// TODO(hausdorff): Look into enabling this test on Windows. Currently `links` +// is not implemented on Windows. See MESOS-5938. +TEST_TEMP_DISABLED_ON_WINDOWS(NetTest, Mac) { Try<set<string>> links = net::links(); ASSERT_SOME(links); http://git-wip-us.apache.org/repos/asf/mesos/blob/2e137e8a/3rdparty/stout/tests/os/process_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/tests/os/process_tests.cpp b/3rdparty/stout/tests/os/process_tests.cpp index 4977d02..4cb3b5f 100644 --- a/3rdparty/stout/tests/os/process_tests.cpp +++ b/3rdparty/stout/tests/os/process_tests.cpp @@ -253,8 +253,8 @@ TEST_F(ProcessTest, Pstree) tree = Fork(None(), // Child. - Fork(Exec("sleep 10")), // Grandchild. - Exec("sleep 10"))(); + Fork(Exec(SLEEP_COMMAND(10))), // Grandchild. + Exec(SLEEP_COMMAND(10)))(); ASSERT_SOME(tree); http://git-wip-us.apache.org/repos/asf/mesos/blob/2e137e8a/3rdparty/stout/tests/os/rmdir_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/tests/os/rmdir_tests.cpp b/3rdparty/stout/tests/os/rmdir_tests.cpp index cd46336..9aa4059 100644 --- a/3rdparty/stout/tests/os/rmdir_tests.cpp +++ b/3rdparty/stout/tests/os/rmdir_tests.cpp @@ -234,7 +234,8 @@ TEST_F(RmdirTest, FailToRemoveNestedInvalidPath) // `mknod` will implement the functionality expressed in this test, and as the // need for these capabilities arise elsewhere in the codebase, we should // rethink abstractions we need here, and subsequently, what this test should -// look like. +// look like. This is `#ifdef`'d rather than `DISABLED_` because `rdev` doesn't +// exist on Windows. TEST_F(RmdirTest, RemoveDirectoryWithDeviceFile) { // mknod requires root permission. @@ -272,9 +273,12 @@ TEST_F(RmdirTest, RemoveDirectoryWithDeviceFile) #endif // __WINDOWS__ +// TODO(hausdorff): Look into enabling this test on Windows. Currently it is +// not possible to create a symlink on Windows unless the target exists. See +// MESOS-5881. // This test verifies that `rmdir` can remove a directory with a // symlink that has no target. -TEST_F(RmdirTest, RemoveDirectoryWithNoTargetSymbolicLink) +TEST_F_TEMP_DISABLED_ON_WINDOWS(RmdirTest, RemoveDirectoryNoTargetSymbolicLink) { const string newDirectory = path::join(os::getcwd(), "newDirectory"); ASSERT_SOME(os::mkdir(newDirectory)); http://git-wip-us.apache.org/repos/asf/mesos/blob/2e137e8a/3rdparty/stout/tests/os_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/tests/os_tests.cpp b/3rdparty/stout/tests/os_tests.cpp index 6a7b836..0b7ee07 100644 --- a/3rdparty/stout/tests/os_tests.cpp +++ b/3rdparty/stout/tests/os_tests.cpp @@ -12,7 +12,7 @@ #include <stdint.h> -#ifndef __linux__ +#if !defined(__linux__) && !defined(__WINDOWS__) #include <sys/time.h> // For gettimeofday. #endif #ifdef __FreeBSD__ @@ -39,6 +39,10 @@ #include <stout/hashset.hpp> #include <stout/numify.hpp> #include <stout/os.hpp> +#include <stout/os/environment.hpp> +#include <stout/os/kill.hpp> +#include <stout/os/killtree.hpp> +#include <stout/os/write.hpp> #include <stout/stopwatch.hpp> #include <stout/strings.hpp> #include <stout/try.hpp> @@ -50,7 +54,9 @@ #include <stout/tests/utils.hpp> +#ifndef __WINDOWS__ using os::Exec; +#endif // __WINDOWS__ using os::Fork; using os::Process; using os::ProcessTree; @@ -79,7 +85,10 @@ static bool isJailed() { class OsTest : public TemporaryDirectoryTest {}; -TEST_F(OsTest, Environment) +// TODO(hausdorff): Enable this test on Windows. Currently setting an +// environment variable to the blank string will cause the environment variable +// to be deleted on Windows. See MESOS-5880. +TEST_F_TEMP_DISABLED_ON_WINDOWS(OsTest, Environment) { // Make sure the environment has some entries with '=' in the value. os::setenv("SOME_SPECIAL_FLAG", "--flag=foobar"); @@ -117,7 +126,7 @@ TEST_F(OsTest, Argv) TEST_F(OsTest, System) { EXPECT_EQ(0, os::system("exit 0")); - EXPECT_EQ(0, os::system("sleep 0")); + EXPECT_EQ(0, os::system(SLEEP_COMMAND(0))); EXPECT_NE(0, os::system("exit 1")); EXPECT_NE(0, os::system("invalid.command")); @@ -127,7 +136,8 @@ TEST_F(OsTest, System) } -TEST_F(OsTest, Cloexec) +// NOTE: Disabled because `os::cloexec` is not implemented on Windows. +TEST_F_TEMP_DISABLED_ON_WINDOWS(OsTest, Cloexec) { Try<int> fd = os::open( "cloexec", @@ -151,6 +161,8 @@ TEST_F(OsTest, Cloexec) } +// NOTE: Disabled because `os::nonblock` doesn't exist on Windows. +#ifndef __WINDOWS__ TEST_F(OsTest, Nonblock) { int pipes[2]; @@ -170,13 +182,17 @@ TEST_F(OsTest, Nonblock) EXPECT_ERROR(os::nonblock(pipes[0])); EXPECT_ERROR(os::nonblock(pipes[1])); } +#endif // __WINDOWS__ +// TODO(hausdorff): Fix this issue and enable the test on Windows. It fails +// because `os::size` is not following symlinks correctly on Windows. See +// MESOS-5939. // Tests whether a file's size is reported by os::stat::size as expected. // Tests all four combinations of following a link or not and of a file // or a link as argument. Also tests that an error is returned for a // non-existing file. -TEST_F(OsTest, Size) +TEST_F_TEMP_DISABLED_ON_WINDOWS(OsTest, Size) { const string file = path::join(os::getcwd(), UUID::random().toString()); @@ -228,7 +244,9 @@ TEST_F(OsTest, BootId) } -TEST_F(OsTest, Sleep) +// TODO(hausdorff): Enable test on Windows after we fix. The test hangs. See +// MESOS-3441. +TEST_F_TEMP_DISABLED_ON_WINDOWS(OsTest, Sleep) { Duration duration = Milliseconds(10); Stopwatch stopwatch; @@ -292,6 +310,8 @@ TEST_F(OsTest, Sysctl) #endif // __APPLE__ || __FreeBSD__ +// TODO(hausdorff): Enable when we implement `Fork` and `Exec`. See MESOS-3638. +#ifndef __WINDOWS__ TEST_F(OsTest, Children) { Try<set<pid_t>> children = os::children(getpid()); @@ -301,8 +321,8 @@ TEST_F(OsTest, Children) Try<ProcessTree> tree = Fork(None(), // Child. - Fork(Exec("sleep 10")), // Grandchild. - Exec("sleep 10"))(); + Fork(Exec(SLEEP_COMMAND(10))), // Grandchild. + Exec(SLEEP_COMMAND(10)))(); ASSERT_SOME(tree); ASSERT_EQ(1u, tree.get().children.size()); @@ -357,10 +377,10 @@ TEST_F(OsTest, Killtree) Fork(None(), // Grandchild. Fork(None(), // Great-grandchild. Fork(&dosetsid, // Great-great-granchild. - Exec("sleep 10")), - Exec("sleep 10")), + Exec(SLEEP_COMMAND(10))), + Exec(SLEEP_COMMAND(10))), Exec("exit 0")), - Exec("sleep 10"))(); + Exec(SLEEP_COMMAND(10)))(); ASSERT_SOME(tree); @@ -479,8 +499,8 @@ TEST_F(OsTest, KilltreeNoRoot) Fork(&dosetsid, // Child. Fork(None(), // Grandchild. Fork(None(), - Exec("sleep 100")), - Exec("sleep 100")), + Exec(SLEEP_COMMAND(100))), + Exec(SLEEP_COMMAND(100))), Exec("exit 0"))(); ASSERT_SOME(tree); @@ -705,11 +725,15 @@ TEST_F(OsTest, User) EXPECT_SOME(os::setgroups(gids.get(), uid.get())); EXPECT_SOME(os::setuid(uid.get())); } +#endif // __WINDOWS__ +// TODO(hausdorff): Look into enabling this on Windows. Right now, +// `LD_LIBRARY_PATH` doesn't exist on Windows, so `setPaths` doesn't work. See +// MESOS-5940. // Test setting/resetting/appending to LD_LIBRARY_PATH environment // variable (DYLD_LIBRARY_PATH on OS X). -TEST_F(OsTest, Libraries) +TEST_F_TEMP_DISABLED_ON_WINDOWS(OsTest, Libraries) { const string path1 = "/tmp/path1"; const string path2 = "/tmp/path1"; @@ -747,7 +771,9 @@ TEST_F(OsTest, Libraries) } -TEST_F(OsTest, Shell) +// TODO(hausdorff): Port this test to Windows; these shell commands as they +// exist now don't make much sense to the Windows cmd prompt. See MESOS-3441. +TEST_F_TEMP_DISABLED_ON_WINDOWS(OsTest, Shell) { Try<string> result = os::shell("echo %s", "hello world"); EXPECT_SOME_EQ("hello world\n", result); @@ -774,6 +800,8 @@ TEST_F(OsTest, Shell) } +// NOTE: Disabled on Windows because `mknod` does not exist. +#ifndef __WINDOWS__ TEST_F(OsTest, Mknod) { // mknod requires root permission. @@ -802,9 +830,13 @@ TEST_F(OsTest, Mknod) EXPECT_SOME(os::rm(another)); } +#endif // __WINDOWS__ -TEST_F(OsTest, Realpath) +// TODO(hausdorff): Look into enabling this test on Windows. Currently it is +// not possible to create a symlink on Windows unless the target exists. See +// MESOS-5881. +TEST_F_TEMP_DISABLED_ON_WINDOWS(OsTest, Realpath) { // Create a file. const Try<string> _testFile = os::mktemp(); @@ -839,6 +871,8 @@ TEST_F(OsTest, Realpath) } +// NOTE: Disabled on Windows because `which` doesn't exist. +#ifndef __WINDOWS__ TEST_F(OsTest, Which) { // TODO(jieyu): Test PATH search ordering and file execution bit. @@ -848,3 +882,4 @@ TEST_F(OsTest, Which) which = os::which("bar"); EXPECT_NONE(which); } +#endif // __WINDOWS__
