Remove switch_to_displayed_app from _set_pref / _get_pref

RESOLVED FIXED

Status

Firefox OS
Gaia::UI Tests
--
blocker
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: zac, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
davehunt
: review+
Details | Review | Splinter Review
(Reporter)

Description

4 years ago
The only GaiaData method that does this is also sometimes called from setUp depending upon whether you have prefs set from test vars.

This makes the setUp unstable, depending upon config, whic apps are loaded, with/without restart, etc.

This also normalises the behaviour of these methods - none of them will switch back to frames automatically. The user/test must make that decision explicitly.
(Reporter)

Comment 1

4 years ago
Created attachment 8435883 [details] [review]
github pr
Attachment #8435883 - Flags: review?(dave.hunt)
Attachment #8435883 - Flags: review?(dave.hunt) → review+
Zac: Is this ready to land or is there anything you need from me?
Flags: needinfo?(zcampbell)
(Reporter)

Comment 3

4 years ago
It's ready to land!
Flags: needinfo?(zcampbell)
Thanks. Landed in:
https://github.com/mozilla-b2g/gaia/commit/72b4922f0176f2342b534d90515df37f7976ceac
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Updated

4 years ago
Duplicate of this bug: 1029946
Zac - Can you get this uplifted to 2.0 to unblock CAF?
Flags: needinfo?(zcampbell)
(Reporter)

Comment 8

4 years ago
Lifted to v2.0 as partner was having usability problems.
https://github.com/mozilla-b2g/gaia/commit/41a3030cbcd55dc78f4f85badedc83b6d6282f95

Test-only commit
(Reporter)

Comment 9

4 years ago
Was already doing it :)
Flags: needinfo?(zcampbell)
(In reply to Zac C (:zac) from comment #8)
> Lifted to v2.0 as partner was having usability problems.
> https://github.com/mozilla-b2g/gaia/commit/
> 41a3030cbcd55dc78f4f85badedc83b6d6282f95
> 
> Test-only commit

I tried this patch on following gaia/gecko : 

gaia : https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gaia/commit/?h=mozilla/v2.0&id=729f214b887ce8efe7d870145d31acb2c6427817
gecko : https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gecko/commit/?h=mozilla/v2.0&id=f3721e7a9961f09722975d0d76af31130f8847ef


I can still reproduce #bug 1029946 .Can you please confirm this on flame device running FFOS 2.0 ? Please let me know if I am missing some patches .
Status: RESOLVED → REOPENED
Flags: needinfo?(zcampbell)
Resolution: FIXED → ---
blocking-b2g: --- → 2.0?
(Reporter)

Comment 11

4 years ago
(In reply to Tapas Kumar Kundu from comment #10)
> (In reply to Zac C (:zac) from comment #8)
> > Lifted to v2.0 as partner was having usability problems.
> > https://github.com/mozilla-b2g/gaia/commit/
> > 41a3030cbcd55dc78f4f85badedc83b6d6282f95
> > 
> > Test-only commit
> 
> I tried this patch on following gaia/gecko : 
> 
> gaia :
> https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gaia/commit/?h=mozilla/
> v2.0&id=729f214b887ce8efe7d870145d31acb2c6427817
> gecko :
> https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gecko/commit/?h=mozilla/
> v2.0&id=f3721e7a9961f09722975d0d76af31130f8847ef
> 
> 
> I can still reproduce #bug 1029946 .Can you please confirm this on flame
> device running FFOS 2.0 ? Please let me know if I am missing some patches .

The Gaia commit you referenced does *not* include the patch I made today. We cannot confirm this on Flame/v2.0 device when the tests include the Gaia patch.

Also if you run with --restart on the command line which will bypass this code block and it will work without the patch.
Flags: needinfo?(zcampbell)
(Reporter)

Comment 12

4 years ago
I also checked your whole tree on that site and as of this moment it does not mirror Mozilla's github perfectly so it does not include the patch I made.
(In reply to Zac C (:zac) from comment #12)
> I also checked your whole tree on that site and as of this moment it does
> not mirror Mozilla's github perfectly so it does not include the patch I
> made.

I also tried with "--restart" option but I still see #bug 1029946  . BTW, "--restart" restarts b2g process so we cannot run it in our automation. 

If it is fixed in tip of gaia then we don't need to do any work here. Could you please give me a link to your patch so that I can apply it manually .  Please note from my previous #comment 10 that I already tried patch [1] 

[1] https://github.com/mozilla-b2g/gaia/commit/41a3030cbcd55dc78f4f85badedc83b6d6282f95
Flags: needinfo?(zcampbell)
Blocks: 1011657
Severity: normal → blocker
I'll + this as a test blocker, but if this no longer reproduces post the testing in comment 13, then we'll want to close this bug.
blocking-b2g: 2.0? → 2.0+
(Reporter)

Comment 15

4 years ago
(In reply to Tapas Kumar Kundu from comment #13)
> (In reply to Zac C (:zac) from comment #12)
> > I also checked your whole tree on that site and as of this moment it does
> > not mirror Mozilla's github perfectly so it does not include the patch I
> > made.
> 
> I also tried with "--restart" option but I still see #bug 1029946  . BTW,
> "--restart" restarts b2g process so we cannot run it in our automation. 
> 
> If it is fixed in tip of gaia then we don't need to do any work here. Could
> you please give me a link to your patch so that I can apply it manually . 
> Please note from my previous #comment 10 that I already tried patch [1] 
> 
> [1]
> https://github.com/mozilla-b2g/gaia/commit/
> 41a3030cbcd55dc78f4f85badedc83b6d6282f95

That is the only patch to apply. Can you give me more information and context about the failure? The code works 100% fine for us on our builds. Can you provide me with the HTML report (obtained using --html-output results.html ) ? Can you tell me what code is running prior to this? What kind of Gaia build are you running?

Is your team performing and customisations to the lockscreen at build time that may override the lockscreen in Gaia?
Flags: needinfo?(zcampbell)
(Reporter)

Comment 16

4 years ago
Right OK I have worked this out..

You have built Gaia with NO_LOCK_SCREEN=1 so that there is no lockscreen app installed. This is why it is failing.

I don't think this kind of Gaia build is a valid testing scenario and as such it's not really one we would support. 

If you build Gaia with NO_LOCK_SCREEN=0 then the tests will work fine.
(In reply to Zac C (:zac) from comment #16)
> Right OK I have worked this out..
> 
> You have built Gaia with NO_LOCK_SCREEN=1 so that there is no lockscreen app
> installed. This is why it is failing.
> 
> I don't think this kind of Gaia build is a valid testing scenario and as
> such it's not really one we would support. 
> 
> If you build Gaia with NO_LOCK_SCREEN=0 then the tests will work fine.

We've had this flag enabled for our internal builds since it was first introduced, so this isn't a new change for us.  There's no way to make the test work again with NO_LOCK_SCREEN=0?
(Reporter)

Comment 18

4 years ago
You could make the unlock function in the atom check that lockscreen object (app) is present before trying to unlock.

Why would you want to build with NO_LOCK_SCREEN though? We prefer to build with as close to production build as is reasonable and thus the tests are designed to work on that kind of build.

I think this used to work by luck but they recently changed how the lockscreen runs (as an app) and now this is failing for you.
(In reply to Zac C (:zac) from comment #18)
> 
> Why would you want to build with NO_LOCK_SCREEN though? We prefer to build
> with as close to production build as is reasonable and thus the tests are
> designed to work on that kind of build.

No disagreement in general.  The lockscreen has two problems in the past that we ran into: 
* no way to prevent it from lockscreen from appearing at boot, very annoying to both human and machine testers
* during monkey testing we prefer to generally not have the lockscreen present over having a poor monkey get stuck in the lockscreen for hours.
(Reporter)

Comment 20

4 years ago
OK yes I see your angle. When we do automated monkey testing we have a different setUp class which re-sets the device in a different way and doesn't call this method so we don't hit this problem.

For example you could just remove the `unlock` line from the setUp in your fork/branch, but then you wouldn't be able to run the lockscreen functional tests.

How do you want to resolve this, can you do it in your branch or you want us to do something?
(Reporter)

Comment 21

4 years ago
Personally I'd rather keep it as it is, I'd rather not put logic in.
When we call lock or unlock it's because we want that action to occur and we wouldn't want to miss a bug like the lockscreen being broken because a logic path tells us to skip that step.
From Moz's perspective, we can't block on this. CAF is welcome to take advantage of the tests & framework written, but Moz is not liable for fixing issues on CAF's behalf. CAF needs to assign their own engineers to resolve the problem.
blocking-b2g: 2.0+ → 2.0?
(Reporter)

Updated

4 years ago
Duplicate of this bug: 1031029
(Reporter)

Comment 24

4 years ago
Further to comment #22, setting wontfix.
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → WONTFIX

Updated

4 years ago
blocking-b2g: 2.0? → ---
This was fixed. I believe it's just the uplift to 2.0 that's not happening.
Resolution: WONTFIX → FIXED
You need to log in before you can comment on or make changes to this bug.