Closed Bug 1179586 Opened 9 years ago Closed 9 years ago

Intermittent apps/sms/test/marionette/conversation_test.js | Conversation Panel Tests General use cases Long SMS thread User can see all messages when scrolls up

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::SMS
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: philor, Assigned: azasypkin)

Details

(Keywords: intermittent-failure, Whiteboard: [sms-sprint-FxOS-S8])

Attachments

(1 file)

[gaia] azasypkin:bug-1179586-conversation-test > mozilla-b2g:master
46 bytes, text/x-github-pull-request
azasypkin
: review+
julienw
: feedback+
Details | Review 
      No description provided.
Component: Gaia → Gaia::SMS
I think I've seen something similar (timeout in "General use cases Long SMS thread User can see all messages when scrolls up." test) locally several times already, will take a look if it happens again.

Keeping ni? for now.
Flags: needinfo?(azasypkin)
I can reproduce the issue locally using my very slow Vagrant VM [1].

The issue seems in the fact that the amount of space Marionette "actions.flick()" scrolls up in low performant environment is not enough to load a new chunk of messages.

Will take a look what we can do here.

[1] https://github.com/azasypkin/gaia-vagrant
Comment on attachment 8665461 [details] [review]
[gaia] azasypkin:bug-1179586-conversation-test > mozilla-b2g:master

Hey Julien,

What do you think about this approach?

The change is that now we scroll until we see the message node we want to see or fail if it takes too much time (scroll doesn't work). It seems to work reliably in both fast (locally on laptop) and slow setup (artificially slowed down VM).

Old approach is quite fragile especially if we change style of message nodes or in case of "long" messages. 

Thanks
Flags: needinfo?(azasypkin)
Attachment #8665461 - Flags: feedback?(felash)
This bug was catching two different issues, so adjusting name to specific "Conversation Panel Tests General use cases Long SMS thread User can see all messages when scrolls up" as "Conversation Panel Tests Action links in messages Send message to unknown number" is not really happening now (I _believe_).
Summary: Intermittent apps/sms/test/marionette/conversation_test.js | Conversation Panel Tests Action links in messages Send message to contact number → Intermittent apps/sms/test/marionette/conversation_test.js | Conversation Panel Tests General use cases Long SMS thread User can see all messages when scrolls up
Assignee: nobody → azasypkin
Status: NEW → ASSIGNED
Whiteboard: [sms-sprint-FxOS-S8]
Comment on attachment 8665461 [details] [review]
[gaia] azasypkin:bug-1179586-conversation-test > mozilla-b2g:master

I never liked the old approach anyway.

I retriggered the Gij16 several times, so please wait for them and merge only if there is no failure.

r=me with this

Thanks !
Attachment #8665461 - Flags: feedback?(felash) → feedback+
Comment on attachment 8665461 [details] [review]
[gaia] azasypkin:bug-1179586-conversation-test > mozilla-b2g:master

(In reply to Julien Wajsberg [:julienw] (PTO Sept 23rd) from comment #21)
> Comment on attachment 8665461 [details] [review]
> [gaia] azasypkin:bug-1179586-conversation-test > mozilla-b2g:master
> 
> I never liked the old approach anyway.

Yeah, I remember :)

> I retriggered the Gij16 several times, so please wait for them and merge
> only if there is no failure.
> 
> r=me with this

I've re-triggered it few more times, so ~30 green tries, adding you r+ and merging.

Thanks!
Attachment #8665461 - Flags: review+
Landed.

Master: https://github.com/mozilla-b2g/gaia/commit/a53c1285d9056b38bef5ef5ec4ee899cde27deb8
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: