Closed Bug 1209419 Opened 4 years ago Closed 4 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-master verified)

VERIFIED FIXED
FxOS-S10 (30Oct)
Tracking Status
b2g-master --- verified

People

(Reporter: azasypkin, Assigned: martianwars, Mentored)

References

Details

(Whiteboard: [lang=js])

Attachments

(2 files, 1 obsolete file)

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).
See Also: → 1209421
Hey Oleg, could I work on this bug or is Kumar Rishav is working on this one as well?? Where could I start?
Flags: needinfo?(azasypkin)
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
Flags: needinfo?(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
Attached file Basic suite added (obsolete) —
Really unsure about the options button. Couldn't find similar code. Which code could I read?
Flags: needinfo?(azasypkin)
Attachment #8670818 - Flags: review?(azasypkin)
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.
Assignee: nobody → kalpeshk2011
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!
Attachment #8670818 - Attachment is obsolete: true
Flags: needinfo?(azasypkin)
Attachment #8670818 - Flags: review?(azasypkin)
Attachment #8670816 - Flags: feedback?(azasypkin)
Comment on attachment 8670816 [details] [review]
[gaia] martiansideofthemoon:my-code-fix > mozilla-b2g:master

Replied at github.
Attachment #8670816 - Flags: feedback?(azasypkin)
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.
Attachment #8670816 - Flags: feedback?(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 :)
Attachment #8670816 - Flags: feedback?(azasypkin)
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?
Attachment #8670816 - Flags: feedback?(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!
Attachment #8670816 - Flags: feedback?(azasypkin)
Depends on: 1215806
See Also: → 1216176
Replied to your last question at GitHub :)
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?
Attachment #8670816 - Flags: feedback?(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
Attachment #8670816 - Flags: feedback?(azasypkin) → feedback+
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.
Attachment #8670816 - Flags: review?(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!
Attachment #8670816 - Flags: review?(azasypkin) → review+
@azasypkin :- I guess I'll mentor the bug to add the last integration test and tag it as a "good first bug". :) What say?
Keywords: checkin-needed
(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 :)
Keywords: checkin-needed
Keywords: checkin-needed
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?
https://github.com/mozilla-b2g/gaia/commit/b93fb040d2a0981aa3a3d9e59225e9b11cf017bd
Status: NEW → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S10 (30Oct)
See Also: → 1218415
(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
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.