Closed Bug 1478799 Opened 2 years ago Closed 2 years ago

[wdspec] Add tests for no browsing context

Categories

(Testing :: geckodriver, enhancement, P2)

Version 3
enhancement

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

Attachments

(1 file)

Only a couple of the WebDriver commands have a test which checks for the behavior when there is no browsing context existent. We should get such a test added to all the other commands.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Priority: P3 → P2
Comment on attachment 8996059 [details]
Bug 1478799 - [wdspec] Add tests for no browsing context.

https://reviewboard.mozilla.org/r/260320/#review267566

::: testing/web-platform/tests/webdriver/tests/support/fixtures.py:280
(Diff revision 2)
> +def closed_window(session, create_window):
> +    new_handle = create_window()
> +    original_handle = session.window_handle
> +
> +    session.window_handle = new_handle
> +    session.close()
> +    assert new_handle not in session.handles
> +
> +    yield new_handle
> +
> +    session.window_handle = original_handle

This is clever.

::: testing/web-platform/tests/webdriver/tests/support/fixtures.py:286
(Diff revision 2)
> +    new_handle = create_window()
> +    original_handle = session.window_handle
> +
> +    session.window_handle = new_handle
> +    session.close()
> +    assert new_handle not in session.handles

Might be a good idea to add an explanation string to this since
it’s an assertion in a fixture in case it should fail:

> assert new_handle not in session.handles, "Unable to close window {}".format(new_handle)
Attachment #8996059 - Flags: review?(ato) → review+
Comment on attachment 8996059 [details]
Bug 1478799 - [wdspec] Add tests for no browsing context.

https://reviewboard.mozilla.org/r/260320/#review267566

> Might be a good idea to add an explanation string to this since
> it’s an assertion in a fixture in case it should fail:
> 
> > assert new_handle not in session.handles, "Unable to close window {}".format(new_handle)

Sure, I can update the fixture with that.
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s a9fcccba1de1d1efa2808a91a5cf83f362c1fff6 -d a1ce51fcdaa3: rebasing 476168:a9fcccba1de1 "Bug 1478799 - [wdspec] Add tests for no browsing context. r=ato" (tip)
merging testing/web-platform/tests/webdriver/tests/conftest.py
merging testing/web-platform/tests/webdriver/tests/minimize_window/minimize.py
merging testing/web-platform/tests/webdriver/tests/support/fixtures.py
warning: conflicts while merging testing/web-platform/tests/webdriver/tests/conftest.py! (edit, then use 'hg resolve --mark')
warning: conflicts while merging testing/web-platform/tests/webdriver/tests/minimize_window/minimize.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b1b2a51c5f31
[wdspec] Add tests for no browsing context. r=ato
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12254 for changes under testing/web-platform/tests
https://hg.mozilla.org/mozilla-central/rev/b1b2a51c5f31
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Upstream PR merged
I'm getting https://treeherder.mozilla.org/logviewer.html#?job_id=204910940&repo=try&lineNumber=15281 when I change click event handling and button hit testing which requires fixing also https://bugzilla.mozilla.org/show_bug.cgi?id=1498383
But I can't reproduce that failure locally.
I'd appreciate any hints what might cause such error.
Flags: needinfo?(hskupin)
Flags: needinfo?(ato)
oh, it is something like `test_no_browsing_context`                                                            | **FAIL: 5/10, PASS: 5/10** | `ValueError: Element already in cache: foo` |

But I'm having great trouble to understand how other changes to clear.py can cause that.
Olli, can you please file a new bug for that or put it on one of the bugs you are currently working on? The question is kinda unrelated to this test patch. The problem at least comes from https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/testing/web-platform/tests/tools/webdriver/webdriver/client.py#672-673. Thanks.
Flags: needinfo?(hskupin)
Flags: needinfo?(bugs)
Flags: needinfo?(ato)
With regards to test_no_browsing_context [1], element_clear is meant
to return an error when the current browsing context (content
browser) has been discarded [2].

This test creates a fake element to test this behaviour, because
element_clear() takes that as input.  Because an element with the
identifier "foo" already exists in the cache, you are seeing a
“ValueError: Element already in cache” exception.  This is likely
a problem with the order in which the tests are run.

The fix here is probably to make it possible to construct the Element
class without a session, so that we can re-use the Element abstraction
for serialising the web element.  I’ve filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1499057 about this.

  [1] https://searchfox.org/mozilla-central/rev/3a54520d8d2319a4116866371ed3d9ed2ec0cc2b/testing/web-platform/tests/webdriver/tests/element_clear/clear.py#49-53
  [2] https://searchfox.org/mozilla-central/rev/3a54520d8d2319a4116866371ed3d9ed2ec0cc2b/testing/marionette/driver.js#2582
Flags: needinfo?(bugs)
Depends on: 1499135
Depends on: 1499159
Maybe more related to bug 1498383?(In reply to Andreas Tolfsen ❲:ato❳ from comment #18)
> This test creates a fake element to test this behaviour, because
> element_clear() takes that as input.  Because an element with the
> identifier "foo" already exists in the cache, you are seeing a
> “ValueError: Element already in cache” exception.  This is likely
> a problem with the order in which the tests are run.

Please note that I will follow-up on bug 1499057.
You need to log in before you can comment on or make changes to this bug.