It should only be possible to hit this in xpcshell tests, but it'd be nice to at least not have this littering the logs: *** WARN addons.manager: Exception calling callback: TypeError: blocklistWindow is null This happens because the xpcshell tests can't open windows, so the open() call returns null. Just need a null check in there: https://hg.mozilla.org/mozilla-central/file/f42a83257653/toolkit/mozapps/extensions/nsBlocklistService.js#l1000 No test needed, though bonus points if you manually verify that the above warning shops showing when running the following xpcshell test: toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js
Created attachment 684837 [details] [diff] [review] add a null check I didn't test, can you please guide me how to run test? Thanks!
Attachment #684837 - Flags: review?(bmcbride)
Great :) I'll give you a chance to test it before I review your patch. (In reply to zeyu [:car] from comment #1) > I didn't test, can you please guide me how to run test? Thanks! You can run just that test via running the following in your repository: mach xpcshell-test toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js Or if you remove just the filename, keeping the path, it'll run all xpcshell tests in that directory. The following page has all the gritty details, though it describes the old (more complex) way of running this type of test: https://developer.mozilla.org/en-US/docs/Writing_xpcshell-based_unit_tests
Assignee: nobody → zeyufly
Status: NEW → ASSIGNED
Hi Blair, I tried to run the test, however it seems that the number of lines that can be displayed in terminal is limited so that I cannot see the full log. I also tried ./mach -l log.log xpcshell-test toolkit/mozapps/extensions/test/xpcshell /test_blocklistchange.js it only gave me an empty file. So how can I see the full test log? Thanks! BTW, I am running Ubuntu 12.04
Mach doesn't currently log test output. I suppose we should file a bug on that. You can always do something like: ./mach xpcshell-test path/to/test > log.log
I've run the test and I believe the warning was gone.
Comment on attachment 684837 [details] [diff] [review] add a null check Taking this since Blair is out sick today
Attachment #684837 - Flags: review?(bmcbride) → review?(dtownsend+bugmail)
Comment on attachment 684837 [details] [diff] [review] add a null check Looks good. One minor style nit, the rest of this file doesn't use braces around single statements like this. I've gone ahead and corrected that and will land it in a moment. Thanks for the patch!
Attachment #684837 - Flags: review?(dtownsend+bugmail) → review+
Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/c615b961c7b4
Target Milestone: --- → mozilla20
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.