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

Issue 674129 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

VSyncMonitorTest#testVSyncPeriod flaky on android

Project Member Reported by torne@chromium.org, Dec 14 2016

Issue description

content_shell_test_apk test org.chromium.content.browser.VSyncMonitorTest#testVSyncPeriod is flaking regularly on android:

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=content_shell_test_apk&tests=org.chromium.content.browser.VSyncMonitorTest%23testVSyncPeriod

The failures are all of the form:

C  358.417s Main  [FAIL] org.chromium.content.browser.VSyncMonitorTest#testVSyncPeriod:
C  358.417s Main  junit.framework.AssertionFailedError: Measured median frame period 13412 differs by more than 10% from the reported frame period 16666 for requested frames
C  358.417s Main  	at org.chromium.content.browser.VSyncMonitorTest.testVSyncPeriod(VSyncMonitorTest.java:113)
C  358.417s Main  	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
C  358.417s Main  	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
C  358.417s Main  	at org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:129)
C  358.417s Main  	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191)
C  358.417s Main  	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176)
C  358.417s Main  	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555)
C  358.417s Main  	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1853)

This suggests the test is too strict or timing is too unreliable on these devices? Looking at the test it's not really clear what it's actually testing - it looks like it's effectively just testing the system vsync mechanism itself and not our code, in which case the test failing doesn't seem to indicate a problem we can actually fix.

Going to disable it for now, assigning/ccing people who recently touched/reviewed the code in question.
 

Comment 1 by boliu@chromium.org, Dec 14 2016

I'm ok with just removing this test. This is testing android behavior, not chromium code behavior. And assuming android behavior is consistent is often wrong...
That's also fine by me, assuming the CTS has a test like this in it somewhere :) We've had at least one device in the past that was reporting bogus vsync values and as a result completely hosed our rendering.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 14 2016

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

commit 97c7de5d2920e9eb6b99bee8a055657f9ee896d2
Author: torne <torne@chromium.org>
Date: Wed Dec 14 18:58:55 2016

Disable flaky VsyncMonitorTest.

Test is flaking frequently because the period of the vsync is outside
the 10% tolerance expected.

BUG= 674129 

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

[modify] https://crrev.com/97c7de5d2920e9eb6b99bee8a055657f9ee896d2/content/public/android/javatests/src/org/chromium/content/browser/VSyncMonitorTest.java

Status: Assigned (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, May 10 2017

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

commit f68f14ece74e5d5654fb85ba6d03b66760c7dca1
Author: Adrienne Walker <enne@chromium.org>
Date: Wed May 10 05:12:06 2017

Remove disabled VSyncMonitor test

This is testing Android behavior and shouldn't be a Chromium test.

Bug:  674129 
Change-Id: Id7932848b331cf48e57e788aec88dcc12a609e28
Reviewed-on: https://chromium-review.googlesource.com/498863
Commit-Queue: enne <enne@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#470473}
[modify] https://crrev.com/f68f14ece74e5d5654fb85ba6d03b66760c7dca1/content/public/android/BUILD.gn
[delete] https://crrev.com/28e267b701804d24313d9a8af3f8714768825c42/content/public/android/javatests/src/org/chromium/content/browser/VSyncMonitorTest.java

Comment 6 by enne@chromium.org, May 10 2017

Status: Fixed (was: Assigned)

Sign in to add a comment