Update method for getting mobile data state in Usage app

RESOLVED FIXED

Status

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: viorela, Assigned: martijn.martijn)

Tracking

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-qa-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

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/
Assignee: nobody → viorela.ioia
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.
Depends on: 1061698
Attachment #8521378 - Flags: review?(robert.chira)
Attachment #8521378 - Flags: review?(florin.strugariu)
QA Whiteboard: [fxosqa-auto-s4]
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
Attachment #8521378 - Flags: review?(robert.chira) → review+
QA Whiteboard: [fxosqa-auto-s4] → [fxosqa-auto-s4][fxosqa-auto-dropped-s4][fxosqa-auto-backlog+]
QA Whiteboard: [fxosqa-auto-s4][fxosqa-auto-dropped-s4][fxosqa-auto-backlog+] → [fxosqa-auto-dropped-s4][fxosqa-auto-backlog+]
Posted file 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.
Attachment #8536927 - Flags: review?(viorela.ioia)
Attachment #8536927 - Flags: review?(jlorenzo)
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.
Flags: needinfo?(mdas)
(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
Flags: needinfo?(mdas)
(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.
Assignee: viorela.ioia → martijn.martijn
I updated the pull request now to use get_attribute().
Attachment #8536927 - Flags: review?(viorela.ioia) → review-
Comment on attachment 8536927 [details] [review]
cost_control_ftu

Like viorela underlined, we might encounter false positives. I let a proposal in the PR.
Attachment #8536927 - Flags: review?(jlorenzo) → review-
(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.
So my question to you is, should is_selected() and get_attribute() work here for user defined objects or not?
Flags: needinfo?(mdas)
(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?
Flags: needinfo?(mdas) → needinfo?(dburns)
Comment on attachment 8536927 [details] [review]
cost_control_ftu

Updated the pull request to go back to the old execute_script workaround.
Attachment #8536927 - Flags: review?(viorela.ioia)
Attachment #8536927 - Flags: review?(jlorenzo)
Attachment #8536927 - Flags: review-
(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
Flags: needinfo?(dburns) → needinfo?(martijn.martijn)
(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 on attachment 8536927 [details] [review]
cost_control_ftu

Clearing the review while we find a consensus around the gaia-checkbox.
Attachment #8536927 - Flags: review+
(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 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.
Attachment #8536927 - Flags: review?(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
Attachment #8536927 - Flags: review?(jlorenzo) → review+
Depends on: 1113742
(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.
Flags: needinfo?(martijn.martijn)
(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.
Attachment #8536927 - Flags: review?(viorela.ioia) → review?(npark)
Attachment #8536927 - Flags: review?(npark) → review+
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/
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.
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?
Flags: needinfo?(jlorenzo)
Nope. You can go ahead and disable that test
Flags: needinfo?(jlorenzo)
Ok, thanks! Then I'll merge this pull request.
Checked in: https://github.com/mozilla-b2g/gaia/commit/6ef93c4e5dbdb35e2816b0884725ec7477a93c20
Bug 1115180 is for getting test_cost_control_data_alert_mobile.py re-enabled.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
QA Whiteboard: [fxosqa-auto-dropped-s4][fxosqa-auto-backlog+] → [fxosqa-auto-dropped-s4][fxosqa-auto-s7][fxosqa-auto-points=4]
Flags: in-qa-testsuite+
No longer depends on: 1061698
Depends on: 1194224
You need to log in before you can comment on or make changes to this bug.