Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"SearchEngineProviderP3ATest.DefaultSearchEngineP3A" failing CI intermittently #13057

Closed
kjozwiak opened this issue Dec 5, 2020 · 4 comments
Closed
Assignees
Labels
dev-concern features/P3A OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA/No release-notes/exclude

Comments

@kjozwiak
Copy link
Member

kjozwiak commented Dec 5, 2020

SearchEngineProviderP3ATest.DefaultSearchEngineP3A is intermittently failing CI. Example:

13:47:30  [ RUN      ] SearchEngineProviderP3ATest.DefaultSearchEngineP3A
13:47:30  ../../base/test/metrics/histogram_tester.cc:208: Failure
13:47:30  Expected equality of these values:
13:47:30    expected_count
13:47:30      Which is: 1
13:47:30    actual_count
13:47:30      Which is: 0
13:47:30  Histogram "Brave.Search.DefaultEngine.3" does not have the right number of samples (1) in the expected bucket (2). It has (0).
13:47:30  Stack trace:
13:47:30  #0 0x55bc039d9099 (/home/ubuntu/workspace/pr-brave-browser-maxk-l10n-1.18.x-linux/src/out/Release/brave_browser_tests+0x5f59098)
13:47:30  
13:47:30  ../../base/test/metrics/histogram_tester.cc:223: Failure
13:47:30  Expected equality of these values:
13:47:30    expected_count
13:47:30      Which is: 2
13:47:30    actual_count
13:47:30      Which is: 1
13:47:30  Histogram "Brave.Search.DefaultEngine.3" does not have the right total number of samples (2). It has (1).
13:47:30  Stack trace:
13:47:30  #0 0x55bc039d9099 (/home/ubuntu/workspace/pr-brave-browser-maxk-l10n-1.18.x-linux/src/out/Release/brave_browser_tests+0x5f59098)
13:47:30  
13:47:30  [410536:410536:1204/184729.894604:ERROR:brave_new_tab_message_handler.cc(192)] Ads service is not initialized!
13:47:30  [410536:410536:1204/184729.984196:WARNING:pref_notifier_impl.cc(61)] Init observer found at shutdown.
13:47:30  [  FAILED  ] SearchEngineProviderP3ATest.DefaultSearchEngineP3A, where TypeParam =  and GetParam() =  (391 ms)

CCing @bsclifton

@rillian
Copy link

rillian commented Sep 30, 2021

I've implemented a proposed fix and re-enable. If the test continues to be intermittent, please report here for followup!

@bsclifton bsclifton assigned rillian and unassigned bsclifton Sep 30, 2021
rillian added a commit to brave/brave-core that referenced this issue Oct 1, 2021
This was disabled in 27bf9e3 for intermittent failures.
Add a call to wait for TemplateURLServiceFactory to finish
loading, and re-enable to see if that stabilizes the browser
test.

See brave/brave-browser#13057 for tracking.

This duplicates VerifyTemplateURLServiceLoad() from the
search_engine_provider browser test, but it's a small piece
of code and having two file-local implementations is simple
enough.
@rillian
Copy link

rillian commented Oct 6, 2021

This was apparently working until the Chromium 95 merge.

rillian added a commit to brave/brave-core that referenced this issue Oct 6, 2021
Add delay at the beginning of the browser test to reduce
intermittent measurement failures.

See brave/brave-browser#13057 for tracking.
rillian added a commit to brave/brave-core that referenced this issue Oct 6, 2021
This test is failing intermittently again since the merge of
chromium 95. Disable until we can determine where the race is
happening.

See brave/brave-browser#13057 for tracking.
rillian added a commit to brave/brave-core that referenced this issue Oct 18, 2021
Like the other SearchEngineTracker browser test, this gives unstable
results in ci, tracked in brave/brave-browser#13057.

Disable the test for now, and remove the `MergeHistogramDeltasForTesting`
which we don't expect is necessary and made no difference in testing.
@rillian
Copy link

rillian commented Oct 19, 2021

This test is re-enabled by brave/brave-core#10300, with the addition of forcing a specific region at the beginning of the test. That seemed to give more consistent results. I'll leave this open for a bit in case it doesn't stick, but for now ci tests seem to be green.

rillian added a commit to brave/brave-core that referenced this issue Oct 20, 2021
Like the other SearchEngineTracker browser test, this gives unstable
results in ci, tracked in brave/brave-browser#13057.

Disable the test for now, and remove the `MergeHistogramDeltasForTesting`
which we don't expect is necessary and made no difference in testing.
rillian added a commit to brave/brave-core that referenced this issue Oct 20, 2021
Like the other SearchEngineTracker browser test, this gives unstable
results in ci, tracked in brave/brave-browser#13057.

Disable the test for now, and remove the `MergeHistogramDeltasForTesting`
which we don't expect is necessary and made no difference in testing.
@rillian
Copy link

rillian commented Oct 26, 2021

Seems to be stable now. Thanks @goodov!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev-concern features/P3A OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA/No release-notes/exclude
Projects
None yet
Development

No branches or pull requests

4 participants