Improve Application Reputation unit tests

RESOLVED FIXED in Firefox 46

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: francois, Assigned: francois)

Tracking

43 Branch
mozilla46
Points:
---

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
The existing tests didn't catch bug 1237103.
(Assignee)

Updated

3 years ago
Summary: Improve apprep tests → Improve Application Reputation tests
Maybe bug 1237396 could help to at least partially test it against the real server?
(Assignee)

Updated

3 years ago
Summary: Improve Application Reputation tests → Improve Application Reputation unit tests
(Assignee)

Comment 2

3 years ago
It turns out this is much harder than I thought. I can't simply add a check to the setup phase of the main unit test to read the current value of the pref because it throws an NS_ERROR_UNEXPECTED. That error comes from:

  https://dxr.mozilla.org/mozilla-central/rev/e0bcd16e1d4b99ba3e542149d0d41e0f60c54b5c/modules/libpref/prefapi.cpp#471

and the "pref" variable is null.

If I can't read the pref in xpcshell tests, then maybe I can read it in a
mochitest. After creating a super simple mochitest, I get the following:

  browser.safebrowsing.appRepURL == http://127.0.0.1:8888/safebrowsing-dummy/update

which comes from:
https://dxr.mozilla.org/mozilla-central/rev/878033e3ca98726f11dd3a30b0768961d221d22a/testing/profiles/prefs_general.js#86

So basically, our automated tests all override the real URL. I guess we'll have to rely on the smoketests (bug 1237763) and the upcoming Marionette tests (bug 1237766).
(Assignee)

Comment 3

3 years ago
Created attachment 8708127 [details] [diff] [review]
bug1237370-better-logging.patch
Attachment #8708127 - Flags: review?(gpascutto)
(Assignee)

Comment 4

3 years ago
Created attachment 8708128 [details] [diff] [review]
bug1237370-cleanup-tests.patch
Attachment #8708128 - Flags: review?(gpascutto)
Attachment #8708127 - Flags: review?(gpascutto) → review+
Attachment #8708128 - Flags: review?(gpascutto) → review+
(In reply to François Marier [:francois] from comment #2)
> So basically, our automated tests all override the real URL. I guess we'll
> have to rely on the smoketests (bug 1237763) and the upcoming Marionette
> tests (bug 1237766).

Please strongly consider fixing this one:
https://bugzilla.mozilla.org/show_bug.cgi?id=1150921

Which will give live feedback if it's working in the field.

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a2cba247fc25
https://hg.mozilla.org/mozilla-central/rev/3714de35de4b
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.