1115905 - Several functions in gaia_test.py switch context to the system app unnecessarily

Status: RESOLVED WONTFIX

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

Description

[Martijn Wargers (dead)]

While working on bug 1109203, I noticed there are several other functions in gaia_test.py that switch context to the system app, where you wouldn't expect it.
It's better that the context doesn't change, when it's not necessary.

I think get_permission, set_permission, set_permission_by_url, displayed_app, is_app_installed, running_apps, _get_pref, _set_pref, bluetooth_enable, bluetooth_disable, connect_to_cell_data, disable_cell_data, enable_wifi, disable_wifi, connect_to_wifi, forget_all_networks, is_wifi_connected, delete_all_sms, get_all_sms, send_sms and is_locked are cases where you wouldn't expect the context to be changed to the system app, but where it happens.

Comment 1

[Martijn Wargers (dead)]

Perhaps a bit unclear worded. Those functions switch context, otherwise they wouldn't work.
I propose to switch context to what it was before, because consumers of these functions shouldn't expect the context to be switched as a side-effect of these functions.

Comment 2

[Dave Hunt [:davehunt] [he/him] ⌚BST]

This has come up before. There's no existing reliable way to switch back to the correct frame. In some cases switching back to the displayed app is satisfactory, however if the test previously had the focus of a nested frame, you will have problems. It's also not possible to restore into a nested frame without also storing a reference to all parent frames. The current scenario of methods changing (and leaving) focus to the system app - although undesirable - was considered the best option. This comes with caution to the test writer, which perhaps should be emphasised (in documentation?)

Of course we could revisit this decision if there is a good proposal for a solution, however I suspect not a lot of time will be invested in this.

Comment 3

[Martijn Wargers (dead)]

Dave, can't we use self.marionette.get_active_frame() to keep a reference to the old frame and then switch back to it after we're done? (like I used in the patch in bug 1109203)

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #3)
> Dave, can't we use self.marionette.get_active_frame() to keep a reference to
> the old frame and then switch back to it after we're done? (like I used in
> the patch in bug 1109203)

Only if the active frame is not a nested one, otherwise you won't be able to switch directly to it from the system app frame - you'd first need to switch to it's ancestor available from the system app and then through each of it's ancestors until you can reach it.

Comment 5

[Martijn Wargers (dead)]

Hmm, that means self.marionette.switch_to_frame(random_frame) doesn't always work?

Comment 6

[Martijn Wargers (dead)]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #5)
> Hmm, that means self.marionette.switch_to_frame(random_frame) doesn't always
> work?
davehunt: if the frame is not available in the current context you will get an exception

davehunt: mwargers: I think perhaps the most complete solution would be to identify not just the active frame but the route to that frame from top
davehunt: then that route can be repeated when switching back
davehunt: alternatively if we could find an alternative to switching to the system app that would work too

Comment 7

[Martijn Wargers (dead)]

From https://github.com/mozilla-b2g/gaia/pull/29160/files#r27468877 :
"


This actually makes us less consistent. In some places we assume that we're in the correct frame, in other places we switch to the system frame but do not attempt to switch back, and here we switch to the system frame and attempt to switch back.

The problem with switching frame is that the test author is not likely to expect this, and may need to account for it. Whilst we can attempt to switch back to the displayed app, this is only an attempt to restore the original frame context - if we were previously in a nested frame, we're going to end up in a different frame. I would favour consistency, and we should either remove the dependency on being in the system frame (where possible), determine if there's a way to somehow fork a process to do the switch and carry out the script, or fall back to some consistent behaviour. Having a decorator for this sounds like a nice approach, and will keep our solution in a central location.

To be clear, I'm r+ this if it works and does not cause regressions, but would like to see a followup that improves our consistency and maintenance.

"

Comment 8

[Martijn Wargers (dead)]

Marking WONTFIX, sorry for the bug spam. If somebody still wants to work on this, please file a new bug for it.