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

Issue 629591 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

CronetHttpURLConnectionTest#testServerHangsUp racy

Project Member Reported by pauljensen@chromium.org, Jul 19 2016

Issue description

Version: ToT
OS: Android

What steps will reproduce the problem?
(1) ./components/cronet/tools/cr_cronet.py build-test
(2) ./components/cronet/tools/cr_cronet.py test -f org.chromium.net.urlconnection.CronetHttpURLConnectionTest#testServerHangsUp

What is the expected output?
C   15.593s Main  ********************************************************************************
C   15.593s Main  Summary
C   15.593s Main  ********************************************************************************
C   15.593s Main  [==========] 1 test ran.
C   15.593s Main  [  PASSED  ] 1 test.
C   15.593s Main  ********************************************************************************

What do you see instead?
C    9.477s Main  ********************************************************************************
C    9.477s Main  Detailed Logs
C    9.477s Main  ********************************************************************************
C    9.477s Main  [FAIL] org.chromium.net.urlconnection.CronetHttpURLConnectionTest#testServerHangsUp:
C    9.477s Main  java.lang.Throwable: CronetTestBase#runTest failed.
C    9.477s Main        at org.chromium.net.CronetTestBase.runTest(CronetTestBase.java:132)
C    9.478s Main        at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:198)
C    9.478s Main        at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:183)
C    9.478s Main        at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:560)
C    9.478s Main        at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1932)
C    9.478s Main  Caused by: junit.framework.AssertionFailedError
C    9.478s Main        at org.chromium.net.urlconnection.CronetHttpURLConnectionTest.testServerHangsUp(CronetHttpURLConnectionTest.java:871)
C    9.478s Main        at org.chromium.net.CronetTestBase.runTest(CronetTestBase.java:118)
C    9.478s Main        ... 9 more
C    9.478s Main  ********************************************************************************
C    9.478s Main  Summary
C    9.478s Main  ********************************************************************************
C    9.478s Main  [==========] 1 test ran.
C    9.478s Main  [  PASSED  ] 0 tests.
C    9.478s Main  [  FAILED  ] 1 test, listed below:
C    9.479s Main  [  FAILED  ] org.chromium.net.urlconnection.CronetHttpURLConnectionTest#testServerHangsUp
C    9.479s Main  
C    9.479s Main  1 FAILED TEST
C    9.479s Main  ********************************************************************************

I suspected this test might be racy by looking at the code.
I verified my suspicions by increasing by 10x TestUtil.REPEAT_COUNT.
The test failed several times with the normal REPEAT_COUNT and passed several times with the 10x REPEAT_COUNT.  I think the test needs to wait for the native server to shutdown.

Andrei did not see this failure on his Nexus 9 running Android N.  I saw this failure on my Nexus 5X running Android N.  I suspect I saw it because the 5X has a faster processor.
 
EmbeddedTestServer's destructor will wait for shutdown to complete synchronously (https://cs.chromium.org/chromium/src/net/test/embedded_test_server/embedded_test_server.cc?rcl=1468927100&l=64). The system implementation fails according to the stack (Cronet's implementation passes the test). This is probably because the system implementation on 5X buffers more data than our synchronous wrapper. I can change the test to only run Cronet. 

Status: Started (was: Assigned)
Proposed: https://codereview.chromium.org/2164863002/
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 21 2016

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 21 2016

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

commit deb7784ea28328d178141c9eaad2e79dd6583a24
Author: xunjieli <xunjieli@chromium.org>
Date: Thu Jul 21 14:25:55 2016

Revert of Fix CronetHttpURLConnectionTest#testServerHangsUp flake (patchset #3 id:60001 of https://codereview.chromium.org/2164863002/ )

Reason for revert:
Broke test on the bot.

Original issue's description:
> Fix CronetHttpURLConnectionTest#testServerHangsUp flake
>
> This CL makes the server to keep sending data, so when we
> shut down the server, we are sure that it is still sending
> (no eof has been sent).
>
> BUG= 629591 
>
> Committed: https://crrev.com/056cabaa7714222bd67ecea221b7b3d4103752ee
> Cr-Commit-Position: refs/heads/master@{#406833}

TBR=pauljensen@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 629591 

Review-Url: https://codereview.chromium.org/2172633002
Cr-Commit-Position: refs/heads/master@{#406844}

[modify] https://crrev.com/deb7784ea28328d178141c9eaad2e79dd6583a24/components/cronet/android/test/javatests/src/org/chromium/net/urlconnection/CronetHttpURLConnectionTest.java
[modify] https://crrev.com/deb7784ea28328d178141c9eaad2e79dd6583a24/components/cronet/android/test/native_test_server.cc
[modify] https://crrev.com/deb7784ea28328d178141c9eaad2e79dd6583a24/components/cronet/android/test/src/org/chromium/net/NativeTestServer.java

Status: Fixed (was: Started)
hmm.. The revert of the revert isn't attached to this bug. I am not sure why. I think this is fixed.

Sign in to add a comment