Skip to content

Commit 279d572

Browse files
committed
Fix PipeClosedWhenNotRedirectedTest.
When checking for a file descriptor open on the child, we now execute a simple test program that just runs fstat on that file descriptor. This is more reliable than calling sh on different platforms. Bug: 26955860 TEST=Ran unittests on edison-eng and Chromium OS. Change-Id: I5d5d87095564159df1a75e78b0aed29c16bc7eb8
1 parent c5081a8 commit 279d572

File tree

4 files changed

+97
-6
lines changed

4 files changed

+97
-6
lines changed

Android.mk

+22
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,26 @@ LOCAL_STATIC_LIBRARIES := \
808808
$(bsdiff_static_libs)
809809
include $(BUILD_EXECUTABLE)
810810

811+
# test_subprocess (type: executable)
812+
# ========================================================
813+
# Test helper subprocess program.
814+
include $(CLEAR_VARS)
815+
LOCAL_MODULE := test_subprocess
816+
ifdef BRILLO
817+
LOCAL_MODULE_TAGS := eng
818+
endif
819+
LOCAL_MODULE_PATH := $(TARGET_OUT_DATA_NATIVE_TESTS)/update_engine_unittests
820+
LOCAL_MODULE_CLASS := EXECUTABLES
821+
LOCAL_CPP_EXTENSION := .cc
822+
LOCAL_CLANG := true
823+
LOCAL_CFLAGS := $(ue_common_cflags)
824+
LOCAL_CPPFLAGS := $(ue_common_cppflags)
825+
LOCAL_LDFLAGS := $(ue_common_ldflags)
826+
LOCAL_C_INCLUDES := $(ue_common_c_includes)
827+
LOCAL_SHARED_LIBRARIES := $(ue_common_shared_libraries)
828+
LOCAL_SRC_FILES := test_subprocess.cc
829+
include $(BUILD_EXECUTABLE)
830+
811831
# update_engine_unittests (type: executable)
812832
# ========================================================
813833
# Main unittest file.
@@ -817,6 +837,8 @@ ifdef BRILLO
817837
LOCAL_MODULE_TAGS := eng
818838
endif
819839
LOCAL_REQUIRED_MODULES := \
840+
test_http_server \
841+
test_subprocess \
820842
ue_unittest_bsdiff \
821843
ue_unittest_delta_generator \
822844
ue_unittest_disk_ext2_1k.img \

common/subprocess_unittest.cc

+8-6
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,15 @@ TEST_F(SubprocessTest, PipeRedirectFdTest) {
171171
// Test that a pipe file descriptor open in the parent is not open in the child.
172172
TEST_F(SubprocessTest, PipeClosedWhenNotRedirectedTest) {
173173
brillo::ScopedPipe pipe;
174-
const vector<string> cmd = {kBinPath "/sh", "-c",
175-
base::StringPrintf("echo on pipe >/proc/self/fd/%d", pipe.writer)};
174+
175+
// test_subprocess will return with the errno of fstat, which should be EBADF
176+
// if the passed file descriptor is closed in the child.
177+
const vector<string> cmd = {
178+
test_utils::GetBuildArtifactsPath("test_subprocess"),
179+
"fstat",
180+
std::to_string(pipe.writer)};
176181
EXPECT_TRUE(subprocess_.ExecFlags(
177-
cmd,
178-
0,
179-
{},
180-
base::Bind(&ExpectedResults, 1, "")));
182+
cmd, 0, {}, base::Bind(&ExpectedResults, EBADF, "")));
181183
loop_.Run();
182184
}
183185

test_subprocess.cc

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
//
2+
// Copyright (C) 2012 The Android Open Source Project
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
//
16+
17+
// This is a simple program used to test interaction with update_engine when
18+
// executing other programs. This program receives pre-programmed actions in the
19+
// command line and executes them in order.
20+
21+
#include <errno.h>
22+
#include <stdlib.h>
23+
#include <sys/stat.h>
24+
#include <sys/types.h>
25+
#include <unistd.h>
26+
27+
#include <string>
28+
29+
#define EX_USAGE_ERROR 100
30+
31+
void usage(const char* program, const char* error) {
32+
if (error)
33+
fprintf(stderr, "ERROR: %s\n", error);
34+
fprintf(stderr, "Usage: %s <cmd> [args..]\n", program);
35+
exit(EX_USAGE_ERROR);
36+
}
37+
38+
int main(int argc, char** argv, char** envp) {
39+
if (argc < 2)
40+
usage(argv[0], "No command passed");
41+
42+
std::string cmd(argv[1]);
43+
if (cmd == "fstat") {
44+
// Call fstat on the passed file descriptor number
45+
if (argc < 3)
46+
usage(argv[0], "No fd passed to fstat");
47+
int fd = atoi(argv[2]);
48+
struct stat buf;
49+
int rc = fstat(fd, &buf);
50+
if (rc < 0) {
51+
int ret = errno;
52+
perror("fstat");
53+
return ret;
54+
}
55+
return 0;
56+
}
57+
58+
usage(argv[0], "Unknown command");
59+
}

update_engine.gyp

+8
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,14 @@
457457
'test_http_server.cc',
458458
],
459459
},
460+
# Test subprocess helper.
461+
{
462+
'target_name': 'test_subprocess',
463+
'type': 'executable',
464+
'sources': [
465+
'test_subprocess.cc',
466+
],
467+
},
460468
# Main unittest file.
461469
{
462470
'target_name': 'update_engine_unittests',

0 commit comments

Comments
 (0)