927139 - Expand automated coverage around keyboard usage when composing a MMS message

Status: RESOLVED FIXED

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

Description

[Jason Smith [:jsmith]]

A big hotspot for regressions is around keyboard usage when composing a MMS message. We should look into expanding automated coverage here that ensures that the keyboard is in correct state when composing a MMS message - active when you are inputting text, not randomly disappearing, disappear after inputting text, etc.

Comment 1

[Zac C (:zac)]

Jason, I've tried to replicate something like this locally.

One example I can see is that if you have a contact called 'test' and in the "To:" field of the message composition, type the name of the contact and push enter key. The contact is 'saved' or locked in and the keyboard is hidden despite the field still having focus.

Is this what you mean?

Aside this is a bit vague and could be a bit of an edge case thing.

Comment 2

[Jason Smith [:jsmith]]

The regressions motivating the filing of this bug are:

- bug 909821 (not caught by automation)
- bug 914100 (not caught by automation)
- bug 917048 (not caught by automation)
- bug 923262 (not caught by automation)
- bug 924581 (not caught by automation)

All of the bugs deal with the state of the keyboard on the compose message UI. What we need to do here is expand coverage to ensure that the keyboard is up when we expect it in compose message workflows & not up when we don't expect it.

Comment 3

[Stephen Donner [:stephend] Not actively reading bugmail]

Walter/Rudy, it seems there more opportunities here to get more coverage -- can you take a look?  Thanks!

Comment 4

[Zac C (:zac)]

These look more like enhancements to existing test cases rather than new test cases. The basic test cases are mostly already automated but we just need to enhance them to be sensitive/assert upon the state of the keyboard at each point in the test case.

Comment 5

[Zac C (:zac)]

Zac, can you write a more detailed test case for whoever is to take this task? Thanks.

Comment 6

[Zac C (:zac)]

I think this is a bit too edge case now and I am not sure it's a good candidate for automation.

I think it would be better for the manual team who can look for this kind of bad ux behaviour in all apps, not just Messages.

Comment 7

[Jason Smith [:jsmith]]

(In reply to Zac C (:zac) from comment #6)
> I think this is a bit too edge case now and I am not sure it's a good
> candidate for automation.

I disagree with this. We've had way too many regressions around the keyboard failing to show up when it should & vice versa. There's already evidence in comment 2 that show this is prominent. It isn't an edge case - this is normal keyboard usage.

> 
> I think it would be better for the manual team who can look for this kind of
> bad ux behaviour in all apps, not just Messages.

We do look out for this, but this is a common sore spot for regressions.

Comment 8

[Zac C (:zac)]

I couldn't replicate that comment #2 failure case anymore :(

Does it happen in other apps too? do we want this kind of smoke test for several apps?

Comment 9

[Tony Chung [:tchung]]

(In reply to Jason Smith [:jsmith] from comment #7)
> (In reply to Zac C (:zac) from comment #6)
> > I think this is a bit too edge case now and I am not sure it's a good
> > candidate for automation.
> 
> I disagree with this. We've had way too many regressions around the keyboard
> failing to show up when it should & vice versa. There's already evidence in
> comment 2 that show this is prominent. It isn't an edge case - this is
> normal keyboard usage.
> 
> > 
> > I think it would be better for the manual team who can look for this kind of
> > bad ux behaviour in all apps, not just Messages.
> 
> We do look out for this, but this is a common sore spot for regressions.

I think in cases like this where it could look edge-casey on paper, but there is a request for adding more test coverage, its worth having the one-off discussions for justifying it.  in this case, i agree with Jason, we are seeing multiple regressions on a highly visible feature like keyboard and messaging, so it would be good to take cases like this and add it to the queue.   

but in the future, having a discussion around "gray area" testareas could be good for improving process.

Does anyone disagree?

Comment 10

[Jason Smith [:jsmith]]

That sounds fine to me.

Comment 11

[Zac C (:zac)]

OK I see, I'm happy to do this type of test for our highest priority apps but for all of them could be a bit messy.
Let's start with messaging.

Comment 12

[Jason Smith [:jsmith]]

(In reply to Zac C (:zac) from comment #11)
> OK I see, I'm happy to do this type of test for our highest priority apps
> but for all of them could be a bit messy.
> Let's start with messaging.

Right. The bug's scope here was to focus purely on the SMS app.

Comment 13

[Bob Silverberg [:bsilverberg]]

Can we get a description of a test case for this? I've read through the bug and I'm not really sure exactly what a test would look like.

Comment 14

[Jason Smith [:jsmith]]

William - Can you look into constructing some formal test cases around comment 2's regression blockers for the keyboard? Once you create them, can you reference them here to allow the automation team to have exact STR to use to write automated tests against?

Comment 15

[Julien Wajsberg [:julienw]]

see also bug 934449 (worked around in SMS, but the real keyboard bug is in bug 937487)

Comment 16

[Julien Wajsberg [:julienw]]

see also bug 912414

Comment 17

[William Hsu [:whsu]]

(In reply to Jason Smith [:jsmith] from comment #2)
> The regressions motivating the filing of this bug are:
> 
> - bug 909821 (not caught by automation)
    Test Case: #1293 (https://moztrap.mozilla.org/manage/case/1293/)

> - bug 914100 (not caught by automation)
    Test Case: #10925 (https://moztrap.mozilla.org/manage/case/10925/)

> - bug 917048 (not caught by automation)
    Test Case: #10926 (https://moztrap.mozilla.org/manage/case/10926/)

> - bug 923262 (not caught by automation)
    Test Case: #10926 (https://moztrap.mozilla.org/manage/case/10926/)
    * A duplicate bug of bug 917048

> - bug 924581 (not caught by automation)
    Test Case: #10928 (https://moztrap.mozilla.org/manage/case/10928/)

Here are the test cases you need.

By the way, you can add a flag "in‑moztrap=?"
I believe that someone will take it.
Thanks. Have a nice day!

Comment 18

[William Hsu [:whsu]]

Any update?
Thanks.

Comment 19

[Zac C (:zac)]

(In reply to William Hsu [:whsu] from comment #18)
> Any update?
> Thanks.

This is not really QA smoketest and quite edge-case, they're not good candidates for on-device testing.

They are good candidates for desktopb2g when the new mock ril is present.

I'll push this back to the SMS component to these tests can be written with marionettejs.

Comment 20

[Jason Smith [:jsmith]]

(In reply to Zac C (:zac) from comment #19)
> (In reply to William Hsu [:whsu] from comment #18)
> > Any update?
> > Thanks.
> 
> This is not really QA smoketest and quite edge-case, they're not good
> candidates for on-device testing.

We've already been through this discussion & I've already clarified that statement is incorrect. Keyboard is a considered a foundational piece of the phone. If it breaks, then multiple smoketests will break with it. This is considered a significant hotspot regressions, so we do need tests here.

> 
> They are good candidates for desktopb2g when the new mock ril is present.

No. As long as OOP doesn't exist on desktop b2g, there is low value to implementing this in MarionetteJS. The best solution at the current time is to implement on device tests, as OOP is supported there.

Comment 21

[Julien Wajsberg [:julienw]]

Doesn't marionette js works with the device as well? Or is it just not launched on TBPL?

Comment 22

[Zac C (:zac)]

(In reply to Julien Wajsberg [:julienw] from comment #21)
> Doesn't marionette js works with the device as well? Or is it just not
> launched on TBPL?

In theory but nobody has stood it up against the nightly builds, afaik.
It's definitely not used on tbpl.

Comment 23

[Julien Wajsberg [:julienw]]

FYI I definitely run the test performance framework (that uses the same client) on the device so this is doable using the env variable: MARIONETTE_RUNNER_HOST=marionette-device-host

Also, OOP is probably soon implemented in desktop b2g (bug 914584).

Comment 24

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to Julien Wajsberg [:julienw] from comment #23)
> FYI I definitely run the test performance framework (that uses the same
> client) on the device so this is doable using the env variable:
> MARIONETTE_RUNNER_HOST=marionette-device-host
> 
> Also, OOP is probably soon implemented in desktop b2g (bug 914584).

Yes, and bug 990635 is well underway as well.

Comment 25

[Florin Strugariu [:Bebe]]

I will write/update tests to make sure the mentioned test cases are automated

Comment 26

[Florin Strugariu [:Bebe]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24952

Comment 27

[Robert Chira [:RobertC]]

Comment on attachment 8501821 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24952

r+, but please address the comment from the PR

Comment 28

[Viorela Ioia [:viorela]]

Comment on attachment 8501821 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24952

r+!

Comment 29

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 8501821 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24952

lgtm pending pr comment

Comment 30

[Florin Strugariu [:Bebe]]

Udated

Comment 31

[Viorela Ioia [:viorela]]

Merged in: https://github.com/mozilla-b2g/gaia/commit/cd20ed48520ba35315c51f37c4a6803306fe93da