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

Issue 749148 link

Starred by 1 user
Status: Fixed
Owner: ----
Closed: Aug 2017
Cc:
█ mge...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

QuicTest.testMetricsWithQuic flake when run in another environment

Project Member Reported by pauljensen@chromium.org, Jul 26 2017 
Seems to fail a significant percentage of the time (maybe as high as 50%) when run in another environment.  I can't get it to fail when run by Chromium test runners, I've even tried running on the same emulator.

Always fails like this:
	at junit.framework.Assert.assertTrue(Assert.java:27)
	at org.chromium.net.MetricsTestUtil.checkTimingMetrics(MetricsTestUtil.java:112)
	at org.chromium.net.MetricsTestUtil.checkRequestFinishedInfo(MetricsTestUtil.java:171)
	at org.chromium.net.QuicTest.testMetricsWithQuic(QuicTest.java:273)

Which I think means Date() returned a time after UrlRequest.Builder.build().start()'s metrics.getResponseStart().

It's interesting that both the checks that metrics.getRequestStart() and metrics.getSendingStart() were after the start time passed.

Comment 1 by pauljensen@chromium.org, Jul 29 2017

Status: Started (was: Untriaged)
I added some logging and reran until it failed, but the logging didn't show much:
07-28 18:33:38.865  7067  7105 E PJPJ    : startTime = Fri Jul 28 18:33:38 PDT 2017
07-28 18:33:38.865  7067  7105 E PJPJ    : ResponseStart = Fri Jul 28 18:33:38 PDT 2017
07-28 18:33:38.866  7067  7105 E PJPJ    : startTime = Fri Jul 28 18:33:38 PDT 2017
07-28 18:33:38.866  7067  7105 E PJPJ    : ResponseStart = Fri Jul 28 18:33:38 PDT 2017
I changed to dump the Date's with .getTime() and reran until it failed:
07-28 19:16:02.314  7161  7199 E PJPJ    : startTime = 1501294562272
07-28 19:16:02.314  7161  7199 E PJPJ    : ResponseStart = 1501294562311
07-28 19:16:02.316  7161  7199 E PJPJ    : startTime = 1501294562315
07-28 19:16:02.316  7161  7199 E PJPJ    : ResponseStart = 1501294562315

I think the problem is on fast emulators/devices using .after() to compare is incorrect as it fails when the Dates are the same.

Fix out for review: https://chromium-review.googlesource.com/c/592751
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 31 2017 

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

commit 907c90c963d8de4e4b3a3e37350cf176b9dfc0aa
Author: Paul Jensen <pauljensen@chromium.org>
Date: Mon Jul 31 16:13:38 2017

[Cronet] Fix some test asserts for time comparison

Date.after() and .before() don't allow for equal times, which
can happen on fast emulators/devices.

Bug:  749148 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: I5e81e17435e5827855e44adbc7eb91052f17aa66
Reviewed-on: https://chromium-review.googlesource.com/592751
Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490772}
[modify] https://crrev.com/907c90c963d8de4e4b3a3e37350cf176b9dfc0aa/components/cronet/android/test/javatests/src/org/chromium/net/MetricsTestUtil.java

Comment 3 by pauljensen@chromium.org, Jul 31 2017

Labels: Merge-Request-61
Project Member

Comment 4 by sheriffbot@chromium.org, Aug 1 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 5 by bugdroid1@chromium.org, Aug 1 2017

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

commit 1bd72f521deb119f46f1a1f137354078d275ba62
Author: Paul Jensen <pauljensen@chromium.org>
Date: Tue Aug 01 18:22:31 2017

[Cronet] Fix some test asserts for time comparison

Date.after() and .before() don't allow for equal times, which
can happen on fast emulators/devices.

TBR=pauljensen@chromium.org

(cherry picked from commit 907c90c963d8de4e4b3a3e37350cf176b9dfc0aa)

Bug:  749148 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: I5e81e17435e5827855e44adbc7eb91052f17aa66
Reviewed-on: https://chromium-review.googlesource.com/592751
Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#490772}
Reviewed-on: https://chromium-review.googlesource.com/596075
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#213}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/1bd72f521deb119f46f1a1f137354078d275ba62/components/cronet/android/test/javatests/src/org/chromium/net/MetricsTestUtil.java

Comment 6 by pauljensen@chromium.org, Aug 1 2017

Status: Fixed (was: Started)
►

Sign in to add a comment