Android webview cts "testZoom" is flaky |
||
Issue descriptionSeems like a number rounding issue. @@@STEP_LOG_LINE@stdout@04-28 00:11:50 I/03848d89f0e93fd0: android.webkit.cts.WebViewTest#testZoom FAIL @@@ @@@STEP_LOG_LINE@stdout@junit.framework.AssertionFailedError: expected:<14.999998> but was:<15.0>@@@ @@@STEP_LOG_LINE@stdout@at junit.framework.Assert.fail(Assert.java:50)@@@ @@@STEP_LOG_LINE@stdout@at junit.framework.Assert.failNotEquals(Assert.java:287)@@@ @@@STEP_LOG_LINE@stdout@at junit.framework.Assert.assertEquals(Assert.java:67)@@@ @@@STEP_LOG_LINE@stdout@at junit.framework.Assert.assertEquals(Assert.java:74)@@@ @@@STEP_LOG_LINE@stdout@at android.webkit.cts.WebViewTest.testZoom(WebViewTest.java:333)@@@ @@@STEP_LOG_LINE@stdout@at java.lang.reflect.Method.invoke(Native Method)@@@ @@@STEP_LOG_LINE@stdout@at java.lang.reflect.Method.invoke(Method.java:372)@@@ @@@STEP_LOG_LINE@stdout@at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)@@@ @@@STEP_LOG_LINE@stdout@at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)@@@ @@@STEP_LOG_LINE@stdout@at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192)@@@ @@@STEP_LOG_LINE@stdout@at junit.framework.TestCase.runBare(TestCase.java:134)@@@ @@@STEP_LOG_LINE@stdout@at junit.framework.TestResult$1.protect(TestResult.java:115)@@@ @@@STEP_LOG_LINE@stdout@at junit.framework.TestResult.runProtected(TestResult.java:133)@@@ @@@STEP_LOG_LINE@stdout@at android.support.test.internal.runner.junit3.DelegatingTestResult.runProtected(DelegatingTestResult.java:90)@@@ @@@STEP_LOG_LINE@stdout@at junit.framework.TestResult.run(TestResult.java:118)@@@ @@@STEP_LOG_LINE@stdout@at android.support.test.internal.runner.junit3.AndroidTestResult.run(AndroidTestResult.java:52)@@@ @@@STEP_LOG_LINE@stdout@at junit.framework.TestCase.run(TestCase.java:124)@@@ @@@STEP_LOG_LINE@stdout@at android.support.test.internal.runner.junit3.NonLeakyTestSuite$NonLeakyTest.run(NonLeakyTestSuite.java:63)@@@ @@@STEP_LOG_LINE@stdout@at junit.framework.TestSuite.runTest(TestSuite.java:243)@@@ @@@STEP_LOG_LINE@stdout@at junit.framework.TestSuite.run(TestSuite.java:238)@@@ @@@STEP_LOG_LINE@stdout@at android.support.test.internal.runner.junit3.DelegatingTestSuite.run(DelegatingTestSuite.java:103)@@@ @@@STEP_LOG_LINE@stdout@at android.support.test.internal.runner.junit3.AndroidTestSuite.run(AndroidTestSuite.java:52)@@@ @@@STEP_LOG_LINE@stdout@at android.support.test.internal.runner.junit3.JUnit38ClassRunner.run(JUnit38ClassRunner.java:90)@@@ @@@STEP_LOG_LINE@stdout@at org.junit.runners.Suite.runChild(Suite.java:128)@@@ @@@STEP_LOG_LINE@stdout@at org.junit.runners.Suite.runChild(Suite.java:24)@@@ @@@STEP_LOG_LINE@stdout@at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)@@@ @@@STEP_LOG_LINE@stdout@at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)@@@ @@@STEP_LOG_LINE@stdout@at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)@@@ @@@STEP_LOG_LINE@stdout@at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)@@@ @@@STEP_LOG_LINE@stdout@at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)@@@ @@@STEP_LOG_LINE@stdout@at org.junit.runners.ParentRunner.run(ParentRunner.java:300)@@@ @@@STEP_LOG_LINE@stdout@at org.junit.runner.JUnitCore.run(JUnitCore.java:157)@@@ @@@STEP_LOG_LINE@stdout@at org.junit.runner.JUnitCore.run(JUnitCore.java:136)@@@ @@@STEP_LOG_LINE@stdout@at android.support.test.runner.AndroidJUnitRunner.onStart(AndroidJUnitRunner.java:245)@@@ @@@STEP_LOG_LINE@stdout@at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1853)@@@
,
Apr 29 2016
I'm also surprised that it didn't fail before. But from the user's perspective, 14.998 zoom factor is no different from 15.0.
,
Apr 29 2016
Interesting... so we do add a PAGE_SCALE_EPSILON in our most updated CTS in N, but we don't have that in L-MR1 cts, which is what the cts bot is using.
,
Apr 29 2016
We need to pull this change from marshmallow to L-MR1 cts in AOSP. commit b14371b785484c05e76c630c6013d52517c00b2b Author: Marcin Kosiba <mkosiba@google.com> Date: Thu Sep 11 15:56:01 2014 +0100 Use an epsilon value when comparing page scale values in WebView CTS test. BUG: 15681329 Change-Id: I9e0d30eece21b87a40abb343ba1cd0294dfae36f
,
May 2 2016
The epsilon in marcin's patch is smaller than the 0.002 difference here. maybe there's something else going on. that's a lot of floating point error to accumulate over the course of the test.
,
May 2 2016
// Used to avoid enabling zooming in / out if resulting zooming will
// produce little visible difference.
private static final float ZOOM_CONTROLS_EPSILON = 0.007f;
I guess this is probably the culprit. Maybe the best plan is to snap to max zoom as soon as we are within ZOOM_CONTROLS_EPSILON of it.
,
May 2 2016
Sorry I was lazy in #2. The difference is 0.000002, which is smaller than 0.0001.
,
May 3 2016
Ok. Still, it seems like snapping the zoom value slightly, given the zoom epsilon, would both be good for WebView, and make the test not flaky. It might also be sensible if the test didn't zoom by values like 0.8, because that's not exactly representable in base 2.
,
May 3 2016
> Ok. Still, it seems like snapping the zoom value slightly, given the zoom epsilon, would both be good for WebView, and make the test not flaky. Will need to rewrite zoom to *not* be based on pinch zoom then..
,
May 3 2016
> Ok. Still, it seems like snapping the zoom value slightly, given the zoom epsilon, would both be good for WebView, and make the test not flaky. Wait actually. That doesn't sound like a very good idea. What we really need is a zoomTo method that directly takes a zoom level. We should not add more corner cases to these zoom APIs that we have to explain in the docs.
,
May 3 2016
Does SyncCompositorMsg_ZoomBy result in the new zoom being set immediately to the new value (once the frame is propagated back), or does it animate to reach the new value? Because it seems like the only way the test can fail, with or without the epsilon comparison is if we receive more than one onScaleChanged per zoomBy call. The bounds checking on AwContents.zoomIn and AwContents.zoomOut is also not considered in AwContents.zoomBy, which seems inconsisent.
,
May 3 2016
SyncCompositorMsg_ZoomBy does a fake pinch zoom on the renderer side. The check on zoomin/zoomout wouldn't be needed if it didn't have to return a bool :/
,
May 4 2016
I don't see any benefit to the user or the app webview developer to make a version of zoomTo that is extremely accurate. (We haven't received any such requests from app devs?) This is just a test having unreasonable standards for zooming accuracy.
,
Jun 13 2016
Hasn't flaked in awhile.. wontfix for now? |
||
►
Sign in to add a comment |
||
Comment 1 by gsennton@chromium.org
, Apr 29 2016