David Burns :automatedtester Assignee
	Description • 7 years ago

This area needs more tests! I have them!

	Andreas Tolfsen ❲:ato❳
	Comment 20 • 7 years ago
mozreview-review

Comment on attachment 8891546 [details]
Bug 1376625 - Add Is Element Selected wdspec tests

https://reviewboard.mozilla.org/r/153468/#review169742

I am also missing a tests for the following:

(1) Using an element that is not <input type=checkbox>, <input type=radio>, or <option>.
(2) <option> that is not in a <select>.*
(3) Passing different types to the "element id" URI template.
(4) Type test on the return value.

::: testing/web-platform/tests/webdriver/get_element_attribute.py:8
(Diff revision 4)
> +alert_doc = inline("<script>window.alert()</script>")
> +check_doc = inline("<input id=checked type=checkbox checked/><input id=notChecked type=checkbox/>")
> +option_doc = inline("<select><option id=notSelected>r-</option><option id=selected selected>r+</option></select>")

Any particular reason for making this so unreadable?

::: testing/web-platform/tests/webdriver/get_element_attribute.py:8
(Diff revision 4)
> +alert_doc = inline("<script>window.alert()</script>")
> +check_doc = inline("<input id=checked type=checkbox checked/><input id=notChecked type=checkbox/>")
> +option_doc = inline("<select><option id=notSelected>r-</option><option id=selected selected>r+</option></select>")

Any particular reason for making this so unreadable?

::: testing/web-platform/tests/webdriver/get_element_attribute.py:10
(Diff revision 4)
> +from support.inline import inline
> +from support.asserts import assert_error, assert_success
> +
> +alert_doc = inline("<script>window.alert()</script>")
> +check_doc = inline("<input id=checked type=checkbox checked/><input id=notChecked type=checkbox/>")
> +option_doc = inline("<select><option id=notSelected>r-</option><option id=selected selected>r+</option></select>")

option_doc isn’t used.

::: testing/web-platform/tests/webdriver/get_element_attribute.py:13
(Diff revision 4)
> +alert_doc = inline("<script>window.alert()</script>")
> +check_doc = inline("<input id=checked type=checkbox checked/><input id=notChecked type=checkbox/>")
> +option_doc = inline("<select><option id=notSelected>r-</option><option id=selected selected>r+</option></select>")
> +
> +
> +# X.2 Get Element Attribute

s/X/13/g

::: testing/web-platform/tests/webdriver/get_element_attribute.py:18
(Diff revision 4)
> +    session.url =  check_doc
> +    element = session.find.css("#checked", all=False)

These two steps are unnecessary.  You can pass an invalid web element reference below, like you do in the user prompt test.

::: testing/web-platform/tests/webdriver/get_element_attribute.py:29
(Diff revision 4)
> +def test_get_element_attribute_alert_prompt(session):
> +    # X.2 step 2
> +    session.url = alert_doc
> +
> +    result = session.transport.send("GET", "session/{session_id}/element/{element_id}/attribute/id"
> +                                    .format(session_id=session.session_id,
> +                                            element_id="random"))
> +
> +    assert_error(result, "unexpected alert open")

This test is good, but you’re missing tests for the user prompt handler.

::: testing/web-platform/tests/webdriver/get_element_attribute.py:38
(Diff revision 4)
> +    result = session.transport.send("GET", "session/{session_id}/element/{element_id}/attribute/id"
> +                                    .format(session_id=session.session_id,
> +                                            element_id="random"))
> +
> +    assert_error(result, "unexpected alert open")
> +

You’re missing a test for step 3:

> Let element be the result of trying to get a known element with argument element id.

::: testing/web-platform/tests/webdriver/get_element_attribute.py:41
(Diff revision 4)
> +
> +    assert_error(result, "unexpected alert open")
> +
> +
> +def test_get_element_attribute_element_stale(session):
> +    # 7.2 step 4

13.2

::: testing/web-platform/tests/webdriver/get_element_attribute.py:49
(Diff revision 4)
> +    session.execute_script("return window.location.reload()")
> +    result = session.transport.send("GET", "session/{session_id}/element/{element_id}/attribute/id"
> +                                    .format(session_id=session.session_id,
> +                                            element_id=element.id))
> +
> +    assert_error(result, "stale element reference")

Also missing test for step 5 regarding boolean attributes.  We have tests for this in Marionette which you can steal.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:7
(Diff revision 4)
> +from support.inline import inline
> +from support.fixtures import create_dialog
> +
> +
> +alert_doc = inline("<script>window.alert()</script>")
> +check_doc = inline("<input id=checked type=checkbox checked/><input id=notChecked type=checkbox/>")

You need an additional document for testing <input type=radio>.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:16
(Diff revision 4)
> +                       </select>
> +                       """)
> +
> +# 13.1 Is Element Selected
> +
> +def test_is_element_selected_no_browsing_context(session, create_window):

No need to include "is_element_selected" here.  That is given by the test file name.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:19
(Diff revision 4)
> +    session.url =  check_doc
> +    element = session.find.css("#checked", all=False)

These lines don’t do anything.  You can pass in an invalid web element reference below.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:30
(Diff revision 4)
> +                                            element_id=element.id))
> +
> +    assert_error(result, "no such window")
> +
> +
> +def test_get_element_property_handle_prompt_dismiss(new_session):

This isn’t a test for getting element properties.  Drop "get_element_property" altogether throughout this file.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:35
(Diff revision 4)
> +def test_get_element_property_handle_prompt_dismiss(new_session):
> +    # 13.1 step 2
> +    _, session = new_session({"alwaysMatch": {"unhandledPromptBehavior": "dismiss"}})
> +    session.url = inline("<input id=foo>")
> +
> +    element = session.find.css("input", all=False)

Use fake reference so reduce number of steps that could go wrong.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:68
(Diff revision 4)
> +    session.url = inline("<input id=foo>")
> +
> +    element = session.find.css("input", all=False)

Drop and use fake web element reference.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:132
(Diff revision 4)
> +
> +    assert_error(result, "unexpected alert open")
> +    assert_dialog_handled(session, "dismiss #3")
> +
> +
> +def test_is_element_selected_element_present_checked(session):

s/is_element_selected//g

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:132
(Diff revision 4)
> +
> +    assert_error(result, "unexpected alert open")
> +    assert_dialog_handled(session, "dismiss #3")
> +
> +
> +def test_is_element_selected_element_present_checked(session):

The test below for <option> is OK because it includes the name in the test title.  This test should in my opinion be called "test_checkbox_selected".

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:133
(Diff revision 4)
> +    assert_error(result, "unexpected alert open")
> +    assert_dialog_handled(session, "dismiss #3")
> +
> +
> +def test_is_element_selected_element_present_checked(session):
> +    # 13.1 step 3 - 5

There should be a separate test for step 3:

> Let element be the result of trying to get a known element with argument element id.

This will return a "no such element" error if the element ID cannot be found.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:133
(Diff revision 4)
> +    assert_error(result, "unexpected alert open")
> +    assert_dialog_handled(session, "dismiss #3")
> +
> +
> +def test_is_element_selected_element_present_checked(session):
> +    # 13.1 step 3 - 5

This is as far as I can tell only a test for step 5.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:143
(Diff revision 4)
> +                                            element_id=element.id))
> +
> +    assert_success(result, True)
> +
> +
> +def test_is_element_selected_element_present_not_checked(session):

Similarly, "test_checkbox_not_selected" would be a better name.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:144
(Diff revision 4)
> +
> +    assert_success(result, True)
> +
> +
> +def test_is_element_selected_element_present_not_checked(session):
> +    # 7.2 step 3 - 5

Only tests step 5.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:144
(Diff revision 4)
> +
> +    assert_success(result, True)
> +
> +
> +def test_is_element_selected_element_present_not_checked(session):
> +    # 7.2 step 3 - 5

This is section 13.1?

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:155
(Diff revision 4)
> +
> +    assert_success(result, False)
> +
> +
> +def test_is_element_selected_element_present_selected(session):
> +    # 7.2 step 3 - 5

13.1 step 5

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:166
(Diff revision 4)
> +
> +    assert_success(result, True)
> +
> +
> +def test_is_element_selected_element_present_not_selected(session):
> +    # 7.2 step 3 - 5

13.1 step 5

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:176
(Diff revision 4)
> +                                            element_id=element.id))
> +
> +    assert_success(result, False)
> +
> +
> +def test_is_element_selected_element_stale(session):

I would move this test earlier so we test step 4 before step 5.

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:177
(Diff revision 4)
> +
> +    assert_success(result, False)
> +
> +
> +def test_is_element_selected_element_stale(session):
> +    # 7.2 step 4

13.1

::: testing/web-platform/tests/webdriver/tests/is_element_selected.py:180
(Diff revision 4)
> +
> +def test_is_element_selected_element_stale(session):
> +    # 7.2 step 4
> +    session.url = check_doc
> +    element = session.find.css("#checked", all=False)
> +    session.execute_script("return window.location.reload()")

Use session.reload() so that the action is synchronous, or better still, navigate to another document entirely.

	Andreas Tolfsen ❲:ato❳
	Comment 21 • 7 years ago
mozreview-review

Comment on attachment 8891546 [details]
Bug 1376625 - Add Is Element Selected wdspec tests

https://reviewboard.mozilla.org/r/153468/#review169840

::: testing/web-platform/tests/webdriver/get_element_attribute.py:1
(Diff revision 4)
> +import json

I think that the get_element_attribute.py test should be in the next commit?

	Andreas Tolfsen ❲:ato❳
	Comment 22 • 7 years ago
mozreview-review

Comment on attachment 8891548 [details]
Bug 1376625 - Add Get Element Property wdspec tests

https://reviewboard.mozilla.org/r/162664/#review169846

Missing tests for:

- Type checks of return value.
- Difference between attributes/properties.
- Properties added dynamically (not in the document).
- Test state after element has been interacted with (click).

::: testing/web-platform/tests/webdriver/tests/get_element_property.py:5
(Diff revision 4)
> +from support.asserts import assert_error, assert_dialog_handled, assert_success
> +from support.inline import inline
> +from support.fixtures import create_dialog
> +
> +input = inline("<input id=i1>")

Rename this variable because "input" is the name of a built-in function in Python.

::: testing/web-platform/tests/webdriver/tests/get_element_property.py:10
(Diff revision 4)
> +input = inline("<input id=i1>")
> +
> +
> +# 13.3 Get Element Property
> +
> +def test_get_element_property_no_browsing_context(session, create_window):

Please drop "get_element_property" for all tests in this file.

::: testing/web-platform/tests/webdriver/tests/get_element_property.py:13
(Diff revision 4)
> +    session.url = input
> +    element = session.find.css("input", all=False)

Use fake element ID instead to reduce the number of steps involved to a minimum.

::: testing/web-platform/tests/webdriver/tests/get_element_property.py:27
(Diff revision 4)
> +    session.url = inline("<input id=foo>")
> +
> +    element = session.find.css("input", all=False)

Use fake element ID.

::: testing/web-platform/tests/webdriver/tests/get_element_property.py:62
(Diff revision 4)
> +    session.url = inline("<input id=foo>")
> +
> +    element = session.find.css("input", all=False)

Use fake element ID.

::: testing/web-platform/tests/webdriver/tests/get_element_property.py:95
(Diff revision 4)
> +    session.url = inline("<input id=foo>")
> +
> +    element = session.find.css("input", all=False)

Use fake element ID.

::: testing/web-platform/tests/webdriver/tests/get_element_property.py:125
(Diff revision 4)
> +                                            element_id=element.id))
> +
> +    assert_error(result, "unexpected alert open")
> +    assert_dialog_handled(session, "dismiss #3")
> +
> +

Missing test for step 3:

> Let element be the result of trying to get a known element with argument element id.

::: testing/web-platform/tests/webdriver/tests/get_element_property.py:130
(Diff revision 4)
> +
> +def test_get_element_property_element_stale(session):
> +    # 13.3 step 4
> +    session.url = input
> +    element = session.find.css("input", all=False)
> +    session.execute_script("return window.location.reload()")

Use session.reload() or better yet, navigate to a different document.

::: testing/web-platform/tests/webdriver/tests/get_element_property.py:138
(Diff revision 4)
> +def test_get_element_property_element_non_existent(session):
> +    # 13.3 step 5-7
> +    session.url = input
> +    element = session.find.css("input", all=False)
> +    result = session.transport.send("GET", "session/{session_id}/element/{element_id}/property/foo"
> +                                    .format(session_id=session.session_id,
> +                                            element_id=element.id))
> +
> +    assert_success(result, None)

Could we also have an assertion in here that compares the return value against execute_script("return input.foo")?

::: testing/web-platform/tests/webdriver/tests/get_element_property.py:153
(Diff revision 4)
> +
> +def test_get_element_property_element(session):
> +    # 13.3 step 5-7
> +    session.url = input
> +    element = session.find.css("input", all=False)
> +    result = session.transport.send("GET", "session/{session_id}/element/{element_id}/property/i1"

This test is plain wrong.  "i1" isn’t a property on <input id=i1>.  This test should call …/property/id.

	Andreas Tolfsen ❲:ato❳
	Comment 23 • 7 years ago
mozreview-review

Comment on attachment 8892336 [details]
Bug 1376625 - Add Get Element Tag Name wdspec tests

https://reviewboard.mozilla.org/r/163300/#review169884

I miss a test for the qualified name, which means including an XHTML document with a namespace.  You also need to check that the returned value is a string and that it matches DOMElement#tagName.

::: testing/web-platform/tests/webdriver/tests/get_element_tag_name.py:6
(Diff revision 3)
> +from support.asserts import assert_error, assert_dialog_handled, assert_success
> +from support.inline import inline
> +from support.fixtures import create_dialog
> +
> +
> +# 13.6 Get Element Property

s/Property/Tag Name/

::: testing/web-platform/tests/webdriver/tests/get_element_tag_name.py:8
(Diff revision 3)
> +from support.fixtures import create_dialog
> +
> +
> +# 13.6 Get Element Property
> +
> +def test_get_element_tag_name_no_browsing_context(session, create_window):

s/get_element_tag_name//g

::: testing/web-platform/tests/webdriver/tests/get_element_tag_name.py:11
(Diff revision 3)
> +    session.url = input
> +    element = session.find.css("input", all=False)

Use fake element ID to reduce number of steps.

::: testing/web-platform/tests/webdriver/tests/get_element_tag_name.py:25
(Diff revision 3)
> +    session.url = inline("<input id=foo>")
> +
> +    element = session.find.css("input", all=False)

Drop.

::: testing/web-platform/tests/webdriver/tests/get_element_tag_name.py:60
(Diff revision 3)
> +    session.url = inline("<input id=foo>")
> +
> +    element = session.find.css("input", all=False)

Drop.

::: testing/web-platform/tests/webdriver/tests/get_element_tag_name.py:123
(Diff revision 3)
> +                                            element_id=element.id))
> +
> +    assert_error(result, "unexpected alert open")
> +    assert_dialog_handled(session, "dismiss #3")
> +
> +

Missing test for step 3:

> Let element be the result of trying to get a known element with argument element id. 

This would return a "no such element" error on an invalid web element reference.

::: testing/web-platform/tests/webdriver/tests/get_element_tag_name.py:126
(Diff revision 3)
> +    assert_dialog_handled(session, "dismiss #3")
> +
> +
> +def test_get_element_tag_name_element_stale(session):
> +    # 13.6 step 4
> +    session.url = input

input is not a variable defined in the test, but a built-in function.

::: testing/web-platform/tests/webdriver/tests/get_element_tag_name.py:128
(Diff revision 3)
> +
> +def test_get_element_tag_name_element_stale(session):
> +    # 13.6 step 4
> +    session.url = input
> +    element = session.find.css("input", all=False)
> +    session.execute_script("return window.location.reload()")

Use session.reload() or better still, navigate to another document.

	Andreas Tolfsen ❲:ato❳
	Comment 24 • 7 years ago
mozreview-review

Comment on attachment 8893129 [details]
Bug 1376625 - Updating meta data for newly added tests

https://reviewboard.mozilla.org/r/164132/#review169888

::: commit-message-cd917:1
(Diff revision 1)
> +Bug 1376625 - Updating Meta data for newly added tests r?ato

s/Meta/meta

::: testing/web-platform/meta/webdriver/tests/is_element_selected.py.ini:3
(Diff revision 1)
> +[is_element_selected.py]
> +  type: wdspec
> +  expected: TIMEOUT

I don’t understand why this test would time out.

	Andreas Tolfsen ❲:ato❳
	Comment 25 • 7 years ago
mozreview-review

Comment on attachment 8892335 [details]
Bug 1376625 - Add Get Element Rect wdspec tests

https://reviewboard.mozilla.org/r/163298/#review169894

I am missing tests for the following:

- The "calculate the absolute position" of the element algorithm in in the spec.
- Tests for elements that are outside the viewport.
- Tests for elements that are partially in the viewport.
- Tests for elements that are outside the document.
- Tests for elements that have to be scrolled to.

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:7
(Diff revision 3)
> +from support.inline import inline
> +from support.fixtures import create_dialog
> +
> +input = inline("<input>")
> +div = inline("""
> +<style type="text/css">

The type is superfluous.

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:23
(Diff revision 3)
> +    }
> +</style>
> +<div>
> +""")
> +
> +# 13.3 Get Element Property

13.7 Get Element Rect

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:25
(Diff revision 3)
> +<div>
> +""")
> +
> +# 13.3 Get Element Property
> +
> +def test_get_element_rect_no_browsing_context(session, create_window):

s/get_element_rect//g

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:26
(Diff revision 3)
> +""")
> +
> +# 13.3 Get Element Property
> +
> +def test_get_element_rect_no_browsing_context(session, create_window):
> +    # 13.3 step 1

13.7

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:28
(Diff revision 3)
> +    session.url = input
> +    element = session.find.css("input", all=False)

Use fake web element reference.

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:40
(Diff revision 3)
> +
> +    assert_error(result, "no such window")
> +
> +
> +def test_get_element_rect_handle_prompt_dismiss(new_session):
> +    # 13.2 step 2

13.7

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:42
(Diff revision 3)
> +    session.url = inline("<input id=foo>")
> +
> +    element = session.find.css("input", all=False)

Drop.

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:75
(Diff revision 3)
> +    assert_dialog_handled(session, "dismiss #3")
> +
> +
> +
> +def test_get_element_rect_handle_prompt_accept(new_session):
> +    # 13.2 step 2

13.7

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:109
(Diff revision 3)
> +    assert_success(result, "foo")
> +    assert_dialog_handled(session, "dismiss #3")
> +
> +
> +def test_get_element_rect_handle_prompt_missing_value(session, create_dialog):
> +    # 13.2 step 2

13.7

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:140
(Diff revision 3)
> +                                            element_id=element.id))
> +
> +    assert_error(result, "unexpected alert open")
> +    assert_dialog_handled(session, "dismiss #3")
> +
> +

Missing test for step 3:

> Let element be the result of trying to get a known element with argument element id.

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:142
(Diff revision 3)
> +    assert_error(result, "unexpected alert open")
> +    assert_dialog_handled(session, "dismiss #3")
> +
> +
> +def test_get_element_rect_element_stale(session):
> +    # 13.3 step 4

13.7

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:145
(Diff revision 3)
> +
> +def test_get_element_rect_element_stale(session):
> +    # 13.3 step 4
> +    session.url = input
> +    element = session.find.css("input", all=False)
> +    session.execute_script("return window.location.reload()")

Use session.reload(), which is synchronous, or navigate to a different document.

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:156
(Diff revision 3)
> +
> +
> +def test_get_element_rect_payload(session):
> +    # step 8
> +    session.url = div
> +    element = session.find.css("div", all=False)

You could use the document element (<body>) here.

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:171
(Diff revision 3)
> +    assert "height" in value
> +    assert "x" in value
> +    assert "y" in value
> +    assert isinstance(value["width"], (int, float))
> +    assert isinstance(value["height"], (int, float))
> +    assert isinstance(value["x"], (int, float))

Missing type check for "y".

::: testing/web-platform/tests/webdriver/tests/get_element_rect.py:182
(Diff revision 3)
> +    assert_success(result, {"x": 10,
> +                            "y": 10,
> +                            "width": 100,
> +                            "height": 50})

I would compare these values to DOM properties extracted via execute_script.

	Andreas Tolfsen ❲:ato❳
	Comment 26 • 7 years ago
mozreview-review

Comment on attachment 8891547 [details]
Bug 1376625 - Add Get Element Attribute wdspec tests

https://reviewboard.mozilla.org/r/153470/#review169900

::: testing/web-platform/tests/webdriver/tests/get_element_attribute.py:10
(Diff revision 4)
>  
> +# 13.2 Get Element Attribute
>  
> -# X.2 Get Element Attribute
> -
>  def test_get_element_attribute_no_browsing_context(session, create_window):

s/get_element_attribute//g

::: testing/web-platform/tests/webdriver/tests/get_element_attribute.py:13
(Diff revision 4)
> -# X.2 Get Element Attribute
> -
>  def test_get_element_attribute_no_browsing_context(session, create_window):
> -    # X.2 step 1
> +    # 13.2 step 1
>      session.window_handle = create_window()
> -    session.url =  check_doc
> +    session.url = input

input is not defined in this file, but it is a built-in function in Python.

::: testing/web-platform/tests/webdriver/tests/get_element_attribute.py:13
(Diff revision 4)
> -    session.url =  check_doc
> -    element = session.find.css("#checked", all=False)
> +    session.url = input
> +    element = session.find.css("input", all=False)

Drop these two lines to keep the number of actually needed steps to a minimum.

::: testing/web-platform/tests/webdriver/tests/get_element_attribute.py:27
(Diff revision 4)
> +    session.url = inline("<input id=foo>")
> +
> +    element = session.find.css("input", all=False)

Drop.

::: testing/web-platform/tests/webdriver/tests/get_element_attribute.py:61
(Diff revision 4)
> +    session.url = inline("<input id=foo>")
> +
> +    element = session.find.css("input", all=False)

Drop.

::: testing/web-platform/tests/webdriver/tests/get_element_attribute.py:94
(Diff revision 4)
> +    session.url = inline("<input id=foo>")
> +
> +    element = session.find.css("input", all=False)

Drop.

::: testing/web-platform/tests/webdriver/tests/get_element_attribute.py:123
(Diff revision 4)
> +                                    .format(session_id=session.session_id,
> +                                            element_id=element.id))
> +
> +    assert_error(result, "unexpected alert open")
> +    assert_dialog_handled(session, "dismiss #3")
>  

Missing test for step 3:

> Let element be the result of trying to get a known element with argument element id. 

This would return a "no such element" error.

::: testing/web-platform/tests/webdriver/tests/get_element_attribute.py:126
(Diff revision 4)
> +    assert_error(result, "unexpected alert open")
> +    assert_dialog_handled(session, "dismiss #3")
>  
>  def test_get_element_attribute_element_stale(session):
> -    # 7.2 step 4
> -    session.url = check_doc
> +    # 13.2 step 4
> +    session.url = input

input is undefined.

::: testing/web-platform/tests/webdriver/tests/get_element_attribute.py:128
(Diff revision 4)
>  
>  def test_get_element_attribute_element_stale(session):
> -    # 7.2 step 4
> -    session.url = check_doc
> -    element = session.find.css("#checked", all=False)
> +    # 13.2 step 4
> +    session.url = input
> +    element = session.find.css("input", all=False)
>      session.execute_script("return window.location.reload()")

Use session.reload(), which is synchronous, or navigate to a different document.

::: testing/web-platform/tests/webdriver/tests/get_element_attribute.py:167
(Diff revision 4)
> +    ("select", ["autofocus", "disabled", "multiple", "required"]),
> +    ("textarea", ["autofocus", "disabled", "readonly", "required"]),
> +    ("track", ["default"]),
> +    ("video", ["autoplay", "controls", "loop", "muted"])
> +])
> +def test_get_element_attribute_boolean(session, tag, attrs):

When you remove "get_element_attribute", please rename this test "test_boolean_attribute".

::: testing/web-platform/tests/webdriver/tests/get_element_attribute.py:215
(Diff revision 4)
> +    element = session.find.css("p", all=False)
> +    result = session.transport.send("GET", "session/{session_id}/element/{element_id}/attribute/{attr}"
> +                                    .format(session_id=session.session_id,
> +                                            element_id=element.id,
> +                                            attr="itemscope"))
> +    assert_success(result, None)

Missing tests for regular attributes that are not boolean attributes, and a test for checking it doesn’t return properties.  It would also be good with a test where the document changes the attribute/property at runtime.

	David Burns :automatedtester Assignee
	Comment 27 • 7 years ago
mozreview-review-reply

Comment on attachment 8891546 [details]
Bug 1376625 - Add Is Element Selected wdspec tests

https://reviewboard.mozilla.org/r/153468/#review169742

> Any particular reason for making this so unreadable?

Looks like a push was missing...

	Andreas Tolfsen ❲:ato❳
	Comment 34 • 7 years ago
mozreview-review

Comment on attachment 8891547 [details]
Bug 1376625 - Add Get Element Attribute wdspec tests

https://reviewboard.mozilla.org/r/153470/#review171476

::: testing/web-platform/tests/webdriver/get_element_attribute.py:1
(Diff revision 5)
> +import json

I think you meant to remove this file?

	Andreas Tolfsen ❲:ato❳
	Comment 35 • 7 years ago
mozreview-review

Comment on attachment 8891548 [details]
Bug 1376625 - Add Get Element Property wdspec tests

https://reviewboard.mozilla.org/r/162664/#review171482

::: testing/web-platform/tests/webdriver/tests/get_element_property.py:153
(Diff revision 5)
> +def test_element(session):
> +    # 13.3 step 5-7
> +    session.url = _input
> +    element = session.find.css("input", all=False)
> +    result = session.transport.send("GET", "session/{session_id}/element/{element_id}/property/id"
> +                                    .format(session_id=session.session_id,
> +                                            element_id=element.id))
> +
> +    assert_success(result, "i1")

Not sure exactly what this test tests.  It gets the <input id=i1> element by it’s attribute which happens to also be the property.

I think we need a test which showcases where the Get Element Property command comes into its right, for example if the attribute ID is changed after the document has loaded, or its value is set dynamically.

	Andreas Tolfsen ❲:ato❳
	Comment 36 • 7 years ago
mozreview-review

Comment on attachment 8892335 [details]
Bug 1376625 - Add Get Element Rect wdspec tests

https://reviewboard.mozilla.org/r/163298/#review171484

	Andreas Tolfsen ❲:ato❳
	Comment 37 • 7 years ago
mozreview-review

Comment on attachment 8892336 [details]
Bug 1376625 - Add Get Element Tag Name wdspec tests

https://reviewboard.mozilla.org/r/163300/#review171486

::: testing/web-platform/tests/webdriver/tests/get_element_tag_name.py:139
(Diff revision 4)
> +def test_get_element_tag_name(session):
> +    # 13.6 step 6
> +    session.url = inline("<input id=foo>")
> +    element = session.find.css("input", all=False)
> +    result = session.transport.send("GET", "session/{session_id}/element/{element_id}/name"
> +                                    .format(session_id=session.session_id,
> +                                            element_id=element.id))
> +    assert_success(result, "input")

This is good, but we also need a test for XHTML namespaces.  It’s fine to leave this for later, but do raise a bug so we can track it.

	David Burns :automatedtester Assignee
	Comment 44 • 7 years ago
mozreview-review-reply

Comment on attachment 8892336 [details]
Bug 1376625 - Add Get Element Tag Name wdspec tests

https://reviewboard.mozilla.org/r/163300/#review171486

> This is good, but we also need a test for XHTML namespaces.  It’s fine to leave this for later, but do raise a bug so we can track it.

https://bugzilla.mozilla.org/show_bug.cgi?id=1390164

	David Burns :automatedtester Assignee
	Comment 45 • 7 years ago
mozreview-review-reply

Comment on attachment 8893129 [details]
Bug 1376625 - Updating meta data for newly added tests

https://reviewboard.mozilla.org/r/164132/#review169888

> I don’t understand why this test would time out.

seems to have cleared up by itself.

	Andreas Tolfsen ❲:ato❳
	Comment 46 • 7 years ago
mozreview-review

Comment on attachment 8891546 [details]
Bug 1376625 - Add Is Element Selected wdspec tests

https://reviewboard.mozilla.org/r/153468/#review173562

	Andreas Tolfsen ❲:ato❳
	Comment 47 • 7 years ago
mozreview-review

Comment on attachment 8893129 [details]
Bug 1376625 - Updating meta data for newly added tests

https://reviewboard.mozilla.org/r/164132/#review173564

	Andreas Tolfsen ❲:ato❳
	Comment 48 • 7 years ago
mozreview-review

Comment on attachment 8891548 [details]
Bug 1376625 - Add Get Element Property wdspec tests

https://reviewboard.mozilla.org/r/162664/#review173568

	Pulsebot
	Comment 55 • 7 years ago

Pushed by dburns@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/29175c7e6faa
Add Get Element Attribute wdspec tests r=ato
https://hg.mozilla.org/integration/autoland/rev/e2ae50805200
Add Get Element Property wdspec tests r=ato
https://hg.mozilla.org/integration/autoland/rev/4af85178dea9
Add Get Element Rect wdspec tests r=ato
https://hg.mozilla.org/integration/autoland/rev/0842eac97833
Add Get Element Tag Name wdspec tests r=ato
https://hg.mozilla.org/integration/autoland/rev/1805706e3d0d
Updating meta data for newly added tests r=ato
https://hg.mozilla.org/integration/autoland/rev/8498aa3df735
Add Is Element Selected wdspec tests r=ato

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 56 • 7 years ago

I had to back this out because it was conflicting with my merge from m-c back to autoland. Feel free to rebase and reland when you're able:

https://hg.mozilla.org/integration/autoland/rev/958c7d99a2c0cf95da3bb4081826e45dfa1ada50

	Pulsebot
	Comment 63 • 7 years ago

Pushed by dburns@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b761a6d0884d
Add Get Element Attribute wdspec tests r=ato
https://hg.mozilla.org/integration/autoland/rev/ff5eb028d845
Add Get Element Property wdspec tests r=ato
https://hg.mozilla.org/integration/autoland/rev/7ee09158305a
Add Get Element Rect wdspec tests r=ato
https://hg.mozilla.org/integration/autoland/rev/323972b0a1e3
Add Get Element Tag Name wdspec tests r=ato
https://hg.mozilla.org/integration/autoland/rev/604f92d3e8ae
Updating meta data for newly added tests r=ato
https://hg.mozilla.org/integration/autoland/rev/3fffaef455b7
Add Is Element Selected wdspec tests r=ato

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 64 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/b761a6d0884d
https://hg.mozilla.org/mozilla-central/rev/ff5eb028d845
https://hg.mozilla.org/mozilla-central/rev/7ee09158305a
https://hg.mozilla.org/mozilla-central/rev/323972b0a1e3
https://hg.mozilla.org/mozilla-central/rev/604f92d3e8ae
https://hg.mozilla.org/mozilla-central/rev/3fffaef455b7