1209419 - [Messages][Tests] Add basic integration tests for the Conversation view Edit mode

Status: VERIFIED FIXED

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

Description

[Oleg Zasypkin [:azasypkin]]

By basic tests I mean at least the following cases:

* Check that user can enter edit mode;
* Check that user can leave edit mode;
* Check Select All / Unselect All functionality;
* Check that user can delete selected messages (including _all_ message).

Comment 1

[Kalpesh Krishna [:martianwars]]

Hey Oleg, could I work on this bug or is Kumar Rishav is working on this one as well?? Where could I start?

Comment 2

[kumar rishav (:rishav_)]

hey Kalpesh, 
It's fine, you can work on it :)
Read the comment of : https://bugzilla.mozilla.org/show_bug.cgi?id=1209421

You can reach out at irc channel too : #fxos, #gaia-messaging

Thanks

Comment 3

[Oleg Zasypkin [:azasypkin]]

Thanks Rishav!

Hey Kalpesh,

Yes what Rishav said and you've worked with this already :)

Regarding this specific bug, I'd start with the following: add a new _very_ basic test in [1] that just enters into "edit mode" (open messages app -> tap on conversation -> tap on options "..." button -> choose "Select messages"). You can see [1] for very similar examples.

Once you done this the rest will be much-much easier :) So please try/ask/ping!

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/test/marionette/conversation_test.js

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master

Comment 5

[Kalpesh Krishna [:martianwars]]

Attachment: Basic suite added

Really unsure about the options button. Couldn't find similar code. Which code could I read?

Comment 6

[kumar rishav (:rishav_)]

Hi Kalpesh,

Thanks for the patch. You have added suite so far, now you have to work on it't test.
 
Few things i want to tell (about the patch review flow) :

First ask for feedback? (for feedback on patch) or needinfo? (if need some info or help related to patch). when you are completely sure (like you are done with patch and got feedback+), then go for review?.

Thanks

Assigning to Kalpesh as Patch submitted.

Comment 7

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8670818 [details] [review]
Basic suite added

Added comprehensive clarifying comment at GitHub, so that you can move forward :)

I'm removing this attachment as Autolander created the same for you already (attachment 8670816 [details] [review]).

Thanks!

Comment 8

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8670816 [details] [review]
[gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master

Replied at github.

Comment 9

[Kalpesh Krishna [:martianwars]]

Comment on attachment 8670816 [details] [review]
[gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master

I succeeded in entering and exiting edit mode :) Not very sure about which asserts to use now.

Comment 10

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8670816 [details] [review]
[gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master

(In reply to Kalpesh Krishna from comment #9)
> Comment on attachment 8670816 [details] [review]
> [gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master
> 
> I succeeded in entering and exiting edit mode :) Not very sure about which
> asserts to use now.

Great! Replied at GitHub :)

Comment 11

[Kalpesh Krishna [:martianwars]]

Comment on attachment 8670816 [details] [review]
[gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master

Integration test works as expected :) What would be the next steps?

Comment 12

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8670816 [details] [review]
[gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master

You're moving faster! Added some comments and next steps at GitHub.

Thanks!

Comment 13

[Oleg Zasypkin [:azasypkin]]

Replied to your last question at GitHub :)

Comment 14

[Kalpesh Krishna [:martianwars]]

Comment on attachment 8670816 [details] [review]
[gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master

@azasypkin Well you pretty much told me the entire test so didn't have any issues :smile: What do I have to do next?

Comment 15

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8670816 [details] [review]
[gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master

Looks good now, we're almost there! I've left mostly cleanup comments, and suggestion for few more simple tests.

Next time you're ready, please rebase your PR on the latest master and squash all your commits into one [1]. From now on you can ask for r? instead of feedback, since you have f+ :)

Thanks!

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Developing_Gaia/Submitting_a_Gaia_patch#Tips_on_Gaia_Rebasing

Comment 16

[Kalpesh Krishna [:martianwars]]

Comment on attachment 8670816 [details] [review]
[gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master

Tests working perfectly on my laptop :) Do let me know about the further nits.

Comment 17

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8670816 [details] [review]
[gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master

Perfect! I've left just a few minor nits. So please:

* fix them :)
* then rebase on the latest master (looks like something weird is happening on Treeherder, everything is failing);
* wait for green Treeherder and then add "checkin-needed" keyword.

So the only thing that is left from what I mentioned in comment 0 is
> * Check that user can delete selected messages (including _all_ message).

If you want to handle this in your PR (that would be great :)), just go ahead and ask r? once again, but I'm totally fine you prefer to land this PR as is and file a follow-up that you or someone else can handle afterwards.

Thanks a lot for your work!

Comment 18

[Kalpesh Krishna [:martianwars]]

@azasypkin :- I guess I'll mentor the bug to add the last integration test and tag it as a "good first bug". :) What say?

Comment 19

[Oleg Zasypkin [:azasypkin]]

(In reply to Kalpesh Krishna from comment #18)
> @azasypkin :- I guess I'll mentor the bug to add the last integration test
> and tag it as a "good first bug". :) What say?

Totally! Could you please file a follow-up then?

I'm removing checkin-needed for now, so that you have chance to leave comment about workaround of "element.scrollIntoView" issue (commented at github). Please, add it back once you leave that comment :)

Comment 20

[Kalpesh Krishna [:martianwars]]

I have added a bug for the same https://bugzilla.mozilla.org/show_bug.cgi?id=1218397

Comment 21

[Kalpesh Krishna [:martianwars]]

I'll add the follow up bug once this bug is merged so that I can make references to the code :)
@azasypkin :- It's fun working with you! What would be a good bug to work on next?

Comment 22

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/b93fb040d2a0981aa3a3d9e59225e9b11cf017bd

Comment 23

[Oleg Zasypkin [:azasypkin]]

(In reply to Kalpesh Krishna from comment #21)
> I'll add the follow up bug once this bug is merged so that I can make
> references to the code :)
> @azasypkin :- It's fun working with you! What would be a good bug to work on
> next?

You can pick one you like more using the link below :)

https://bugzilla.mozilla.org/buglist.cgi?o2=isnotempty&j17=OR&f10=OP&f19=CP&f1=OP&o3=equals&f8=CP&f0=OP&v3=nobody%40mozilla.org&f18=CP&f9=CP&query_format=advanced&f3=assigned_to&f2=bug_mentor&f11=OP&bug_status=NEW&component=Gaia%3A%3ASMS&product=Firefox%20OS&list_id=12639116

Comment 24

[zouyi(Leave from Mozilla)]

Attachment: Verified video: Aries_v2.5.3GP

Hey,Oleg! This bug has been verified as pass on latest build of Flame KK v2.5.
STR:
Precondition： There have some messages on device.
1. Launch Messages.
2. Try to enter the edit mode.
3. Try to use Slecte function.
4. Try to select All / Unselect All functionality.
5. Try to delete selected messages (including _all_ message). 
6. Try to mark is a text message has been read or not read.
7. Repeat step2~7serevel times.
Actully results:All of the function is OK.

See attachment:Aries_v2.53GP.

Reproduce rate:0/10

Device:Flame KK v2.5[pass]
Build ID               20151028150209
Gaia Revision          2e89362de40a6c9c36525d36317fa1ae8e67e143
Gaia Date              2015-10-28 04:56:28
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/fc706d376f0658e560a59c3dd520437b18e8c4a4
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151028.182746
Firmware Date          Wed Oct 28 18:27:57 EDT 2015
Firmware version V18D V4
Bootloader             L1TC000118D0


Device:AriesKK v2.5[pass]
Build ID               20151029010303
Gaia Revision          75f2236d36cc9f9c02d3596fae3de014007cfd82
Gaia Date              2015-10-28 17:02:16
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/1e700005a0ddf2b17803213e1f3f8d78a7a618b8
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151029.002011
Firmware Date          Thu Oct 29 00:20:19 UTC 2015
Bootloader             s1