Issue metadata
Sign in to add a comment
|
org.chromium.chrome.browser.contextual_suggestions.ContextualSuggestionsTest#testMultiInstanceMode failing on KitKat on CQ and waterfall |
||||||||||||||||||||||
Issue descriptionThis test is failing frequently on the android-kitkat-arm-rel trybot, blocking the CQ: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/android-kitkat-arm-rel?limit=200 for example: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/android-kitkat-arm-rel/1086 See the flakiness dashboard: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=chrome_public_test_apk&tests=org.chromium.chrome.browser.contextual_suggestions.ContextualSuggestionsTest#testMultiInstanceMode and this waterfall bot, where it's failing frequently: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/KitKat%20Phone%20Tester%20%28dbg%29 It looks like this might have been suppressed by the sheriffs but I can't tell for sure from Sheriff-O-Matic. Can someone please triage this? It's blocking the CQ. Thanks.
,
May 31 2018
bauerb@ - can you see whether this is related to your change to migrate to use Use ListObserver instead of NodeParent to notify about changes? It's listed on the first KitKat Phone Tester flake identified on the flakiness dashboard: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/KitKat%20Phone%20Tester%20%28dbg%29/7348 Based on results details, the test flaked twice on that run.
,
May 31 2018
Ugh, and of course I can't reproduce this locally without a KitKat device :-/
,
May 31 2018
In the part of the test that's failing, we wait for a itemRangeInsertedCallback then assert the model has the correct number of items:
itemRangeInsertedCallback.waitForCallback(0);
assertEquals("Second model has incorrect number of items.",
(int) FakeContextualSuggestionsSource.TOTAL_ITEM_COUNT,
mModel2.getClusterList().getItemCount());
I suspect we're getting multiple itemRangeInsertedCallback calls and that to fix this we could just wait for more callbacks or change the callback to wait until there are 6 total items.
,
May 31 2018
Oh right, and in addition we access the cluster list on the instrumentation thread, so if we finish waiting after the first callback, we have a race condition between reading the number of items on the instrumentation thread and inserting additional items on the UI thread. Thanks for the investigation :)
,
May 31 2018
Do you have time to look into this before you're off for the day? If not, I can pick it up or at least disable the test for now to unblock the CQ.
,
May 31 2018
Well, I have a notion of how to fix it, but I'm not going to get around to doing it today. I guess we can disable the test for now?
,
May 31 2018
I have a CL out for review: https://chromium-review.googlesource.com/c/chromium/src/+/1081105
,
May 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7c3409cd24a81b560477e558664ea71e6cbcbdff commit 7c3409cd24a81b560477e558664ea71e6cbcbdff Author: Theresa <twellington@chromium.org> Date: Thu May 31 19:36:44 2018 Fix ContextualSuggestionsTest#testMultiInstanceMode onItemRangeInserted() can be called multiple times. Wait until the expected number of items have been inserted before notifying the callback helper. Also get the total model count on the UI thread to avoid a race between waiting for the callback on the instrumentation thread and updating the model state on the UI thread. BUG= 848089 Change-Id: Ie4ae5cfeef41176d7ea800842c5e106e3bfa8511 Reviewed-on: https://chromium-review.googlesource.com/1081105 Commit-Queue: Theresa <twellington@chromium.org> Reviewed-by: Becky Zhou <huayinz@chromium.org> Cr-Commit-Position: refs/heads/master@{#563351} [modify] https://crrev.com/7c3409cd24a81b560477e558664ea71e6cbcbdff/chrome/android/javatests/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsTest.java
,
Jun 1 2018
,
Jun 1 2018
,
Jun 4 2018
,
Jun 5 2018
The NextAction date has arrived: 2018-06-05 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by twelling...@chromium.org
, May 31 2018