Closed Bug 916026 Opened 11 years ago Closed 11 years ago

Permanent Orange: TEST-UNEXPECTED-FAIL | test_bug854467.js | true == false

Categories

(Thunderbird :: Testing Infrastructure, defect)

Product:
Thunderbird
Component:
Testing Infrastructure
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(thunderbird26 fixed, thunderbird27 fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 27.0
Tracking Flags:
Tracking Status
thunderbird26 --- fixed
thunderbird27 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

The fix v2
(deleted), patch
keeler
: review+
Details | Diff | Splinter Review
Seen on Thunderbird since bug 854467 landed: TEST-UNEXPECTED-FAIL | /builds/slave/test/build/xpcshell/tests/dom/plugins/test/unit/test_bug854467.js | test failed (with xpcshell return code: 0), see following log: >>>>>>> TEST-INFO | (xpcshell/head.js) | test MAIN run_test pending (1) TEST-UNEXPECTED-FAIL | /builds/slave/test/build/xpcshell/tests/dom/plugins/test/unit/test_bug854467.js | true == false - See following stack: JS frame :: /builds/slave/test/build/xpcshell/tests/dom/plugins/test/unit/test_bug854467.js :: check_state :: line 7 JS frame :: /builds/slave/test/build/xpcshell/tests/dom/plugins/test/unit/test_bug854467.js :: run_test :: line 13 JS frame :: /builds/slave/test/build/xpcshell/head.js :: _execute_test :: line 348 JS frame :: -e :: <TOP_LEVEL> :: line 1 native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0 TEST-INFO | (xpcshell/head.js) | exiting test
David, I've done some testing, and it appears that for Thunderbird, the "Test Plugin" defaults to click-to-play, whereas for Firefox it is defaulting to enabled. Any ideas where we might look for the difference?
Flags: needinfo?(dkeeler)
Keywords: intermittent-failure
The reason of this failure is the place of all-thunderbird.js. "plugin.default.state" is set to 2 in dist/bin/greprefs.js but it seems to be overwritten by dist/bin/defaults/pref/all-thunderbird.js. The place should be dist/bin/mail/defaults/pref/ to solve this failure? Or we should use the preference value in test_bug854467.js?
There's something strange going on here. dist/bin/greprefs.js aka all.js sets plugin.default.state to 2. firefox.js sets plugin.default.state to 1. all-thunderbird.js sets plugin.default.state to 1. If I run the latest nightly of Firefox, the default plugin state is 1. It looks to me like xpcshell-tests aren't picking up the "app" part of the Firefox preferences. I wonder if this is due to the recent separation of the browser chrome... So I think the solution would be to change dist/bin/greprefs.js to set plugin.default.state to 1.
(and fix the assumptions in the xpcshell test).
I didn't think xpcshell read app prefs. We didn't want to change the default state in greprefs.js because we weren't sure what apps it would affect (Seamonkey is certainly not ready for it). I think this test should just be rewritten to set the state to whatever it means to be testing. See the test fixups that happened when we changed the default state for Firefox in bug 899080.
Flags: needinfo?(dkeeler)
Attached patch The fix (obsolete) (deleted) — — Splinter Review
This ensures the preference is set to what the test is expecting before it starts.
Assignee: nobody → mbanner
Status: NEW → ASSIGNED
Attachment #805296 - Flags: review?(dkeeler)
Comment on attachment 805296 [details] [diff] [review] The fix Review of attachment 805296 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/test/unit/test_bug854467.js @@ +11,5 @@ > } > > function run_test() { > + Services.prefs.setIntPref("plugin.default.state", 2); > + I think it would be less fragile to set the enabled state on the plugin tag, like setTestPluginEnabledState() and it's callers: http://hg.mozilla.org/mozilla-central/annotate/dc909122bcf5/dom/plugins/test/mochitest/utils.js#l37 I don't expect the implementation and meaning of the pref to change soon, but wouldn't rely on it.
Now that i noticed that this test already sets the plugins enabledState... it should just be enough to set the initial expected |tag.enabledState=Ci.nsIPluginTag.STATE_ENABLED| and reset it to the original value later. If other tests in dom/plugins/unit need this as well, a utility function could be added to head_plugins.js, utilizing the existing |get_test_plugintag()|.
Attached patch The fix v2 (deleted) — — Splinter Review
With setting the flag on the plugin rather than the preference.
Attachment #805296 - Attachment is obsolete: true
Attachment #805296 - Flags: review?(dkeeler)
Attachment #805366 - Flags: review?(dkeeler)
Comment on attachment 805366 [details] [diff] [review] The fix v2 Review of attachment 805366 [details] [diff] [review]: ----------------------------------------------------------------- Thanks. ::: dom/plugins/test/unit/test_bug854467.js @@ +3,4 @@ > * file, You can obtain one at http://mozilla.org/MPL/2.0/. > */ > > +Components.utils.import("resource://gre/modules/Services.jsm"); I don't think this is needed anymore? @@ +12,4 @@ > > function run_test() { > let tag = get_test_plugintag(); > + tag.enabledState = Ci.nsIPluginTag.STATE_ENABLED; Could we also reset the state to the original (default) value?
Comment on attachment 805366 [details] [diff] [review] The fix v2 Review of attachment 805366 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. I'm not a plugin peer, but it's a test-only, one-line change and I think we've had enough eyes on it, so r+. ::: dom/plugins/test/unit/test_bug854467.js @@ +12,4 @@ > > function run_test() { > let tag = get_test_plugintag(); > + tag.enabledState = Ci.nsIPluginTag.STATE_ENABLED; My understanding is that each xpcshell test environment is self-contained, so the plugin's enabledState doesn't need to be reset at the end. (If this isn't the case, it would be good to disabuse me of the notion, though.)
Attachment #805366 - Flags: review?(dkeeler) → review+
(In reply to David Keeler (:keeler) from comment #31) > My understanding is that each xpcshell test environment is self-contained, > so the plugin's enabledState doesn't need to be reset at the end. Right, every xpcshell test get's a new profile, temp directory and recently plugins directory for things like the plugin renaming tests.
https://hg.mozilla.org/mozilla-central/rev/558d0fec359c
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 27.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: