Expand automated coverage around keyboard usage when composing a MMS message

RESOLVED FIXED

Status

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jsmith, Assigned: Bebe)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Reporter

Description

6 years ago
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

6 years ago
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.
Reporter

Comment 2

6 years ago
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.
Walter/Rudy, it seems there more opportunities here to get more coverage -- can you take a look?  Thanks!

Comment 4

6 years ago
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

6 years ago
Zac, can you write a more detailed test case for whoever is to take this task? Thanks.
Flags: needinfo?(zcampbell)

Updated

6 years ago
Assignee: nobody → andrei.hutusoru

Comment 6

6 years ago
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.
Assignee: andrei.hutusoru → nobody
Flags: needinfo?(zcampbell)
Reporter

Comment 7

6 years ago
(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

6 years ago
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?
(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?
Reporter

Comment 10

6 years ago
That sounds fine to me.

Comment 11

6 years ago
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.
Reporter

Comment 12

6 years ago
(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.
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.
Reporter

Comment 14

6 years ago
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?
Flags: needinfo?(whsu)
see also bug 934449 (worked around in SMS, but the real keyboard bug is in bug 937487)
(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!
Flags: needinfo?(whsu)
Any update?
Thanks.

Comment 19

5 years ago
(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.
Component: Gaia::UI Tests → Gaia::SMS
Reporter

Comment 20

5 years ago
(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.
Component: Gaia::SMS → Gaia::UI Tests
Doesn't marionette js works with the device as well? Or is it just not launched on TBPL?

Comment 22

5 years ago
(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.
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).
(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.
I will write/update tests to make sure the mentioned test cases are automated
Assignee: nobody → florin.strugariu
Attachment #8501821 - Flags: review?(viorela.ioia)
Attachment #8501821 - Flags: review?(robert.chira)
Attachment #8501821 - Flags: review?(gmealer)
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
Attachment #8501821 - Flags: review?(robert.chira) → review+
Comment on attachment 8501821 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24952

r+!
Attachment #8501821 - Flags: review?(viorela.ioia) → review+
Comment on attachment 8501821 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24952

lgtm pending pr comment
Attachment #8501821 - Flags: review?(gmealer) → review+
Udated
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
QA Whiteboard: [fxosqa-auto-s1]
You need to log in before you can comment on or make changes to this bug.