Intermittent failure in test_classifier.html and test_classifier_worker.html

RESOLVED FIXED in Firefox 3.6a1

Status

()

Toolkit
Safe Browsing
RESOLVED FIXED
9 years ago
4 years ago

People

(Reporter: mossop, Assigned: mossop)

Tracking

({intermittent-failure})

Trunk
Firefox 3.6a1
intermittent-failure
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
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.
(Assignee)

Updated

9 years ago
Whiteboard: [orange]
(Assignee)

Comment 1

9 years ago
Created attachment 383089 [details] [diff] [review]
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.

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?
(Assignee)

Updated

9 years ago
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).
(Assignee)

Comment 4

9 years ago
(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.

Comment 5

9 years ago
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.

Updated

9 years ago
Attachment #383089 - Flags: review?(ddahl) → review?(sdwilsh)

Updated

9 years ago
Attachment #383089 - Flags: review?(sdwilsh) → review+
Comment on attachment 383089 [details] [diff] [review]
patch rev 1

r=sdwilsh
(Assignee)

Comment 7

9 years ago
Fixed on trunk: http://hg.mozilla.org/mozilla-central/rev/f6ba964bf6e3
Status: NEW → RESOLVED
Last Resolved: 9 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
(Assignee)

Comment 10

9 years ago
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]
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
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!
Keywords: intermittent-failure
Whiteboard: [orange]
Component: Phishing Protection → Phishing Protection
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.