Better fix for flaky LitePagesTimeout test
Figured out the problem and the best way to address it. Comments
document my reasoning.
Bug: 874150
Change-Id: Iba4b95fd615137d08836c98ec89bcb36db39327b
Reviewed-on: https://chromium-review.googlesource.com/c/1348549
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610488}diff --git a/chrome/browser/previews/previews_lite_page_browsertest.cc b/chrome/browser/previews/previews_lite_page_browsertest.cc
index 5d23bc8d..c01042c4 100644
--- a/chrome/browser/previews/previews_lite_page_browsertest.cc
+++ b/chrome/browser/previews/previews_lite_page_browsertest.cc
@@ -1077,8 +1077,16 @@
max_penalty = bucket.min;
}
}
- // |kTimeoutMs| is flaky, so use something slightly less.
- EXPECT_GE(max_penalty, kTimeoutMs - 50);
+ // Expecting |max_penalty| > |kTimeoutMs| is flaky in release builds because
+ // of histogram bucketing. Since HistogramTester::Bucket doesn't provide a
+ // bucket max, if |max_penalty| < |kTimeoutMs|, check that a sample exists
+ // in the |kTimeoutMs| bucket.
+ if (max_penalty <= kTimeoutMs) {
+ EXPECT_GE(histogram_tester.GetBucketCount(
+ "Previews.ServerLitePage.ReportedNavigationRestartPenalty",
+ kTimeoutMs),
+ 1);
+ } // else, test passes
}
{
