Repository: mesos Updated Branches: refs/heads/master eb9ab8089 -> 40b40d9b7
Windows: Fixed quoted cases of `SubprocessTest.Flags` test. Enabling the unit tests for flags with quotes in them required switching from `cmd /c echo` to a native binary. The command prompt built-in does not perform the same command line argument parsing that native Windows binaries automatically do, so we added `test-echo.exe` for the purposes of this test. Note that the `BUILD_DIR` compile definition has to include the configuration folder for the Visual Studio generator. Review: https://reviews.apache.org/r/67075/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/40b40d9b Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/40b40d9b Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/40b40d9b Branch: refs/heads/master Commit: 40b40d9b73221388e583fc140280f1eb2b48b832 Parents: eb9ab80 Author: Radhika Jandhyala <[email protected]> Authored: Wed May 16 11:11:05 2018 -0700 Committer: Andrew Schwartzmeyer <[email protected]> Committed: Wed May 16 11:44:11 2018 -0700 ---------------------------------------------------------------------- 3rdparty/libprocess/src/tests/CMakeLists.txt | 17 +++++++-- .../libprocess/src/tests/subprocess_tests.cpp | 37 ++++++++++++++------ 3rdparty/libprocess/src/tests/test_echo.cpp | 28 +++++++++++++++ 3 files changed, 69 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/40b40d9b/3rdparty/libprocess/src/tests/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/tests/CMakeLists.txt b/3rdparty/libprocess/src/tests/CMakeLists.txt index 1b2d75d..8c1353b 100644 --- a/3rdparty/libprocess/src/tests/CMakeLists.txt +++ b/3rdparty/libprocess/src/tests/CMakeLists.txt @@ -87,14 +87,25 @@ target_link_libraries(libprocess-tests PRIVATE process-interface) # NOTE: This is for the generated gRPC headers. target_include_directories(libprocess-tests PRIVATE ${CMAKE_CURRENT_BINARY_DIR}) -target_compile_definitions( - libprocess-tests PRIVATE - BUILD_DIR="${CMAKE_CURRENT_BINARY_DIR}") +if (CMAKE_GENERATOR MATCHES "Visual Studio") + target_compile_definitions( + libprocess-tests PRIVATE + BUILD_DIR="${CMAKE_CURRENT_BINARY_DIR}/$<CONFIG>") +else () + target_compile_definitions( + libprocess-tests PRIVATE + BUILD_DIR="${CMAKE_CURRENT_BINARY_DIR}") +endif () add_executable(test-linkee EXCLUDE_FROM_ALL test_linkee.cpp) target_link_libraries(test-linkee PRIVATE process-interface) add_dependencies(libprocess-tests test-linkee) +if (WIN32) + add_executable(test-echo EXCLUDE_FROM_ALL test_echo.cpp) + add_dependencies(libprocess-tests test-echo) +endif () + if (ENABLE_SSL) add_executable(ssl-client EXCLUDE_FROM_ALL ssl_client.cpp) target_link_libraries(ssl-client PRIVATE process-interface) http://git-wip-us.apache.org/repos/asf/mesos/blob/40b40d9b/3rdparty/libprocess/src/tests/subprocess_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/tests/subprocess_tests.cpp b/3rdparty/libprocess/src/tests/subprocess_tests.cpp index 269918e..568d77b 100644 --- a/3rdparty/libprocess/src/tests/subprocess_tests.cpp +++ b/3rdparty/libprocess/src/tests/subprocess_tests.cpp @@ -782,10 +782,35 @@ TEST_F(SubprocessTest, Flags) string out = path::join(os::getcwd(), "stdout"); +#ifdef __WINDOWS__ + // The Windows version of `echo` is a built-in of the command + // prompt, and it simply reproduces the entire command line string. + // However, the flags class (and thus this test) is expecting the + // semantics of a native binary interpreting the command line + // arguments via the Windows API `CommandLineToArgv`. When a regular + // Windows application (in contrast to `echo`) gets command line + // arguments, the text is processed automatically by + // `CommandLineToArgv`, which converts the command line string into + // an array. For example, this is the output of `echo`: + // + // > cmd.exe /c echo "--s3=\"geek\"" + // "--s3=\"geek\"" + // + // With `test-echo.exe`, a small native binary that just prints its + // arguments, the output is: + // + // > test-echo.exe "--s3=\"geek\"" + // --s3="geek" + // + // This is the behavior expected by the test as the POSIX version of + // `echo` is a native binary. + string test_echo_path = path::join(BUILD_DIR, "test-echo.exe"); +#endif + Try<Subprocess> s = subprocess( #ifdef __WINDOWS__ - os::Shell::name, - {os::Shell::arg0, os::Shell::arg1, "echo"}, + test_echo_path, + {test_echo_path}, #else "/bin/echo", vector<string>(1, "echo"), @@ -831,18 +856,10 @@ TEST_F(SubprocessTest, Flags) EXPECT_EQ(flags.i, flags2.i); EXPECT_EQ(flags.s, flags2.s); EXPECT_EQ(flags.s2, flags2.s2); - // TODO(andschwa): Fix the `flags.load()` or `stringify_args` logic. - // Currently, this gets escaped to `"\"--s3=\\\"geek\\\"\""` because - // it has double quotes in it. See MESOS-8857. -#ifndef __WINDOWS__ EXPECT_EQ(flags.s3, flags2.s3); -#endif // __WINDOWS__ EXPECT_EQ(flags.d, flags2.d); EXPECT_EQ(flags.y, flags2.y); - // TODO(andschwa): Same problem as above. -#ifndef __WINDOWS__ EXPECT_EQ(flags.j, flags2.j); -#endif // __WINDOWS__ for (int i = 1; i < argc; i++) { ::free(argv[i]); http://git-wip-us.apache.org/repos/asf/mesos/blob/40b40d9b/3rdparty/libprocess/src/tests/test_echo.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/tests/test_echo.cpp b/3rdparty/libprocess/src/tests/test_echo.cpp new file mode 100644 index 0000000..5a53bc6 --- /dev/null +++ b/3rdparty/libprocess/src/tests/test_echo.cpp @@ -0,0 +1,28 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License + +#include <stdio.h> + +// This program mimic's the `echo` binary found on POSIX systems. It +// exists because the `echo` equivalent on Windows is a command-prompt +// built-in, and does not behave the same as a native binary. +// +// It prints out `argv` (except the 0th argument) separated by spaces. +int main(int argc, char** argv) +{ + for (int i = 1; i < argc; i++) { + printf("%s ", argv[i]); + } + printf("\n"); + + return 0; +}
