Skip to content

Commit

Permalink
Revert of Re-enable multi-process IPC tests on Android. (patchset #3
Browse files Browse the repository at this point in the history
…id:40001 of https://codereview.chromium.org/1914143004/ )

Reason for revert:
They are breaking various downstream bots:
https://uberchromegw.corp.google.com/i/chromium.android/builders/Marshmallow%20Tablet%20Tester
https://uberchromegw.corp.google.com/i/internal.client.clank/builders/asan-clang-phone

Original issue's description:
> Re-enable multi-process IPC tests on Android.
>
> This is possible by using the alternate test child implementation.
>
> Committed: https://crrev.com/3f7857b03d9560943c20e88f3802aefe082fcdbf
> Cr-Commit-Position: refs/heads/master@{#391178}

TBR=tsepez@chromium.org,amistry@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.

Review-Url: https://codereview.chromium.org/1950553003
Cr-Commit-Position: refs/heads/master@{#391498}
  • Loading branch information
mounirlamouri authored and Commit bot committed May 4, 2016
1 parent e5f4e55 commit ddf1999
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 35 deletions.
2 changes: 1 addition & 1 deletion ipc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ test("ipc_tests") {
"ipc_test_message_generator.cc",
"ipc_test_message_generator.h",
"ipc_test_messages.h",
"run_all_unittests.cc",
"sync_socket_unittest.cc",
"unix_domain_socket_util_unittest.cc",
]
Expand All @@ -200,6 +199,7 @@ test("ipc_tests") {
":test_support",
"//base",
"//base:i18n",
"//base/test:run_all_unittests",
"//base/test:test_support",
"//crypto",
"//testing/gtest",
Expand Down
38 changes: 22 additions & 16 deletions ipc/ipc_channel_posix_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,6 @@ class IPCChannelPosixTest : public base::MultiProcessTest {
static const std::string GetConnectionSocketName();
static const std::string GetChannelDirName();

bool WaitForExit(base::Process& process, int* exit_code) {
#if defined(OS_ANDROID)
return AndroidWaitForChildExitWithTimeout(
process, base::TimeDelta::Max(), exit_code);
#else
return process.WaitForExit(exit_code);
#endif // defined(OS_ANDROID)
}

protected:
void SetUp() override;
void TearDown() override;
Expand Down Expand Up @@ -267,7 +258,12 @@ TEST_F(IPCChannelPosixTest, AcceptHangTest) {
ASSERT_EQ(IPCChannelPosixTestListener::CHANNEL_ERROR, out_listener.status());
}

TEST_F(IPCChannelPosixTest, AdvancedConnected) {
#if defined(OS_ANDROID)
#define MAYBE_AdvancedConnected DISABLED_AdvancedConnected
#else
#define MAYBE_AdvancedConnected AdvancedConnected
#endif
TEST_F(IPCChannelPosixTest, MAYBE_AdvancedConnected) {
// Test creating a connection to an external process.
IPCChannelPosixTestListener listener(false);
IPC::ChannelHandle chan_handle(GetConnectionSocketName());
Expand All @@ -289,14 +285,19 @@ TEST_F(IPCChannelPosixTest, AdvancedConnected) {
channel->Send(message);
SpinRunLoop(TestTimeouts::action_timeout());
int exit_code = 0;
EXPECT_TRUE(WaitForExit(process, &exit_code));
EXPECT_TRUE(process.WaitForExit(&exit_code));
EXPECT_EQ(0, exit_code);
ASSERT_EQ(IPCChannelPosixTestListener::CHANNEL_ERROR, listener.status());
ASSERT_FALSE(channel->HasAcceptedConnection());
unlink(chan_handle.name.c_str());
}

TEST_F(IPCChannelPosixTest, ResetState) {
#if defined(OS_ANDROID)
#define MAYBE_ResetState DISABLED_ResetState
#else
#define MAYBE_ResetState ResetState
#endif
TEST_F(IPCChannelPosixTest, MAYBE_ResetState) {
// Test creating a connection to an external process. Close the connection,
// but continue to listen and make sure another external process can connect
// to us.
Expand Down Expand Up @@ -329,7 +330,7 @@ TEST_F(IPCChannelPosixTest, ResetState) {
SpinRunLoop(TestTimeouts::action_timeout());
EXPECT_TRUE(process.Terminate(0, false));
int exit_code = 0;
EXPECT_TRUE(WaitForExit(process2, &exit_code));
EXPECT_TRUE(process2.WaitForExit(&exit_code));
EXPECT_EQ(0, exit_code);
ASSERT_EQ(IPCChannelPosixTestListener::CHANNEL_ERROR, listener.status());
ASSERT_FALSE(channel->HasAcceptedConnection());
Expand Down Expand Up @@ -358,7 +359,12 @@ TEST_F(IPCChannelPosixTest, BadChannelName) {
EXPECT_FALSE(channel2->Connect());
}

TEST_F(IPCChannelPosixTest, MultiConnection) {
#if defined(OS_ANDROID)
#define MAYBE_MultiConnection DISABLED_MultiConnection
#else
#define MAYBE_MultiConnection MultiConnection
#endif
TEST_F(IPCChannelPosixTest, MAYBE_MultiConnection) {
// Test setting up a connection to an external process, and then have
// another external process attempt to connect to us.
IPCChannelPosixTestListener listener(false);
Expand All @@ -379,7 +385,7 @@ TEST_F(IPCChannelPosixTest, MultiConnection) {
ASSERT_TRUE(process2.IsValid());
SpinRunLoop(TestTimeouts::action_max_timeout());
int exit_code = 0;
EXPECT_TRUE(WaitForExit(process2, &exit_code));
EXPECT_TRUE(process2.WaitForExit(&exit_code));
EXPECT_EQ(exit_code, 0);
ASSERT_EQ(IPCChannelPosixTestListener::DENIED, listener.status());
ASSERT_TRUE(channel->HasAcceptedConnection());
Expand All @@ -388,7 +394,7 @@ TEST_F(IPCChannelPosixTest, MultiConnection) {
IPC::Message::PRIORITY_NORMAL);
channel->Send(message);
SpinRunLoop(TestTimeouts::action_timeout());
EXPECT_TRUE(WaitForExit(process, &exit_code));
EXPECT_TRUE(process.WaitForExit(&exit_code));
EXPECT_EQ(exit_code, 0);
ASSERT_EQ(IPCChannelPosixTestListener::CHANNEL_ERROR, listener.status());
ASSERT_FALSE(channel->HasAcceptedConnection());
Expand Down
21 changes: 18 additions & 3 deletions ipc/ipc_channel_proxy_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,12 @@ class IPCChannelProxyTest : public IPCTestBase {
std::unique_ptr<QuitListener> listener_;
};

TEST_F(IPCChannelProxyTest, MessageClassFilters) {
#if defined(OS_ANDROID)
#define MAYBE_MessageClassFilters DISABLED_MessageClassFilters
#else
#define MAYBE_MessageClassFilters MessageClassFilters
#endif
TEST_F(IPCChannelProxyTest, MAYBE_MessageClassFilters) {
// Construct a filter per message class.
std::vector<scoped_refptr<MessageCountFilter> > class_filters;
class_filters.push_back(make_scoped_refptr(
Expand All @@ -297,7 +302,12 @@ TEST_F(IPCChannelProxyTest, MessageClassFilters) {
EXPECT_EQ(1U, class_filters[i]->messages_received());
}

TEST_F(IPCChannelProxyTest, GlobalAndMessageClassFilters) {
#if defined(OS_ANDROID)
#define MAYBE_GlobalAndMessageClassFilters DISABLED_GlobalAndMessageClassFilters
#else
#define MAYBE_GlobalAndMessageClassFilters GlobalAndMessageClassFilters
#endif
TEST_F(IPCChannelProxyTest, MAYBE_GlobalAndMessageClassFilters) {
// Add a class and global filter.
scoped_refptr<MessageCountFilter> class_filter(
new MessageCountFilter(TestMsgStart));
Expand Down Expand Up @@ -326,7 +336,12 @@ TEST_F(IPCChannelProxyTest, GlobalAndMessageClassFilters) {
EXPECT_EQ(3U, global_filter->messages_received());
}

TEST_F(IPCChannelProxyTest, FilterRemoval) {
#if defined(OS_ANDROID)
#define MAYBE_FilterRemoval DISABLED_FilterRemoval
#else
#define MAYBE_FilterRemoval FilterRemoval
#endif
TEST_F(IPCChannelProxyTest, MAYBE_FilterRemoval) {
// Add a class and global filter.
scoped_refptr<MessageCountFilter> class_filter(
new MessageCountFilter(TestMsgStart));
Expand Down
21 changes: 18 additions & 3 deletions ipc/ipc_fuzzing_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,14 @@ MULTIPROCESS_IPC_TEST_CLIENT_MAIN(FuzzServerClient) {
class IPCFuzzingTest : public IPCTestBase {
};

#if defined(OS_ANDROID)
#define MAYBE_SanityTest DISABLED_SanityTest
#else
#define MAYBE_SanityTest SanityTest
#endif
// This test makes sure that the FuzzerClientListener and FuzzerServerListener
// are working properly by generating two well formed IPC calls.
TEST_F(IPCFuzzingTest, SanityTest) {
TEST_F(IPCFuzzingTest, MAYBE_SanityTest) {
Init("FuzzServerClient");

FuzzerClientListener listener;
Expand All @@ -296,12 +301,17 @@ TEST_F(IPCFuzzingTest, SanityTest) {
DestroyChannel();
}

#if defined(OS_ANDROID)
#define MAYBE_MsgBadPayloadShort DISABLED_MsgBadPayloadShort
#else
#define MAYBE_MsgBadPayloadShort MsgBadPayloadShort
#endif
// This test uses a payload that is smaller than expected. This generates an
// error while unpacking the IPC buffer which in debug trigger an assertion and
// in release is ignored (!). Right after we generate another valid IPC to make
// sure framing is working properly.
#if defined(NDEBUG) && !defined(DCHECK_ALWAYS_ON)
TEST_F(IPCFuzzingTest, MsgBadPayloadShort) {
TEST_F(IPCFuzzingTest, MAYBE_MsgBadPayloadShort) {
Init("FuzzServerClient");

FuzzerClientListener listener;
Expand All @@ -325,11 +335,16 @@ TEST_F(IPCFuzzingTest, MsgBadPayloadShort) {
}
#endif

#if defined(OS_ANDROID)
#define MAYBE_MsgBadPayloadArgs DISABLED_MsgBadPayloadArgs
#else
#define MAYBE_MsgBadPayloadArgs MsgBadPayloadArgs
#endif
// This test uses a payload that has too many arguments, but so the payload size
// is big enough so the unpacking routine does not generate an error as in the
// case of MsgBadPayloadShort test. This test does not pinpoint a flaw (per se)
// as by design we don't carry type information on the IPC message.
TEST_F(IPCFuzzingTest, MsgBadPayloadArgs) {
TEST_F(IPCFuzzingTest, MAYBE_MsgBadPayloadArgs) {
Init("FuzzServerClient");

FuzzerClientListener listener;
Expand Down
7 changes: 6 additions & 1 deletion ipc/ipc_send_fds_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,12 @@ class IPCSendFdsTest : public IPCTestBase {
}
};

TEST_F(IPCSendFdsTest, DescriptorTest) {
#if defined(OS_ANDROID)
#define MAYBE_DescriptorTest DISABLED_DescriptorTest
#else
#define MAYBE_DescriptorTest DescriptorTest
#endif
TEST_F(IPCSendFdsTest, MAYBE_DescriptorTest) {
Init("SendFdsClient");
RunServer();
}
Expand Down
5 changes: 0 additions & 5 deletions ipc/ipc_test_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,8 @@ bool IPCTestBase::WaitForClientShutdown() {
DCHECK(client_process_.IsValid());

int exit_code;
#if defined(OS_ANDROID)
bool rv = AndroidWaitForChildExitWithTimeout(
client_process_, base::TimeDelta::FromSeconds(5), &exit_code);
#else
bool rv = client_process_.WaitForExitWithTimeout(
base::TimeDelta::FromSeconds(5), &exit_code);
#endif // defined(OS_ANDROID)
client_process_.Close();
return rv;
}
Expand Down
8 changes: 7 additions & 1 deletion ipc/mojo/ipc_mojo_bootstrap_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,13 @@ void TestingDelegate::OnBootstrapError() {
quit_callback_.Run();
}

TEST_F(IPCMojoBootstrapTest, Connect) {
// Times out on Android; see http://crbug.com/502290
#if defined(OS_ANDROID)
#define MAYBE_Connect DISABLED_Connect
#else
#define MAYBE_Connect Connect
#endif
TEST_F(IPCMojoBootstrapTest, MAYBE_Connect) {
base::MessageLoop message_loop;
base::RunLoop run_loop;
TestingDelegate delegate(run_loop.QuitClosure());
Expand Down
3 changes: 0 additions & 3 deletions ipc/run_all_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "base/bind.h"
#include "base/test/launcher/unit_test_launcher.h"
#include "base/test/multiprocess_test.h"
#include "base/test/test_suite.h"
#include "build/build_config.h"

Expand All @@ -15,8 +14,6 @@

int main(int argc, char** argv) {
#if defined(OS_ANDROID)
base::InitAndroidMultiProcessTestHelper(main);

JNIEnv* env = base::android::AttachCurrentThread();
base::RegisterContentUriTestUtils(env);
#endif
Expand Down
14 changes: 12 additions & 2 deletions ipc/sync_socket_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,12 @@ class SyncSocketClientListener : public IPC::Listener {
class SyncSocketTest : public IPCTestBase {
};

TEST_F(SyncSocketTest, SanityTest) {
#if defined(OS_ANDROID)
#define MAYBE_SanityTest DISABLED_SanityTest
#else
#define MAYBE_SanityTest SanityTest
#endif
TEST_F(SyncSocketTest, MAYBE_SanityTest) {
Init("SyncSocketServerClient");

SyncSocketClientListener listener;
Expand Down Expand Up @@ -249,8 +254,13 @@ TEST_F(SyncSocketTest, DisconnectTest) {
EXPECT_EQ(0U, received);
}

#if defined(OS_ANDROID)
#define MAYBE_BlockingReceiveTest DISABLED_BlockingReceiveTest
#else
#define MAYBE_BlockingReceiveTest BlockingReceiveTest
#endif
// Tests that read is a blocking operation.
TEST_F(SyncSocketTest, BlockingReceiveTest) {
TEST_F(SyncSocketTest, MAYBE_BlockingReceiveTest) {
base::CancelableSyncSocket pair[2];
ASSERT_TRUE(base::CancelableSyncSocket::CreatePair(&pair[0], &pair[1]));

Expand Down

0 comments on commit ddf1999

Please sign in to comment.