754178 - Expose SpecialPowers in Marionette JS

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Philipp von Weitershausen [:philikon]]

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!

Comment 1

[Philipp von Weitershausen [:philikon]]

Attachment: Part 1 (v1): Add SpecialPowers to chrome scripts

Comment 2

[Philipp von Weitershausen [:philikon]]

Attachment: Part 2 (v1): Add SpecialPowers to content scripts

Comment 3

[Jonathan Griffin (:jgriffin)]

Comment on attachment 623040 [details] [diff] [review]
Part 1 (v1): Add SpecialPowers to chrome scripts

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

Nice!

Comment 4

[Jonathan Griffin (:jgriffin)]

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.

Comment 5

[Philipp von Weitershausen [:philikon]]

(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.

Comment 6

[Philipp von Weitershausen [:philikon]]

Attachment: Part 2 (v2): Add SpecialPowers to content scripts

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.

Comment 7

[Jonathan Griffin (: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?

Comment 8

[Philipp von Weitershausen [:philikon]]

Attachment: Part 2 (v2, correct one): Add SpecialPowers to content scripts

Gah, uploaded the wrong file. Sorry about that.

Comment 9

[Jonathan Griffin (: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.

Comment 10

[Philipp von Weitershausen [:philikon]]

Attachment: Part 2 (v3): Add SpecialPowers to content scripts

Excellent points. Comment 9 addressed.

Comment 11

[Jonathan Griffin (: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!

Comment 12

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/mozilla-central/rev/cc0b4c5c3ac2
https://hg.mozilla.org/mozilla-central/rev/921706236f86