Closed Bug 497884 Opened 12 years ago Closed 11 years ago

Intermittent failure in test_classifier.html and test_classifier_worker.html

Categories

(Toolkit :: Safe Browsing, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3.6a1

People

(Reporter: mossop, Assigned: mossop)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

See http://tinderbox.mozilla.org/showlog.cgi?tree=Firefox&errorparser=unittest&logfile=1244812416.1244818784.3640.gz&buildtime=1244812416

These tests will fail with NS_ERROR_NOT_AVAILABLE if they happen to run while the background update of the url-classifier database is in progress. I have a fix in progress but want to try to track bug 486489 a bit more first.
Whiteboard: [orange]
Attached patch patch rev 1Splinter Review
This patch has two effects.

It allows us to control the url-classifier update interval from a preference and in tests sets that so the earliest an update will happen is after a days running. This should ensure no background updates happen to affect unit tests.

It redirects the url-classifier lookup urls to the local test server. We shouldn't be touching the network during automated tests. The test server will just return errors as nothing exists there, url-classifier should cope with errors ok.

The former is the most sure fix, but we might only want to take the latter on branch since it is a test only change. That should pretty much fix this bug on its own, or at least reduce chances of it happening to pretty much zero.

All of this will also stop bug 486489 from showing up on the tinderbox.
Attachment #383089 - Flags: review?
Attachment #383089 - Flags: review? → review?(ddahl)
(In reply to comment #1)
> Created an attachment (id=383089) [details]
> patch rev 1
> 
> This patch has two effects.
> 
> It allows us to control the url-classifier update interval from a preference
> and in tests sets that so the earliest an update will happen is after a days
> running. This should ensure no background updates happen to affect unit tests.
> 
> It redirects the url-classifier lookup urls to the local test server. We
> shouldn't be touching the network during automated tests. The test server will
> just return errors as nothing exists there, url-classifier should cope with
> errors ok.

Do browser-chrome tests pass with this fix? There are a few tests in there for the about:blocked error pages - the tests themselves are pretty sparse, I'm just curious as to whether the changes you've made cause the MalwareWarden init to be delayed, such that those entries aren't added to the DB (which means the tests would fail, since they would retrieve the actual web content, not the error page).
(In reply to comment #3)
> Do browser-chrome tests pass with this fix? There are a few tests in there for
> the about:blocked error pages - the tests themselves are pretty sparse, I'm
> just curious as to whether the changes you've made cause the MalwareWarden init
> to be delayed, such that those entries aren't added to the DB (which means the
> tests would fail, since they would retrieve the actual web content, not the
> error page).

Yes all tests pass. The MalwareWarden init continues as normal and so the test data is injected into the database and after that the backend list manager is asked to start requesting table updates. It is just these table updates that are affected.
Mossop: I think I better fix a few bugs in the safebrowsing module before I review any patches. Is there anyone else that can review this? I should find a couple of basic-functionality bugs in this module work on asap.
Attachment #383089 - Flags: review?(ddahl) → review?(sdwilsh)
Attachment #383089 - Flags: review?(sdwilsh) → review+
Comment on attachment 383089 [details] [diff] [review]
patch rev 1

r=sdwilsh
Fixed on trunk: http://hg.mozilla.org/mozilla-central/rev/f6ba964bf6e3
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1245457934.1245462125.8540.gz
OS X 10.5.2 mozilla-1.9.1 unit test on 2009/06/19 17:32:14
Flags: wanted1.9.1.x?
Flags: in-testsuite+
Whiteboard: [orange] → [needs 1.9.1 landing: needs approval] [orange]
Target Milestone: --- → Firefox 3.6a1
I've landed just the test harness part of this on the 1.9.1 branch, it should be enough on its own to clear up the leaks and this intermittent failure, however I'll still want to land the rest at a later point.

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/fc26906ae71d
Flags: wanted1.9.1.x?
Whiteboard: [needs 1.9.1 landing: needs approval] [orange] → [orange]
cdleary: Comment 13 is a mis-starring -- #1 it's a timeout and not a failure, and #2 this bug is (purportedly) fixed since 5 months ago.

(I'd suggest filling a new bug, except the starring was for a TryServer build, so it's not necessarily new-bug-worthy as it may have been an issue in the TryServer'd patch)
(In reply to comment #14)

My mistake, didn't mean to star stuff on try, let alone the wrong stuff!
Whiteboard: [orange]
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.