Closed Bug 1341514 Opened 3 years ago Closed 3 years ago

Intermittent toolkit/components/url-classifier/tests/mochitest/test_reporturl.html | Test timed out.

Categories

(Toolkit :: Safe Browsing, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- unaffected
firefox53 --- unaffected
firefox54 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: tnguyen)

References

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell fixed])

Attachments

(1 file, 3 obsolete files)

It looks like this test was just added in bug 1288633.

:tnguyen - Please try to make this test run more reliably. Consider disabling it while you work on it.
Blocks: 1288633
Flags: needinfo?(tnguyen)
I still don't figure out why the intermittent failed.
Should disable the test first before I figure out why this failed.
Flags: needinfo?(tnguyen)
Whiteboard: [test disabled][leave open]
Assignee: nobody → tnguyen
Status: NEW → ASSIGNED
Disable the test for now
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6332dc0a183
Disable test_reporturl.html due to intermittent failure. a=testonly
Keywords: checkin-needed
Whiteboard: [test disabled][leave open] → [test disabled][leave open][stockwell disabled]
Attachment #8841462 - Attachment is obsolete: true
MozReview-Commit-ID: 46bzJ4DVh44
Attachment #8842366 - Flags: review?(gpascutto)
"fake" provider appears to be failed to update because the update url does not exist
https://searchfox.org/mozilla-central/rev/9c1c7106eef137e3413fd867fc1ddfb1d3f6728c/addon-sdk/source/test/preferences/firefox.json#12

We may use "mozilla" in this case, this is good enough to test non-google provider.
MozReview-Commit-ID: BxIW4cMD4V0
Attachment #8842366 - Attachment is obsolete: true
Attachment #8842366 - Flags: review?(gpascutto)
Attachment #8842409 - Flags: review?(gpascutto)
(In reply to Thomas Nguyen[:tnguyen] ni plz from comment #16)
> "fake" provider appears to be failed to update because the update url does
> not exist


Can you explain why this causes the test to fail?
happy to see a fix in here :)
As we added a test data to "fake-phish-simple" list, we enabled "fake" provider actively, then in the next update we will try to connect to server and update the fake-phish-simple
The update url of "fake" provider currently is empty (set to about:blank as default, others default is set) and apprenrently will be failed. 
The about:blank is malformed download url, so the update task will bailed out before the channel is opened (asyncopen) https://searchfox.org/mozilla-central/rev/9c1c7106eef137e3413fd867fc1ddfb1d3f6728c/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp#162
OnStopRequest will not be fired and somehow the next update task will be blocked (timed out).
This is what I traced and this occurs in all failures
OnStopRequest will cancel/finish update then change the inUpdate status of dbservice so the next update could run
https://searchfox.org/mozilla-central/rev/9c1c7106eef137e3413fd867fc1ddfb1d3f6728c/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#1919
That explains why the update tests fail. But why is that affecting this test? Is it failing to get its own DB entries inserted that it's using to trigger a warning screen?
If that causes problems, doesn't this mean our own test tables would be affected too?
I think it would be ok if we run update from listmanager (trigger another update request of another list), but has been effected if we run mochitest which calls update directly from dbservice such as
https://searchfox.org/mozilla-central/source/toolkit/components/url-classifier/tests/UrlClassifierTestUtils.jsm#90
or https://searchfox.org/mozilla-central/source/toolkit/components/url-classifier/tests/browser/classifierHelper.js#193
These call will be blocked because dbservice is still in "inupdate" and it may lead to not be able to "setup" data for next certain test.
That's why after running the test test_reporturl.html, we may have randomly timeout of succession test (like bug 1203438 or bug 1209786).
Comment on attachment 8842409 [details] [diff] [review]
Using the valid update url provider in test

Review of attachment 8842409 [details] [diff] [review]:
-----------------------------------------------------------------

Add a small comment explaining the provider needs a valid update URL or the updates inserting the test data will fail.
Attachment #8842409 - Flags: review?(gpascutto) → review+
MozReview-Commit-ID: 50NcRd2e9hB
Attachment #8842409 - Attachment is obsolete: true
Comment on attachment 8842457 [details] [diff] [review]
Using the valid update url provider in test

Thanks gcp for your review
Attachment #8842457 - Flags: review+
Keywords: checkin-needed
Whiteboard: [test disabled][leave open][stockwell disabled]
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/30dc81445a1b
Using the valid update url provider in test. r=gcp
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/30dc81445a1b
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Whiteboard: [stockwell fixed]
You need to log in before you can comment on or make changes to this bug.