New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 657080 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
please use my google.com address
Closed: Oct 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Test IPCChannelProxyTest.FilterRemoval is flaky

Project Member Reported by manisca...@chromium.org, Oct 18 2016

Issue description

It looks like test IPCChannelProxyTest.FilterRemoval is flaky.  I get about one failure in several thousand runs on my workstation.  The failures look like this:

Note: Google Test filter = IPCChannelProxyTest.FilterRemoval
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from IPCChannelProxyTest
[ RUN      ] IPCChannelProxyTest.FilterRemoval
[11932:11932:1018/092148:1035931310496:WARNING:test_suite.cc(211)] Test launcher output path /tmp/.org.chromium.Chromium.mZ2vVe/test_results.xml exists. Not adding test launcher result printer.
../../ipc/ipc_channel_proxy_unittest.cc:178: Failure
Value of: last_filter_event_
  Actual: 2
Expected: CHANNEL_CLOSING
Which is: 4
../../ipc/ipc_channel_proxy_unittest.cc:178: Failure
Value of: last_filter_event_
  Actual: 2
Expected: CHANNEL_CLOSING
Which is: 4
[  FAILED  ] IPCChannelProxyTest.FilterRemoval (25 ms)
[----------] 1 test from IPCChannelProxyTest (25 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (25 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] IPCChannelProxyTest.FilterRemoval

 1 FAILED TEST
[1018/092148:ERROR:kill_posix.cc(84)] Unable to terminate process group 11927: No such process
[1/1] IPCChannelProxyTest.FilterRemoval (25 ms)
1 test failed:
    IPCChannelProxyTest.FilterRemoval (../../ipc/ipc_channel_proxy_unittest.cc:330)
Tests took 0 seconds.

 
xyzzyz pointed out that you can repro reliably by inserting a sleep (of about 1 second) in between lines 338 and 339:

https://chromium.googlesource.com/chromium/src/+/e0072ebd0fa7c6b465d71ab1032f2e113c7e7695/ipc/ipc_channel_proxy_unittest.cc#338

I'll upload a patch that demonstrates the failure.

This patch demonstrates the failure: https://codereview.chromium.org/2428983002/
Looks like the sleep can be even earlier (e.g. at the end of SetUp).

When the EXPECT on line 178 executes, the value could be NONE, CHANNEL_CLOSING, or CHANNEL_CONNECTED.  It's not clear to me if all three are valid values.

Cc: roc...@chromium.org
Owner: manisca...@chromium.org
Status: Available (was: Untriaged)
+rockot to get his opinion.

Ken, can you take a look at this test (picked you because you're an OWNER and likely know more about it than I do :)

On line 177, what are the valid values for last_filter_event_?

https://chromium.googlesource.com/chromium/src/+/e0072ebd0fa7c6b465d71ab1032f2e113c7e7695/ipc/ipc_channel_proxy_unittest.cc#177

Should we also accept CHANNEL_CONNECTED here?

Comment 5 by xyzzyz@chromium.org, Oct 21 2016

Ken, ping?

Comment 6 by roc...@chromium.org, Oct 21 2016

I haven't had time to look yet. Will do some time today.

Comment 7 by roc...@chromium.org, Oct 21 2016

Cc: manisca...@chromium.org
Owner: roc...@chromium.org
Status: Assigned (was: Available)
That code is pretty confusing. The expectations in MessageCountFilter are clearly wrong and CHANNEL_CONNECTED should also be accepted... however, I'm not sure simply hacking in a broader expectation is the right thing to here. Let me see about cleaning this up a bit.
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a96cca441f37f144074b24e713f75ca553a1c20

commit 8a96cca441f37f144074b24e713f75ca553a1c20
Author: rockot <rockot@chromium.org>
Date: Fri Oct 21 22:47:50 2016

Fix some test expectations for IPC tests

Corrects an expectation in a test filter which could lead to infrequent
flakiness. Also adds a new expectation for good measure.

BUG= 657080 

Review-Url: https://chromiumcodereview.appspot.com/2437283005
Cr-Commit-Position: refs/heads/master@{#426920}

[modify] https://crrev.com/8a96cca441f37f144074b24e713f75ca553a1c20/ipc/ipc_channel_proxy_unittest.cc

Comment 9 by xyzzyz@chromium.org, Oct 21 2016

Status: Fixed (was: Assigned)
Cool! Thanks Ken! We're going to deploy the updated binary to our runner on Monday. I'll reopen this bug if we see any more flakes.

Sign in to add a comment