1613549 - Intermittent [tier2] browser/components/urlbar/tests/browser/browser_search_tips.js | This test exceeded the timeout threshold. It should be rewritten or split up. If that's not possible, use requestLongerTimeout(N), but only as a last resort. -

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Treeherder Bug Filer]

Filed by: dvarga [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=287655807&repo=mozilla-central
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/B4Ws0baYQIieQWq6n5sBMw/runs/0/artifacts/public/logs/live_backing.log

---

[task 2020-02-05T23:29:24.761Z] 23:29:24     INFO - TEST-PASS | browser/components/urlbar/tests/browser/browser_search_tips.js | {"_name":"Google","_shortName":"google-b-1-d","_loadPath":"[other]addEngineWithDetails:[e-mail]","description":"Google Search","__searchForm":"https://www.google.com/search?client=firefox-b-1-d&q={searchTerms}","_iconURL":"moz-extension://37b45f1f-51a7-4050-b13a-da279933d1c6/favicon.ico","_iconMapObj":{"{}":"moz-extension://37b45f1f-51a7-4050-b13a-da279933d1c6/favicon.ico"},"_metaData":{"alias":null,"order":1},"_urls":[{"params":[{"condition":"pref","mozparam":true,"name":"channel","pref":"google_channel_us"},{"name":"client","value":"firefox-b-1-d"},{"name":"q","value":"{searchTerms}"}],"rels":[],"resultDomain":"www.google.com","template":"https://www.google.com/search"},{"params":[],"rels":[],"resultDomain":"www.google.com","template":"https://www.google.com/complete/search?client=firefox&q={searchTerms}","type":"application/x-suggestions+json"}],"_isBuiltin":true,"queryCharset":"UTF-8","extensionID":"[e-mail]","extensionLocale":"b-1-d"} == true - 
[task 2020-02-05T23:29:24.762Z] 23:29:24     INFO - Buffered messages finished
[task 2020-02-05T23:29:24.762Z] 23:29:24     INFO - TEST-UNEXPECTED-FAIL | browser/components/urlbar/tests/browser/browser_search_tips.js | This test exceeded the timeout threshold. It should be rewritten or split up. If that's not possible, use requestLongerTimeout(N), but only as a last resort. - 
[task 2020-02-05T23:29:24.762Z] 23:29:24     INFO - GECKO(7289) | MEMORY STAT | vsize 3570MB | residentFast 631MB | heapAllocated 157MB

Comment 1

[Intermittent Failures Robot]

11 failures in 3364 pushes (0.003 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-central: 11

Platform breakdown:

• linux64-ccov: 10
• linux1804-64-ccov: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1613549&startday=2020-02-03&endday=2020-02-09&tree=all

Comment 2

[Harry Twyford]

Attachment: Bug 1613549 - Part 1 - Move existing tests into a urlbar/tests/browser/tips directory.

Comment 3

[Harry Twyford]

Attachment: Bug 1613549 - Part 2 - Split browser_search_tips.js in two.

Depends on D62205

Comment 4

[Harry Twyford]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=14dae464ab44a30983101631671ee6f13f8db6ae

Comment 5

[Harry Twyford]

I'll be on PTO next week, but my patch here is pretty firmly blocked by bug 1612903. If this failure becomes an issue in the meantime, we could reduce the timeout in checkTip when waiting for a tip to not appear. Since we don't use SHOW_TIP_DELAY_MS anymore in the Search Tip provider, we could reduce that wait from 600ms to something much shorter like 100ms. This would save a significant amount of time on the test since that timeout is used often.

Comment 6

[Intermittent Failures Robot]

6 failures in 4690 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-central: 6

Platform breakdown:

• linux64-ccov: 4
• windows10-64-shippable-qr: 1
• linux1804-64-ccov: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1613549&startday=2020-02-10&endday=2020-02-16&tree=all

Comment 7

[Marco Castelluccio [:marco]]

(In reply to Harry Twyford [:harry] [PTO 2/17-21] from comment #5)

I'll be on PTO next week, but my patch here is pretty firmly blocked by bug 1612903. If this failure becomes an issue in the meantime, we could reduce the timeout in checkTip when waiting for a tip to not appear. Since we don't use SHOW_TIP_DELAY_MS anymore in the Search Tip provider, we could reduce that wait from 600ms to something much shorter like 100ms. This would save a significant amount of time on the test since that timeout is used often.

It seems like we should do this anyway, or we pointlessly wait making these tests slower without a real reason.
If we don't need to wait anymore, why not removing the timeout altogether instead of just reducing to 100 ms?

Comment 8

[Marco Bonardo [:mak]]

(In reply to Marco Castelluccio [:marco] from comment #7)

If we don't need to wait anymore, why not removing the timeout altogether instead of just reducing to 100 ms?

Most of the urlbar code and handling is async and depends on I/O and network, when some things are supposed to NOT happen there's nothing to listen for, thus to stay on the safe side it's better to check after a timeout. I think 100ms would be an acceptable compromise.
I must note though that it would only save 5s, the timeout is at 45s and the failure above takes 48s, so we'd be barely in, that means the refactoring is still necessary longer term.
I will prepare a patch to reduce the timeout, and investigate bug 1612903 in the meanwhile.

Comment 9

[Marco Bonardo [:mak]]

Attachment: Bug 1613549 - Reduce the timeout in browser_search_tips.js because the original delay has been removed. r=adw

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba373603f390
Reduce the timeout in browser_search_tips.js because the original delay has been removed. r=adw

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ba373603f390

Comment 12

[Marco Bonardo [:mak]]

To avoid confusion I'm going to clone this bug to work on moving/splitting tests, regardless harry's patches are bitrotted and I'd prefer a different tests structure.
Since we landed a fix here, let's close this and file a new one for that work.
That will also help tracking uplifts.

Comment 13

[Marco Bonardo [:mak]]

Comment on attachment 9127328 [details]
Bug 1613549 - Reduce the timeout in browser_search_tips.js because the original delay has been removed. r=adw

Beta/Release Uplift Approval Request

• User impact if declined: test only change
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: test only change
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): test only change
• String changes made/needed:

Comment 14

[Ryan VanderMeulen [:RyanVM]]

test-only changes don't need approval to land

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/efcde5f316e0dffca74535da62562e88976a88bf

Comment 16

[Intermittent Failures Robot]

5 failures in 4045 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-central: 5

Platform breakdown:

• linux64-ccov: 5

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1613549&startday=2020-02-17&endday=2020-02-23&tree=all