1097605 - Update method for getting mobile data state in Usage app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Viorela Ioia [:viorela]]

Test test_cost_control_data_alert_mobile.py is failing in latest master, due to changes in https://github.com/mozilla-b2g/gaia/commit/5ae28ff11b982e2bd7d1aa097cda131536952bdc

We need to update the way we check if data mobile tracking is on in Usage app. 

We currently use 'is_selected' method, in order to do that: https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/apps/cost_control/app.py#L31

'is_mobile_data_tracking_on' method should be modified, according to changes in Bug 1069484

Jenkins report: http://jenkins1.qa.scl3.mozilla.com/job/flame-kk-319.b2g-inbound.ui.functional.smoke/1098/HTML_Report/

Comment 1

[Viorela Ioia [:viorela]]

I worked on this issue and I didn't found any way of getting the state of the gaia-checkbox. 
That is because it is in the shadow DOM and marionette cannot see it. 
There's already Bug 1061698 opened for getting the ability to work in shadow DOM.

As we are blocked by Bug 1061698, I will disable test_cost_control_data_alert_mobile.py for now. 
I'll leave this bug open, in order to do the updates in the test, after Bug 1061698 is fixed.

Comment 2

[Viorela Ioia [:viorela]]

Attachment: Github PR: https://github.com/mozilla-b2g/gaia/pull/26064

Comment 3

[Viorela Ioia [:viorela]]

Test test_cost_control_ftu.py is also failing because of this: http://jenkins1.qa.scl3.mozilla.com/job/flame-kk-319.mozilla-central.ui.functional.non-smoke/115/HTML_Report/

I'll disable it too in the PR I opened

Comment 4

[Florin Strugariu [:Bebe]]

Comment on attachment 8521378 [details] [review]
Github PR: https://github.com/mozilla-b2g/gaia/pull/26064

https://github.com/mozilla-b2g/gaia/commit/acf0bfd223f6b4acb3a62db48746c7a591e1479f

Comment 5

[Martijn Wargers (dead)]

Attachment: cost_control_ftu

I was able to get these tests working locally on my phone. No need for any shadow dom usage, the elements were all accessible by itself.

I added comments to the sections were things go a little weird.

It seems that is_selected() isn't able to get the checked state of a gaia-checkbox.
Gaia-checkbox is a custom made element:
http://mxr.mozilla.org/gaia/source/shared/elements/gaia_checkbox/script.js#95
I guess Marionette can't get the checked state of those custom elements (it has to have wrappedJSObject btw, because 'checked' is also 'user' defined). I guess this method was only made for regular form control checkboxes in websites, as this is all based on Selenium, afaik.

Comment 6

[Martijn Wargers (dead)]

The same goes for get_attribute(), btw. I tried to use that too, but I'm not able to get the checked property of that element with that, either: http://selenium-python.readthedocs.org/en/latest/api.html
"
Gets the given attribute or property of the element.

This method will return the value of the given property if this is set, otherwise it returns the value of the attribute with the same name if that exists, or None.
"

Malini, do you know if Marionette is able to return custom defined properties of elements? In my testing, I wasn't able to.

Comment 7

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #6)
> The same goes for get_attribute(), btw. I tried to use that too, but I'm not
> able to get the checked property of that element with that, either:
> http://selenium-python.readthedocs.org/en/latest/api.html
> "
> Gets the given attribute or property of the element.
> 
> This method will return the value of the given property if this is set,
> otherwise it returns the value of the attribute with the same name if that
> exists, or None.
> "
> 
> Malini, do you know if Marionette is able to return custom defined
> properties of elements? In my testing, I wasn't able to.

For get_attribute, we follow exactly what Selenium is doing, which is: if element.hasAttribute(<attribute name>) returns something, we should return that value. Do you know if hasAttribute() works for custom attributes?

What other ways have you tried to retrieve it? Have you tried with marionette.execute_script("return arguments[0].<attribute name>;", [element]) or marionette.execute_script("return arguments[0].wrappedJSObject.<attribute name>;", [element])? I wonder what this would yield

Comment 8

[Martijn Wargers (dead)]

(In reply to Malini Das [:mdas] from comment #7)
> (In reply to Martijn Wargers [:mwargers] (QA) from comment #6)
> What other ways have you tried to retrieve it? Have you tried with
> marionette.execute_script("return arguments[0].<attribute name>;",
> [element]) or marionette.execute_script("return
> arguments[0].wrappedJSObject.<attribute name>;", [element])? I wonder what
> this would yield

It turns out that get_attribute() does work as expected. is_selected() doesn't work, still.

Comment 9

[Martijn Wargers (dead)]

I updated the pull request now to use get_attribute().

Comment 10

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8536927 [details] [review]
cost_control_ftu

Like viorela underlined, we might encounter false positives. I let a proposal in the PR.

Comment 11

[Martijn Wargers (dead)]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #8)
> (In reply to Malini Das [:mdas] from comment #7)
> > (In reply to Martijn Wargers [:mwargers] (QA) from comment #6)
> > What other ways have you tried to retrieve it? Have you tried with
> > marionette.execute_script("return arguments[0].<attribute name>;",
> > [element]) or marionette.execute_script("return
> > arguments[0].wrappedJSObject.<attribute name>;", [element])? I wonder what
> > this would yield
> 
> It turns out that get_attribute() does work as expected. is_selected()
> doesn't work, still.

Oh, never mind. get_attribute() doesn't actually work.
The workaround is to use:
return self.marionette.execute_script("""
            return window.wrappedJSObject.document.getElementById('mobileCheck').checked;
        """)
Because usage of wrappedJSObject allows you to access user defined objects.

Comment 12

[Martijn Wargers (dead)]

So my question to you is, should is_selected() and get_attribute() work here for user defined objects or not?

Comment 13

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #12)
> So my question to you is, should is_selected() and get_attribute() work here
> for user defined objects or not?

is_selected() shouldn't work because this is a custom attribute that this method doesn't know anything about. I'm not sure if get_attribute should be modified to work here... dburns, do you have an opinion about get_attributes working on custom attributes?

Comment 14

[Martijn Wargers (dead)]

Comment on attachment 8536927 [details] [review]
cost_control_ftu

Updated the pull request to go back to the old execute_script workaround.

Comment 15

[David Burns :automatedtester]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #12)
> So my question to you is, should is_selected() and get_attribute() work here
> for user defined objects or not?

Can you give an example in html? get_attribute should return anything that getAttribute/hasAttribute works against

Comment 16

[Martijn Wargers (dead)]

(In reply to David Burns :automatedtester from comment #15)
> (In reply to Martijn Wargers [:mwargers] (QA) from comment #12)
> > So my question to you is, should is_selected() and get_attribute() work here
> > for user defined objects or not?
> 
> Can you give an example in html? get_attribute should return anything that
> getAttribute/hasAttribute works against

See comment 5, it's a custom made element, gaia-checkbox:
http://mxr.mozilla.org/gaia/source/shared/elements/gaia_checkbox/script.js#95

http://mxr.mozilla.org/gaia/source/apps/costcontrol/index.html?force=1#67
http://mxr.mozilla.org/gaia/source/apps/costcontrol/elements/datausage-tab.html?force=1#19

I guess I should be able to write an example of that in html.

Comment 17

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8536927 [details] [review]
cost_control_ftu

About the checked of the box, it's because we use a webcomponent (gaia-checkbox) and the value belongs to the shadow DOM now. I think you should update the comment to point to bug 1061698.

After that, we're good to go, to me.

Comment 18

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8536927 [details] [review]
cost_control_ftu

Clearing the review while we find a consensus around the gaia-checkbox.

Comment 19

[Martijn Wargers (dead)]

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #18)
> Comment on attachment 8536927 [details] [review]
> cost_control_ftu
> 
> Clearing the review while we find a consensus around the gaia-checkbox.

Why? I can't get get_attribute()/is_selected() to work, so I have to use the workaround for now (which is really not too bad).

gaia-checkbox doesn't even set an attribute, only a property. That might actually be the reason why get_attribute()/is_selected() is not working.

Comment 20

[Martijn Wargers (dead)]

Comment on attachment 8536927 [details] [review]
cost_control_ftu

See comment 19. I think the workaround is good, since I can't use the webdriver api for it, currently.

Comment 21

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8536927 [details] [review]
cost_control_ftu

We can go with the workaround then. But before merging it, please file a bug to track the issue and paste the bug number in this comment [1]

[1] https://github.com/mozilla-b2g/gaia/pull/26803/files#diff-986525b7445d1f327d5e604e23352313R34

Comment 22

[Martijn Wargers (dead)]

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #21)
> Comment on attachment 8536927 [details] [review]
> cost_control_ftu
> 
> We can go with the workaround then. But before merging it, please file a bug
> to track the issue and paste the bug number in this comment [1]
> 
> [1]
> https://github.com/mozilla-b2g/gaia/pull/26803/files#diff-
> 986525b7445d1f327d5e604e23352313R34

Ok, I filed bug 1113742 and updated the pull request.

Comment 23

[Martijn Wargers (dead)]

(In reply to David Burns :automatedtester from comment #15)
> (In reply to Martijn Wargers [:mwargers] (QA) from comment #12)
> > So my question to you is, should is_selected() and get_attribute() work here
> > for user defined objects or not?
> 
> Can you give an example in html? get_attribute should return anything that
> getAttribute/hasAttribute works against

I attached a testcase in bug 1113742 now.

Comment 24

[Martijn Wargers (dead)]

Adhoc runs for the 2 tests, to make sure there are no intermittent failures:
http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/507/
http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/508/

Comment 25

[Martijn Wargers (dead)]

test_cost_control_ftu is passing 21 times in a row.
test_cost_control_data_alert_mobile was failing all the time, but I think it happened, because I didn't specify "carrier" in "TYPE".
So I started up another adhoc test run with that TYPE:
http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/509/

Comment 26

[Martijn Wargers (dead)]

Ok, that's not it, It's still timing out in connect_to_cell_data. I'll just keep that test disabled for now and file a new bug for it.

Comment 27

[Martijn Wargers (dead)]

Ok, I disabled test_cost_control_data_alert_mobile.py again, referencing the new bug number:
+disabled = Bug 1115180 for getting it re-enabled
https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=4909b081b121

I would like to get this pull request in. Should I re-ask reviews for disabling this test? Seems a bit overkill, right?

Comment 28

[Florin Strugariu [:Bebe]]

Nope. You can go ahead and disable that test

Comment 29

[Martijn Wargers (dead)]

Ok, thanks! Then I'll merge this pull request.

Comment 30

[Martijn Wargers (dead)]

Checked in: https://github.com/mozilla-b2g/gaia/commit/6ef93c4e5dbdb35e2816b0884725ec7477a93c20
Bug 1115180 is for getting test_cost_control_data_alert_mobile.py re-enabled.