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

Issue 781178 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

PaymentRequestTestRule setup occasionally failing on bots

Project Member Reported by peconn@chromium.org, Nov 3 2017

Issue description

It has been failing occasionally, see on Kitkat Phone Test (dbg) and Lollipop Tablet Tester.

Example failures:
- https://uberchromegw.corp.google.com/i/chromium.android/builders/KitKat%20Phone%20Tester%20%28dbg%29/builds/828
  - Failed on PaymentRequestJourneyLoggerTest#testNumberOfSelectionEdits_ShippingAddress_Completed
- https://uberchromegw.corp.google.com/i/chromium.android/builders/Lollipop%20Tablet%20Tester/builds/10576
  - Failed on PaymentRequestPaymentAppAndCardsTest#testAddPaymentMethodAndCancelEditorShouldKeepExistingCardSelected

In both cases the stack trace was:

java.lang.AssertionError: Tab never selected/initialized.
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.chromium.content.browser.test.util.CriteriaHelper.pollInstrumentationThread(CriteriaHelper.java:93)
	at org.chromium.content.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:172)
	at org.chromium.content.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:194)
	at org.chromium.chrome.test.ChromeActivityTestRule.startMainActivityFromIntent(ChromeActivityTestRule.java:387)
	at org.chromium.chrome.test.ChromeActivityTestRule.startMainActivityWithURL(ChromeActivityTestRule.java:352)
	at org.chromium.chrome.browser.payments.PaymentRequestTestCommon.startMainActivity(PaymentRequestTestCommon.java:139)
	at org.chromium.chrome.browser.payments.PaymentRequestTestRule.startMainActivity(PaymentRequestTestRule.java:131)
	at org.chromium.chrome.browser.payments.PaymentRequestTestRule$1.evaluate(PaymentRequestTestRule.java:580)


 
Owner: tedc...@chromium.org
Ted, can you help triage please? (This might be a duplicate.)
Cc: yolandyan@chromium.org wychen@chromium.org
+yolandyan...seems like we're seeing a random collection of failures during initial startup of tests.  Should we just increase the timeout during the setup for each point?  I don't "think" tests should be slowing these down as the bulk of their work is afterwards.  This could indicate a general slowness in other parts of the system, or regressions in startup, but I'm not sure we want random tests failing because of that but should have some other system for monitoring it.

Like this one as well that wychen@ did some investigation on:
https://bugs.chromium.org/p/chromium/issues/detail?id=779846
There could be a gradual regression in startup, but test flakiness is a noisy signal. Even if we have increasing flakiness due to slowness, that could also be due to wear of testing devices. Agree we should have some system to monitor start up performance.

As for the tests, we could add this before waiting for the tab, but in the end this might just be equivalent to doubling the timeout.

        CriteriaHelper.pollUiThread(() -> ChromeBrowserInitializer.getInstance(
                getActivity()).hasNativeInitializationCompleted(),
                "Native initialization never finished");
Adding that SGTM.  That boolean is global to all activities though, so wouldn't be a guarantee that the activity has fully been initialized (we'd need to expose that in our base activity somewhere).

Regardless, still seems like a fine thing to add as initializing native is still where I expect 90% of the time to go.
Although, we might want to increase the timeout for that call as well since I would actually expect them to be roughly equivalent.  If that is the thing that is timing out, we'd just be changing the failure cause to something that is clearer (which is good), but maybe we need the timeout increase too.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 3 2017

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

commit fc73bed7f9534c2c157f86f547f5b87b264298d3
Author: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Date: Fri Nov 03 18:36:28 2017

Better failure message in startMainActivityFromIntent

"Tab never selected/initialized" was shown in flaky tests, while
native initialization should have occupied the majority of the time,
so it is clearer to show that.

Also doubled the timeout to lower the probability of failing.

Bug:  781178 
Change-Id: I1417e8a0b2d5d5f0964675c16a97ce6381dfb8c2
Reviewed-on: https://chromium-review.googlesource.com/753980
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513850}
[modify] https://crrev.com/fc73bed7f9534c2c157f86f547f5b87b264298d3/chrome/test/android/javatests/src/org/chromium/chrome/test/ChromeActivityTestRule.java

Owner: wychen@chromium.org
Status: Fixed (was: Untriaged)
Going to mark this fixed with wychen@'s CL.  Hopefully that reduces the noise from slow startup.

Sign in to add a comment