1499135 - ensure 'no browsing context' test in clear.py isn't relying on the previous page runs

Status: RESOLVED WONTFIX

(Testing :: geckodriver, defect, P2)

Description

[Olli Pettay [:smaug]]

Attachment: clear_py_no_browsing_context.diff

Is this too ugly for now?

Comment 1

[Andreas Tolfsen 「:ato」]

Comment on attachment 9017242 [details] [diff] [review]
clear_py_no_browsing_context.diff

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

r+wc

This should do the trick for the time being.  I’ve filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1499057 to fix the
underlying problem, but feel free to apply this first: I can’t see
it doing any harm.

::: testing/web-platform/tests/webdriver/tests/element_clear/clear.py
@@ +47,5 @@
>      assert value is None
>  
>  
>  def test_no_browsing_context(session, closed_window):
> +    element = Element("foo" . time.time(), session)

Are you sure this works?

I would expect "foo()".format(time.time()) to be more successful.

Comment 2

[Olli Pettay [:smaug]]

oops. And lovely. that didn't show up as an error when running the test.

Comment 3

[Olli Pettay [:smaug]]

Attachment: clear_py_no_browsing_context.diff

Just a simple concatenation then :)

Comment 4

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

(In reply to Andreas Tolfsen ❲:ato❳ from comment #1)
> I would expect "foo()".format(time.time()) to be more successful.

I assume you meant `"foo{}".format(..)` here.

Comment 6

[Andreas Tolfsen 「:ato」]

(In reply to Henrik Skupin (:whimboo) from comment #4)
> (In reply to Andreas Tolfsen ❲:ato❳ from comment #1)
> > I would expect "foo()".format(time.time()) to be more successful.
> 
> I assume you meant `"foo{}".format(..)` here.

Yes, sorry.  My Mac keyboard option key gets stuck all the time.

Comment 7

[Andreas Tolfsen 「:ato」]

Comment on attachment 9017271 [details] [diff] [review]
clear_py_no_browsing_context.diff

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

This works too, thanks 👍

I’ll clean this up later when I address the underlying problem with
the Element abstraction.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/97be1d70d0cd
ensure 'no browsing context' test in clear.py isn't relying on the previous page runs, r=ato

Comment 9

[Web Platform Test Sync Bot (IRC #interop)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13541 for changes under testing/web-platform/tests

Comment 10

[Web Platform Test Sync Bot (IRC #interop)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/97be1d70d0cd

Comment 12

[Web Platform Test Sync Bot (IRC #interop)]

Upstream PR merged

Comment 13

[Web Platform Test Sync Bot (IRC #interop)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/13541
* Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/aNfcHWFHRLq-mn8AKgHzPg)

Comment 14

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

The patch got backed out because it causes test failures and is not needed anytime longer due to the full fix on bug 1499057:

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