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

Issue 904108 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Flaky-Test: ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange



Sign in to add a comment

ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange is flaky

Project Member Reported by Findit, Nov 10

Issue description


Flaky test: ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange
Sample failed build due to flakiness: https://ci.chromium.org/buildbot/chromium.mac/Mac10.12%20Tests/16631
Test output log: https://chromium-swarm.appspot.com/task?id=411647bd264b2a10
Culprit (70.0% confidence): r607022
Analysis: https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVy_gELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCLHAWNocm9taXVtLm1hYy9NYWMxMC4xMiBUZXN0cy8xNjYzMS9uZXR3b3JrX3NlcnZpY2VfYnJvd3Nlcl90ZXN0cyBvbiBJbnRlbCBHUFUgb24gTWFjIG9uIE1hYy0xMC4xMi42L1EyaHliMjFsUW5KdmQzTmxjazFoYVc1Q2NtOTNjMlZ5VkdWemRDNVdZWEpwWVhScGIyNXpVMlZ5ZG1salpWTjBZWEowYzFKbGNYVmxjM1JQYms1bGRIZHZjbXREYUdGdVoyVT0MCxITTWFzdGVyRmxha2VBbmFseXNpcxgBDA

Please revert the culprit, or disable the test and find the appropriate owner.

If the culprit above is wrong, please file a bug using this link:
https://bugs.chromium.org/p/chromium/issues/entry?status=Unconfirmed&labels=Pri-1,Test-Findit-Wrong&components=Tools%3ETest%3EFindit%3EFlakiness&summary=%5BFindit%5D%20Flake%20Analyzer%20-%20Wrong%20result%20for%20ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange&comment=Link%20to%20Analysis%3A%20https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVy_gELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCLHAWNocm9taXVtLm1hYy9NYWMxMC4xMiBUZXN0cy8xNjYzMS9uZXR3b3JrX3NlcnZpY2VfYnJvd3Nlcl90ZXN0cyBvbiBJbnRlbCBHUFUgb24gTWFjIG9uIE1hYy0xMC4xMi42L1EyaHliMjFsUW5KdmQzTmxjazFoYVc1Q2NtOTNjMlZ5VkdWemRDNVdZWEpwWVhScGIyNXpVMlZ5ZG1salpWTjBZWEowYzFKbGNYVmxjM1JQYms1bGRIZHZjbXREYUdGdVoyVT0MCxITTWFzdGVyRmxha2VBbmFseXNpcxgBDA

Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N).
 
Components: Internals>Metrics>Variations
Components: Tests>Flaky
Labels: -Sheriff-Chromium
Owner: carlosil@chromium.org
Status: Assigned (was: Untriaged)
Assigning to author of likely culprit CL.
I reverted the CL for now (crrev.com/c/1334421), the starting number of variations seed requests on the failing run is 0, and the ending one is 2, so it seems the retry logic is triggering at some point. (So the bug that the test checks for didn't regress).
It seems the browser test goes through the usual path for retrieving the seed so if the first attempt fails it will always cause a retry (but it was not being surfaced prior to this CL since the tests use an HTTP url, so the logic didn't trigger), a simple solution would be to check for one or two requests happening, since the test would still validate a request happened on network changes, but won't fail if a retry happens.

Another alternative would be to check both the URL, and whether or not the previous attempt was a retry when retrying, but I'm not too keen about this since it feels like tweaking Chrome's behavior to match a quirk in the test.

Variations folks, do you have any opinions on this one? Otherwise I'll write a CL adding the check for one or two requests, and revert the revert.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 27

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

commit 871a1ef511b48acdad53f7d8e8fafd6362fc52d2
Author: Carlos IL <carlosil@chromium.org>
Date: Tue Nov 27 17:05:29 2018

Reland "Variations service now checks explicitely if previous attempt was HTTP."

This reverts commit 8d41f8c9c5f71f94133e1e557701521b5e464c63.

Reason for revert: This CL reverts the revert, and fixes the flakiness in the test by allowing one or two requests in the test.

Bug:  904108 , 902727

Original change's description:
> Revert "Variations service now checks explicitely if previous attempt was HTTP."
>
> This reverts commit 91d246885ead6155c131e027d99be1aa79b9ae3c.
>
> Reason for revert: This CL made ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange flaky ( crbug.com/904108 ) reverting while investigating why.
>
> Original change's description:
> > Variations service now checks explicitely if previous attempt was HTTP.
> >
> > Variations service now sets a flag if the last request was an HTTP
> > retry, and uses it to decide whether to retry again, instead of
> > relying on the scheme of the final URL. Also, retries are now disabled
> > if the fallback url is HTTPS.
> >
> > Bug: 902727
> > Change-Id: Ibd6df4a22bc6302b231aff0ae32f8e1c8a1f277c
> > Reviewed-on: https://chromium-review.googlesource.com/c/1325035
> > Commit-Queue: Carlos IL <carlosil@chromium.org>
> > Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#607022}
>
> TBR=asvitkine@chromium.org,carlosil@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 902727
> Change-Id: I67036ea20097f889e71de5179a2e4510a201388d
> Reviewed-on: https://chromium-review.googlesource.com/c/1334421
> Reviewed-by: Carlos IL <carlosil@chromium.org>
> Commit-Queue: Carlos IL <carlosil@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#607882}

Change-Id: I5618dd7c48849ddf1fc8cd8f613ba7d83ebc66c0
Reviewed-on: https://chromium-review.googlesource.com/c/1335666
Commit-Queue: Carlos IL <carlosil@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611152}
[modify] https://crrev.com/871a1ef511b48acdad53f7d8e8fafd6362fc52d2/chrome/browser/chrome_browser_main_browsertest.cc
[modify] https://crrev.com/871a1ef511b48acdad53f7d8e8fafd6362fc52d2/components/variations/service/variations_service.cc
[modify] https://crrev.com/871a1ef511b48acdad53f7d8e8fafd6362fc52d2/components/variations/service/variations_service.h
[modify] https://crrev.com/871a1ef511b48acdad53f7d8e8fafd6362fc52d2/components/variations/service/variations_service_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment