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

Issue 751184 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Cronet timing tests sometimes fail on fast devices/emulators

Project Member Reported by pauljensen@chromium.org, Aug 1 2017

Issue description

An example failure of RequestFinishedInfoTest.testRequestFinishedListenerFailedRequest looks like:

Caused by: 
	at junit.framework.Assert.fail(Assert.java:48)
	at junit.framework.Assert.assertTrue(Assert.java:20)
	at junit.framework.Assert.assertTrue(Assert.java:27)
	at org.chromium.net.RequestFinishedInfoTest.testRequestFinishedListenerFailedRequest(RequestFinishedInfoTest.java:247)

Fix: https://chromium-review.googlesource.com/c/596073
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 1 2017

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

commit 03efa26cafe6f71cdd37d04fe40d264761bb9d8d
Author: Paul Jensen <pauljensen@chromium.org>
Date: Tue Aug 01 19:13:50 2017

[Cronet] Adjust some tests to run on fast devices/emulators.

Some devices/emulators run quickly so requests take less than 1ms,
which causes some test failures.  Fix them using MetricsTestUtil.assertAfter()
and also clean up copies of that code.

Bug:  751184 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Icdce80479a887276b8921b4168a96c65b12d0143
Reviewed-on: https://chromium-review.googlesource.com/596073
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491076}
[modify] https://crrev.com/03efa26cafe6f71cdd37d04fe40d264761bb9d8d/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java
[modify] https://crrev.com/03efa26cafe6f71cdd37d04fe40d264761bb9d8d/components/cronet/android/test/javatests/src/org/chromium/net/MetricsTestUtil.java
[modify] https://crrev.com/03efa26cafe6f71cdd37d04fe40d264761bb9d8d/components/cronet/android/test/javatests/src/org/chromium/net/RequestFinishedInfoTest.java

Labels: Merge-Request-61
Project Member

Comment 3 by sheriffbot@chromium.org, Aug 3 2017

Labels: -Merge-Request-61 Hotlist-Merge-Approved Merge-Approved-61
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 3 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f897be169d1458d00076e623cb4cd6b6a7d8e4f9

commit f897be169d1458d00076e623cb4cd6b6a7d8e4f9
Author: Paul Jensen <pauljensen@chromium.org>
Date: Thu Aug 03 11:37:09 2017

[Cronet] Adjust some tests to run on fast devices/emulators.

Some devices/emulators run quickly so requests take less than 1ms,
which causes some test failures.  Fix them using MetricsTestUtil.assertAfter()
and also clean up copies of that code.

TBR=pauljensen@chromium.org

(cherry picked from commit 03efa26cafe6f71cdd37d04fe40d264761bb9d8d)

Bug:  751184 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Icdce80479a887276b8921b4168a96c65b12d0143
Reviewed-on: https://chromium-review.googlesource.com/596073
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#491076}
Reviewed-on: https://chromium-review.googlesource.com/600107
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#269}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/f897be169d1458d00076e623cb4cd6b6a7d8e4f9/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java
[modify] https://crrev.com/f897be169d1458d00076e623cb4cd6b6a7d8e4f9/components/cronet/android/test/javatests/src/org/chromium/net/MetricsTestUtil.java
[modify] https://crrev.com/f897be169d1458d00076e623cb4cd6b6a7d8e4f9/components/cronet/android/test/javatests/src/org/chromium/net/RequestFinishedInfoTest.java

Status: Fixed (was: Started)
FYI M61 merge first appeared in 61.0.3163.32 which is newer than latest dev channel release.
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 2 2017

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

commit fe4368e39afae6f3a454c23445b2b400c3b859ee
Author: Paul Jensen <pauljensen@chromium.org>
Date: Mon Oct 02 14:47:12 2017

[Cronet] Adjust some tests to run on fast devices/emulators.

Some devices/emulators run quickly so requests take less than 1ms,
which causes some test failures.

Bug:  751184 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Ib41da1c337bf7dc08316cf7d4f083f969e8b3280
Reviewed-on: https://chromium-review.googlesource.com/694904
Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505607}
[modify] https://crrev.com/fe4368e39afae6f3a454c23445b2b400c3b859ee/components/cronet/android/test/javatests/src/org/chromium/net/MetricsTestUtil.java

I'm going to merge back the fix from Comment 7 to M62.  This change does not affect Chrome, only Cronet, so I'm not seeking merge approval as per discussions with amineer@.
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 2 2017

Labels: merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c3bf29037eb6910fb11e5221a09b66d1da415a6

commit 8c3bf29037eb6910fb11e5221a09b66d1da415a6
Author: Paul Jensen <pauljensen@chromium.org>
Date: Mon Oct 02 16:42:53 2017

[Merge M62] [Cronet] Adjust some tests to run on fast devices/emulators.

Some devices/emulators run quickly so requests take less than 1ms,
which causes some test failures.

TBR=pauljensen@chromium.org

(cherry picked from commit fe4368e39afae6f3a454c23445b2b400c3b859ee)

Bug:  751184 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Ib41da1c337bf7dc08316cf7d4f083f969e8b3280
Reviewed-on: https://chromium-review.googlesource.com/694904
Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#505607}
Reviewed-on: https://chromium-review.googlesource.com/695226
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#536}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/8c3bf29037eb6910fb11e5221a09b66d1da415a6/components/cronet/android/test/javatests/src/org/chromium/net/MetricsTestUtil.java

Sign in to add a comment