1021767 - Remove switch_to_displayed_app from _set_pref / _get_pref

Status: RESOLVED FIXED

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

Description

[Zac C (:zac)]

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.

Comment 1

[Zac C (:zac)]

Attachment: github pr

Comment 2

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

Zac: Is this ready to land or is there anything you need from me?

Comment 3

[Zac C (:zac)]

It's ready to land!

Comment 4

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

Thanks. Landed in:
https://github.com/mozilla-b2g/gaia/commit/72b4922f0176f2342b534d90515df37f7976ceac

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e865305e4b35

Comment 7

[Jason Smith [:jsmith]]

Zac - Can you get this uplifted to 2.0 to unblock CAF?

Comment 8

[Zac C (:zac)]

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

Test-only commit

Comment 9

[Zac C (:zac)]

Was already doing it :)

Comment 10

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(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 .

Comment 11

[Zac C (:zac)]

(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.

Comment 12

[Zac C (:zac)]

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.

Comment 13

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(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

Comment 14

[Jason Smith [:jsmith]]

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.

Comment 15

[Zac C (:zac)]

(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?

Comment 16

[Zac C (:zac)]

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.

Comment 17

[Michael Vines [:m1] [:evilmachines]]

(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?

Comment 18

[Zac C (:zac)]

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.

Comment 19

[Michael Vines [:m1] [:evilmachines]]

(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.

Comment 20

[Zac C (:zac)]

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?

Comment 21

[Zac C (:zac)]

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.

Comment 22

[Jason Smith [:jsmith]]

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.

Comment 24

[Zac C (:zac)]

Further to comment #22, setting wontfix.

Comment 25

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

This was fixed. I believe it's just the uplift to 2.0 that's not happening.