Skip to content

Commit

Permalink
[fuchsia] Stop relying on /dev/null in tests.
Browse files Browse the repository at this point in the history
ProcessUtilTest.FDRemappingIncludesStdio opens /dev/null on other Posix
systems. The fd returned from fdio_fd_create_null can't be cloned, so we
use a temporary file on Fuchsia in place of /dev/null.

Various MessageAttachmentSet tests use /dev/null just for the sake of
having an fd. Use fdio_fd_create_null() to get the fd rather than open.

Bug: 1256502
Change-Id: Ifd5bb3e8eba6f0b77854e8af9d15e5a67c812352
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3257280
Commit-Queue: Greg Thompson <[email protected]>
Commit-Queue: Daniel Cheng <[email protected]>
Auto-Submit: Greg Thompson <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Reviewed-by: Benjamin Lerman <[email protected]>
Reviewed-by: Tom Sepez <[email protected]>
Cr-Commit-Position: refs/heads/main@{#939491}
  • Loading branch information
GregTho authored and Chromium LUCI CQ committed Nov 8, 2021
1 parent 45664b6 commit 0d56b8e
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 6 deletions.
1 change: 1 addition & 0 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3268,6 +3268,7 @@ test("base_unittests") {
"//base/third_party/dynamic_annotations",
"//build:chromecast_buildflags",
"//build:chromeos_buildflags",
"//build:os_buildflags",
"//testing/gmock",
"//testing/gtest",
"//third_party/icu",
Expand Down
24 changes: 19 additions & 5 deletions base/process/process_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "base/threading/thread.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "build/os_buildflags.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/multiprocess_func_list.h"

Expand Down Expand Up @@ -1154,8 +1155,21 @@ MULTIPROCESS_TEST_MAIN(ProcessUtilsVerifyStdio) {
}

TEST_F(ProcessUtilTest, FDRemappingIncludesStdio) {
int dev_null = open("/dev/null", O_RDONLY);
ASSERT_LT(2, dev_null);
#if BUILDFLAG(IS_FUCHSIA)
// The fd obtained from fdio_fd_create_null cannot be cloned while spawning a
// child proc, so open a true file in a transient temp dir.
ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
FilePath temp_file_path;
ASSERT_TRUE(CreateTemporaryFileInDir(temp_dir.GetPath(), &temp_file_path));
File temp_file(temp_file_path,
File::FLAG_CREATE_ALWAYS | File::FLAG_READ | File::FLAG_WRITE);
ASSERT_TRUE(temp_file.IsValid());
ScopedFD some_fd(temp_file.TakePlatformFile());
#else // BUILDFLAG(IS_FUCHSIA)
ScopedFD some_fd(open("/dev/null", O_RDONLY));
#endif // BUILDFLAG(IS_FUCHSIA)
ASSERT_LT(2, some_fd.get());

// Backup stdio and replace it with the write end of a pipe, for our
// child process to inherit.
Expand All @@ -1169,7 +1183,7 @@ TEST_F(ProcessUtilTest, FDRemappingIncludesStdio) {

// Launch the test process, which should inherit our pipe stdio.
LaunchOptions options;
options.fds_to_remap.emplace_back(dev_null, dev_null);
options.fds_to_remap.emplace_back(some_fd.get(), some_fd.get());
Process process = SpawnChildWithOptions("ProcessUtilsVerifyStdio", options);
ASSERT_TRUE(process.IsValid());

Expand All @@ -1184,8 +1198,8 @@ TEST_F(ProcessUtilTest, FDRemappingIncludesStdio) {

result = IGNORE_EINTR(close(backup_stdio));
ASSERT_EQ(0, result);
result = IGNORE_EINTR(close(dev_null));
ASSERT_EQ(0, result);
// Also close the remapped descriptor.
some_fd.reset();

// Read from the pipe to verify that it is connected to the child
// process' stdio.
Expand Down
1 change: 0 additions & 1 deletion build/config/fuchsia/test/minimum_capabilities.test-cmx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
},
"sandbox": {
"dev": [
"null",
"zero"
],
"features": [
Expand Down
1 change: 1 addition & 0 deletions ipc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ if (!is_ios) {
"//base",
"//base:i18n",
"//base/test:test_support",
"//build:os_buildflags",
"//crypto",
"//mojo/core/test:test_support",
"//testing/gtest",
Expand Down
9 changes: 9 additions & 0 deletions ipc/ipc_message_attachment_set_posix_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,24 @@

#include "base/posix/eintr_wrapper.h"
#include "build/build_config.h"
#include "build/os_buildflags.h"
#include "ipc/ipc_platform_file_attachment_posix.h"
#include "testing/gtest/include/gtest/gtest.h"

#if BUILDFLAG(IS_FUCHSIA)
#include <lib/fdio/fdio.h>
#endif

namespace IPC {
namespace {

// Get a safe file descriptor for test purposes.
int GetSafeFd() {
#if BUILDFLAG(IS_FUCHSIA)
return fdio_fd_create_null();
#else
return open("/dev/null", O_RDONLY);
#endif
}

// Returns true if fd was already closed. Closes fd if not closed.
Expand Down

0 comments on commit 0d56b8e

Please sign in to comment.