The existing tests didn't catch bug 1237103.
Summary: Improve apprep tests → Improve Application Reputation tests
Maybe bug 1237396 could help to at least partially test it against the real server?
Summary: Improve Application Reputation tests → Improve Application Reputation unit tests
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).
Created attachment 8708127 [details] [diff] [review] bug1237370-better-logging.patch
Attachment #8708127 - Flags: review?(gpascutto)
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.
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.