1135383 - Convert Marionette unit tests to use SpecialPowers.pushPrefEnv

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Martijn Wargers (dead)]

Attachment: marionette.diff

These are the marionette-test files that need converting:
layout/base/tests/marionette/test_touchcaret.py
layout/base/tests/marionette/test_selectioncarets.py
layout/base/tests/marionette/test_selectioncarets_multiplerange.py
testing/marionette/client/marionette/tests/unit/test_switch_remote_frame.py 
testing/marionette/client/marionette/tests/unit/test_getactiveframe_oop.py (this test doesn't seem to work currently, see bug 925688, comment 124)

You can run and test those files, using: ./mach marionette-test TEST_FILE

Also, if you happen to see instances of addPermission, those need to be converted to use pushPermissions, see here for example of this:
http://mxr.mozilla.org/mozilla-central/source/dom/base/test/test_XHR_anon.html?force=1#172

Comment 1

[Anish]

Attachment: marionette.patch

Comment 2

[Martijn Wargers (dead)]

Comment on attachment 8567670 [details] [diff] [review]
marionette.patch

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

::: layout/base/tests/marionette/test_selectioncarets_multiplerange.py
@@ +29,2 @@
>              ('true' if enabled else 'false'))
> +        

The pushPrefEnv calls (and hence the execute_async_script calls can be combined into one, I think.

::: testing/marionette/client/marionette/tests/unit/test_switch_remote_frame.py
@@ +22,5 @@
>              """)
> +        self.marionette.execute_async_script(
> +            'SpecialPowers.pushPrefEnv({"set": [["dom.ipc.browser_frames.oop_by_default", true]]}, marionetteScriptFinished);')
> +        self.marionette.execute_async_script(
> +            'SpecialPowers.pushPrefEnv({"set": [["dom.mozBrowserFramesEnabled", true]]}, marionetteScriptFinished);')

I guess I could have also said that in the previous time I review, but the execute_async_script calls could be combined into one, combining the pushPrefEnv calls into one.

Comment 3

[Martijn Wargers (dead)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3445f0bb7da1

Comment 4

[Martijn Wargers (dead)]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #2)
> I guess I could have also said that in the previous time I review, but the
> execute_async_script calls could be combined into one, combining the
> pushPrefEnv calls into one.

But don't bother with it anymore, at this point, unless Joel Maher sees something wrong with the patch and you need to make other changes, anyway.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8567670 [details] [diff] [review]
marionette.patch

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

code wise this looks great and please address the issues Martijn has already brought up.

Comment 6

[Anish]

Attachment: marionette.patch

Comment 7

[Martijn Wargers (dead)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=75cd2b486908

Comment 8

[Martijn Wargers (dead)]

Attachment: marionette.diff (for checkin)

Now with check-in comment, this is ready to be checked in.

Comment 9

[Carsten Book [:Tomcat]]

Hi, this failed to apply:

applying marionette.diff
patching file layout/base/tests/marionette/test_selectioncarets_multiplerange.py
Hunk #1 FAILED at 15
1 out of 1 hunks FAILED -- saving rejects to file layout/base/tests/marionette/test_selectioncarets_multiplerange.py.rej
patch failed, unable to continue (try -v)


could you take a look, thanks!

Comment 10

[Anish]

Attachment: marionette.diff

Comment 11

[Martijn Wargers (dead)]

Comment on attachment 8569311 [details] [diff] [review]
marionette.diff

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

test_selectioncarets_multiplerange.py is not there in the patch.

Comment 12

[Martijn Wargers (dead)]

Attachment: marionette.diff

This is an update patch, ready for checkin. It's updated to the latest trunk, so it should apply cleanly now.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc2c018e12c7

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/dc2c018e12c7