Closed Bug 754178 Opened 9 years ago Closed 9 years ago

Expose SpecialPowers in Marionette JS

Categories

(Testing :: Marionette, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla15

People

(Reporter: philikon, Assigned: philikon)

References

Details

(Keywords: dev-doc-needed)

Attachments

(2 files, 3 obsolete files)

As part of writing tests wholesale in content JS, we need to be able to modify a few things like prefs. Mochitests typically do this with SpecialPowers, so let's have those in Marionette, too!
Assignee: nobody → philipp
Attachment #623040 - Flags: review?(jgriffin)
Comment on attachment 623040 [details] [diff] [review]
Part 1 (v1): Add SpecialPowers to chrome scripts

Review of attachment 623040 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!
Attachment #623040 - Flags: review?(jgriffin) → review+
Comment on attachment 623042 [details] [diff] [review]
Part 2 (v1): Add SpecialPowers to content scripts

Review of attachment 623042 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but we should use a different pref value for the content and chrome versions of the test.  Otherwise, we cannot detect a silent failure of setCharPref in whichever test is run second.

Also, we should have some tests for specialpowers after switching frames/windows; I'll file a followup bug for that.
Attachment #623042 - Flags: review?(jgriffin) → review+
(In reply to Jonathan Griffin (:jgriffin) from comment #4)
> Looks good, but we should use a different pref value for the content and
> chrome versions of the test.  Otherwise, we cannot detect a silent failure
> of setCharPref in whichever test is run second.
> 
> Also, we should have some tests for specialpowers after switching
> frames/windows; I'll file a followup bug for that.

Excellent points, I'll address them.
Use different prefs for content and chrome; add tests where we first navigate to a different page before accessing SpecialPowers.

For some reason the content navigation test fails for me at the navigate()/goUrl() step, but a bunch of other tests are also broken right now, so I'm not sure what to make of that.
Attachment #623042 - Attachment is obsolete: true
Attachment #624279 - Flags: review?(jgriffin)
Comment on attachment 624279 [details] [diff] [review]
Part 2 (v2): Add SpecialPowers to content scripts

This looks like the wrong patch; this seems to be the "add specialpowers to chrome" patch.  Can you confirm?
Gah, uploaded the wrong file. Sorry about that.
Attachment #624279 - Attachment is obsolete: true
Attachment #624279 - Flags: review?(jgriffin)
Attachment #624551 - Flags: review?(jgriffin)
Comment on attachment 624551 [details] [diff] [review]
Part 2 (v2, correct one): Add SpecialPowers to content scripts

Review of attachment 624551 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great!  The only error I saw when running the tests was due to navigating in chrome; if you skip test_prefs_after_navigate() in chrome by defining it as pass, then all tests pass.

::: testing/marionette/client/marionette/tests/unit/test_specialpowers.py
@@ +19,5 @@
> +
> +    def test_prefs_after_navigate(self):
> +        test_html = self.marionette.absolute_url("test.html")
> +        self.marionette.navigate(test_html)
> +        self.test_prefs()

The chrome version of this test should be "pass", since navigating in chrome causes us to replace shell.xul with whatever, and that completely fuxors Marionette.  For now, it's not a use case we care about.

@@ +23,5 @@
> +        self.test_prefs()
> +
> +class TestSpecialPowersChrome(TestSpecialPowersContent):
> +
> +    testpref = "testing.marionette.contentcharpref"

I think this should be "testing.marionette.chromecharpref", otherwise we're still testing the same pref in content and chrome.
Attachment #624551 - Flags: review?(jgriffin) → review+
Excellent points. Comment 9 addressed.
Attachment #624551 - Attachment is obsolete: true
Attachment #624616 - Flags: review?(jgriffin)
Comment on attachment 624616 [details] [diff] [review]
Part 2 (v3): Add SpecialPowers to content scripts

Review of attachment 624616 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!
Attachment #624616 - Flags: review?(jgriffin) → review+
https://hg.mozilla.org/mozilla-central/rev/cc0b4c5c3ac2
https://hg.mozilla.org/mozilla-central/rev/921706236f86
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: dev-doc-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Blocks: 756607
You need to log in before you can comment on or make changes to this bug.