1330323 - Intermittent test_execute_script.py TestExecuteChrome.* | UnsupportedOperationException: Only supported in Firefox

Status: RESOLVED FIXED

(Testing :: Marionette Client and Harness, defect)

Description

[Treeherder Bug Filer]

Filed by: hskupin [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=67955621&repo=try

https://queue.taskcluster.net/v1/task/RNZrNhpqQsCdECMkjubnjA/runs/0/artifacts/public/logs/live_backing.log

Mass failures for test_execute_script.py which were hidden due to bug  1330321

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1330323 - Fix test failures in test_execute_script for Fennec.

A bit of refactoring has to be done to make the tests all working in Fennec.
Those which absolutely cannot be run, have been marked as skipped.

Further some other code has been found which wasn't clearly testing what it
should have been.

Review commit: https://reviewboard.mozilla.org/r/103908/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/103908/

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8825834 [details]
Bug 1330323 - Fix test failures in test_execute_script for Fennec.

https://reviewboard.mozilla.org/r/103908/#review104580

::: testing/marionette/harness/marionette_harness/tests/unit/test_execute_script.py:197
(Diff revision 1)
> +        try:
> -        self.marionette.execute_script("window.wrappedJSObject.foo = 3")
> +            self.marionette.execute_script("window.wrappedJSObject.foo = 3")
> -        self.assertEqual(
> +            self.assertEqual(
> -            3, self.marionette.execute_script("return window.wrappedJSObject.foo"))
> +                self.marionette.execute_script("return window.wrappedJSObject.foo"), 3)
> +        finally:
> +            self.marionette.execute_script("delete window.wrappedJSObject.foo")

Andreas, do you know why it is necessary to reset the property? If we don't do it here the test_exception() will fail because it checks for the same property as not existent.

Comment 3

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8825834 [details]
Bug 1330323 - Fix test failures in test_execute_script for Fennec.

https://reviewboard.mozilla.org/r/103908/#review104580

> Andreas, do you know why it is necessary to reset the property? If we don't do it here the test_exception() will fail because it checks for the same property as not existent.

This is because `execute_script` without a `sandbox` kwarg gets executed in the same sandbox, which means `window.wrappedJSObject.foo` and the `window` state is being preserved.  So the adding a finally-block here makes total sense and is the correct solution.

Comment 4

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8825834 [details]
Bug 1330323 - Fix test failures in test_execute_script for Fennec.

https://reviewboard.mozilla.org/r/103908/#review104594

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d22302d25085
Fix test failures in test_execute_script for Fennec. r=ato

Comment 6

[Guilherme Lima]

Patch landed few hours ago https://hg.mozilla.org/mozilla-central/rev/d22302d25085

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Please uplift this test-only patch to aurora. Thanks.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/36f85907a16e

Comment 9

[BMO Automation]

Moving bug to Testing::Marionette Client and Harness component per bug 1815831.