From 14138936b5c4bdef716765d04fd926de73d9d0f7 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Sat, 18 Mar 2017 00:18:20 -0400 Subject: [PATCH] =?UTF-8?q?test:=20Compare=20ProcessInfo::Arguments()=20to?= =?UTF-8?q?=20main()=E2=80=99s=20argc/argv=20on=20POSIX?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously on macOS, the test used an OS-specific library function to recover the original argc and argv. On Linux/Android, it essentially reimplemented the very code it was testing, which didn’t make for a very good test. The new approach is to save argc and argv in main() and base the comparison on that. Bug: crashpad:30 Test: crashpad_util_test ProcessInfo.*, crashpad_test_test MainArguments.* Change-Id: I578abed3b04ae10a22f79a193bbb8b6589276c97 Reviewed-on: https://chromium-review.googlesource.com/456798 Commit-Queue: Mark Mentovai Reviewed-by: Joshua Peraza --- client/client_test.gyp | 2 +- minidump/minidump_test.gyp | 2 +- snapshot/snapshot_test.gyp | 2 +- test/gmock_main.cc | 23 ++++++++++ test/gtest_main.cc | 22 ++++++++++ test/main_arguments.cc | 38 +++++++++++++++++ test/main_arguments.h | 49 +++++++++++++++++++++ test/main_arguments_test.cc | 34 +++++++++++++++ test/test.gyp | 30 +++++++++++++ test/test_test.gyp | 3 +- util/posix/process_info_test.cc | 75 ++++++++++----------------------- util/util_test.gyp | 2 +- 12 files changed, 225 insertions(+), 57 deletions(-) create mode 100644 test/gmock_main.cc create mode 100644 test/gtest_main.cc create mode 100644 test/main_arguments.cc create mode 100644 test/main_arguments.h create mode 100644 test/main_arguments_test.cc diff --git a/client/client_test.gyp b/client/client_test.gyp index 66ec59e..dec8f60 100644 --- a/client/client_test.gyp +++ b/client/client_test.gyp @@ -23,9 +23,9 @@ 'dependencies': [ 'client.gyp:crashpad_client', '../handler/handler.gyp:crashpad_handler', + '../test/test.gyp:crashpad_gmock_main', '../test/test.gyp:crashpad_test', '../third_party/gtest/gmock.gyp:gmock', - '../third_party/gtest/gmock.gyp:gmock_main', '../third_party/gtest/gtest.gyp:gtest', '../third_party/mini_chromium/mini_chromium.gyp:base', '../util/util.gyp:crashpad_util', diff --git a/minidump/minidump_test.gyp b/minidump/minidump_test.gyp index cdae4ca..acf4d63 100644 --- a/minidump/minidump_test.gyp +++ b/minidump/minidump_test.gyp @@ -23,9 +23,9 @@ 'dependencies': [ 'minidump.gyp:crashpad_minidump', '../snapshot/snapshot_test.gyp:crashpad_snapshot_test_lib', + '../test/test.gyp:crashpad_gtest_main', '../test/test.gyp:crashpad_test', '../third_party/gtest/gtest.gyp:gtest', - '../third_party/gtest/gtest.gyp:gtest_main', '../third_party/mini_chromium/mini_chromium.gyp:base', '../util/util.gyp:crashpad_util', ], diff --git a/snapshot/snapshot_test.gyp b/snapshot/snapshot_test.gyp index 4ca9f4d..b31b060 100644 --- a/snapshot/snapshot_test.gyp +++ b/snapshot/snapshot_test.gyp @@ -57,9 +57,9 @@ 'snapshot.gyp:crashpad_snapshot_api', '../client/client.gyp:crashpad_client', '../compat/compat.gyp:crashpad_compat', + '../test/test.gyp:crashpad_gtest_main', '../test/test.gyp:crashpad_test', '../third_party/gtest/gtest.gyp:gtest', - '../third_party/gtest/gtest.gyp:gtest_main', '../third_party/mini_chromium/mini_chromium.gyp:base', '../util/util.gyp:crashpad_util', ], diff --git a/test/gmock_main.cc b/test/gmock_main.cc new file mode 100644 index 0000000..0549281 --- /dev/null +++ b/test/gmock_main.cc @@ -0,0 +1,23 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// 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 "gmock/gmock.h" +#include "gtest/gtest.h" +#include "test/main_arguments.h" + +int main(int argc, char* argv[]) { + crashpad::test::InitializeMainArguments(argc, argv); + testing::InitGoogleMock(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/gtest_main.cc b/test/gtest_main.cc new file mode 100644 index 0000000..5608d2c --- /dev/null +++ b/test/gtest_main.cc @@ -0,0 +1,22 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// 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 "gtest/gtest.h" +#include "test/main_arguments.h" + +int main(int argc, char* argv[]) { + crashpad::test::InitializeMainArguments(argc, argv); + testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/main_arguments.cc b/test/main_arguments.cc new file mode 100644 index 0000000..c28d2fa --- /dev/null +++ b/test/main_arguments.cc @@ -0,0 +1,38 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// 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 "test/main_arguments.h" + +#include "base/logging.h" + +namespace crashpad { +namespace test { + +const std::vector* g_arguments; + +void InitializeMainArguments(int argc, char* argv[]) { + CHECK(!g_arguments); + CHECK(argc); + CHECK(argv); + + g_arguments = new const std::vector(argv, argv + argc); +} + +const std::vector& GetMainArguments() { + CHECK(g_arguments); + return *g_arguments; +} + +} // namespace test +} // namespace crashpad diff --git a/test/main_arguments.h b/test/main_arguments.h new file mode 100644 index 0000000..d6c4d5c --- /dev/null +++ b/test/main_arguments.h @@ -0,0 +1,49 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// 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. + +#ifndef CRASHPAD_TEST_MAIN_ARGUMENTS_H_ +#define CRASHPAD_TEST_MAIN_ARGUMENTS_H_ + +#include +#include + +namespace crashpad { +namespace test { + +//! \brief Saves the arguments to `main()` for later use. +//! +//! Call this function from a test program’s `main()` function so that tests +//! that require access to these variables can retrieve them from +//! GetMainArguments(). +//! +//! The contents of \a argv, limited to \a argc elements, will be copied, so +//! that subsequent modifications to these variables by `main()` will not affect +//! the state returned by GetMainArguments(). +//! +//! This function must be called exactly once during the lifetime of a test +//! program. +void InitializeMainArguments(int argc, char* argv[]); + +//! \brief Retrieves pointers to the arguments to `main()`. +//! +//! Tests that need to access the original values of a test program’s `main()` +//! function’s parameters at process creation can use this function to retrieve +//! them, provided that `main()` called InitializeMainArguments() before making +//! any changes to its arguments. +const std::vector& GetMainArguments(); + +} // namespace test +} // namespace crashpad + +#endif // CRASHPAD_TEST_MAIN_ARGUMENTS_H_ diff --git a/test/main_arguments_test.cc b/test/main_arguments_test.cc new file mode 100644 index 0000000..85d1598 --- /dev/null +++ b/test/main_arguments_test.cc @@ -0,0 +1,34 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// 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 "test/main_arguments.h" + +#include "gtest/gtest.h" + +namespace crashpad { +namespace test { +namespace { + +TEST(MainArguments, GetMainArguments) { + // Make sure that InitializeMainArguments() has been called and that + // GetMainArguments() provides reasonable values. + const std::vector& arguments = GetMainArguments(); + + ASSERT_FALSE(arguments.empty()); + EXPECT_FALSE(arguments[0].empty()); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/test/test.gyp b/test/test.gyp index d84ff64..eb0a28b 100644 --- a/test/test.gyp +++ b/test/test.gyp @@ -42,6 +42,8 @@ 'mac/mach_errors.h', 'mac/mach_multiprocess.cc', 'mac/mach_multiprocess.h', + 'main_arguments.cc', + 'main_arguments.h', 'multiprocess.h', 'multiprocess_exec.h', 'multiprocess_exec_posix.cc', @@ -63,6 +65,11 @@ 'win/win_multiprocess.cc', 'win/win_multiprocess.h', ], + 'direct_dependent_settings': { + 'include_dirs': [ + '..', + ], + }, 'conditions': [ ['OS=="mac"', { 'link_settings': { @@ -87,5 +94,28 @@ }], ], }, + { + 'target_name': 'crashpad_gtest_main', + 'type': 'static_library', + 'dependencies': [ + 'crashpad_test', + '../third_party/gtest/gtest.gyp:gtest', + ], + 'sources': [ + 'gtest_main.cc', + ], + }, + { + 'target_name': 'crashpad_gmock_main', + 'type': 'static_library', + 'dependencies': [ + 'crashpad_test', + '../third_party/gtest/gmock.gyp:gmock', + '../third_party/gtest/gtest.gyp:gtest', + ], + 'sources': [ + 'gmock_main.cc', + ], + }, ], } diff --git a/test/test_test.gyp b/test/test_test.gyp index 2c58c17..053d9c8 100644 --- a/test/test_test.gyp +++ b/test/test_test.gyp @@ -22,10 +22,10 @@ 'type': 'executable', 'dependencies': [ 'crashpad_test_test_multiprocess_exec_test_child', + 'test.gyp:crashpad_gmock_main', 'test.gyp:crashpad_test', '../compat/compat.gyp:crashpad_compat', '../third_party/gtest/gmock.gyp:gmock', - '../third_party/gtest/gmock.gyp:gmock_main', '../third_party/gtest/gtest.gyp:gtest', '../third_party/mini_chromium/mini_chromium.gyp:base', '../util/util.gyp:crashpad_util', @@ -36,6 +36,7 @@ 'sources': [ 'hex_string_test.cc', 'mac/mach_multiprocess_test.cc', + 'main_arguments_test.cc', 'multiprocess_exec_test.cc', 'multiprocess_posix_test.cc', 'paths_test.cc', diff --git a/util/posix/process_info_test.cc b/util/posix/process_info_test.cc index 8d63b72..6a1a132 100644 --- a/util/posix/process_info_test.cc +++ b/util/posix/process_info_test.cc @@ -15,28 +15,21 @@ #include "util/posix/process_info.h" #include -#include #include #include +#include #include #include #include -#include "base/files/scoped_file.h" +#include "base/strings/stringprintf.h" #include "build/build_config.h" #include "gtest/gtest.h" #include "test/errors.h" +#include "test/main_arguments.h" #include "util/misc/implicit_cast.h" -#if defined(OS_MACOSX) -#include -#endif - -#if defined(OS_LINUX) || defined(OS_ANDROID) -#include -#endif - namespace crashpad { namespace test { namespace { @@ -100,51 +93,29 @@ void TestProcessSelfOrClone(const ProcessInfo& process_info) { std::vector argv; ASSERT_TRUE(process_info.Arguments(&argv)); - // gtest argv processing scrambles argv, but it leaves argc and argv[0] - // intact, so test those. + const std::vector& expect_argv = GetMainArguments(); -#if defined(OS_MACOSX) - int expect_argc = *_NSGetArgc(); - char** expect_argv = *_NSGetArgv(); -#elif defined(OS_LINUX) || defined(OS_ANDROID) - std::vector expect_arg_vector; - { - base::ScopedFILE cmdline(fopen("/proc/self/cmdline", "re")); - ASSERT_NE(nullptr, cmdline.get()) << ErrnoMessage("fopen"); + // expect_argv always contains the initial view of the arguments at the time + // the program was invoked. argv may contain this view, or it may contain the + // current view of arguments after gtest argv processing. argv may be a subset + // of expect_argv. + // + // gtest argv processing always leaves argv[0] intact, so this can be checked + // directly. + ASSERT_FALSE(expect_argv.empty()); + ASSERT_FALSE(argv.empty()); + EXPECT_EQ(expect_argv[0], argv[0]); - int expect_arg_char; - std::string expect_arg_string; - while ((expect_arg_char = fgetc(cmdline.get())) != EOF) { - if (expect_arg_char != '\0') { - expect_arg_string.append(1, expect_arg_char); - } else { - expect_arg_vector.push_back(expect_arg_string); - expect_arg_string.clear(); - } - } - ASSERT_EQ(0, ferror(cmdline.get())) << ErrnoMessage("fgetc"); - ASSERT_TRUE(expect_arg_string.empty()); + EXPECT_LE(argv.size(), expect_argv.size()); + + // Everything else in argv should have a match in expect_argv too, but things + // may have moved around. + for (size_t arg_index = 1; arg_index < argv.size(); ++arg_index) { + const std::string& arg = argv[arg_index]; + SCOPED_TRACE( + base::StringPrintf("arg_index %zu, arg %s", arg_index, arg.c_str())); + EXPECT_NE(expect_argv.end(), std::find(argv.begin(), argv.end(), arg)); } - - std::vector expect_argv_storage; - for (const std::string& expect_arg_string : expect_arg_vector) { - expect_argv_storage.push_back(expect_arg_string.c_str()); - } - - int expect_argc = expect_argv_storage.size(); - const char* const* expect_argv = - !expect_argv_storage.empty() ? &expect_argv_storage[0] : nullptr; -#else -#error Obtain expect_argc and expect_argv correctly on your system. -#endif - - int argc = implicit_cast(argv.size()); - EXPECT_EQ(expect_argc, argc); - - ASSERT_GE(expect_argc, 1); - ASSERT_GE(argc, 1); - - EXPECT_EQ(std::string(expect_argv[0]), argv[0]); } void TestSelfProcess(const ProcessInfo& process_info) { diff --git a/util/util_test.gyp b/util/util_test.gyp index bf57981..e1d2c02 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -24,9 +24,9 @@ 'util.gyp:crashpad_util', '../client/client.gyp:crashpad_client', '../compat/compat.gyp:crashpad_compat', + '../test/test.gyp:crashpad_gmock_main', '../test/test.gyp:crashpad_test', '../third_party/gtest/gmock.gyp:gmock', - '../third_party/gtest/gmock.gyp:gmock_main', '../third_party/gtest/gtest.gyp:gtest', '../third_party/mini_chromium/mini_chromium.gyp:base', '../third_party/zlib/zlib.gyp:zlib',