945544 - [e-mail][user story] Support Empty Trash for POP3

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

As a user I would like to be able to empty the contents of the trash folder for POP3 accounts without having to manually select all of the messages and re-delete them.

General Context: Deleted messages are moved to the trash folder.  When they are deleted from the trash-folder, they are purged into oblivion.  However, because we leave all POP3 messages on the server a simplifying implementation assumption (and to avoid data-loss), if the user deletes the account and re-creates it, the messages will be eligible to be downloaded again.

The UX wireframe links I have been provided are these:
===
Link to the wires on Box:
https://app.box.com/files/0/f/1272831540/1/f_11985783354

Link on LayerVault:
https://layervault.com/mozilla/Productivity%201.3/Email_1.3_131121.pdf/2
===

The box link appears to require Mozilla credentials, but layervault worked for me.  There may also be a new revision on layervault.

The summary of the implementation specification is:

- For the POP3 trash folder, have edit mode display an "Empty Trash" text button in the (upper) right of the edit header.

- If the user presses the button, display a labeled confirmation prompt with actions "Cancel" and "Delete".  The example string provided is "Delete all email messages?"  I am going to make the editorial proposal to extend this to "Delete all email messages in the trash folder?" since we have a lot of available screen real estate and it reduces ambiguity.


I am going to try and sanity check existing l10n strings for Thunderbird for "Empty Trash" in conjunction with our existing "Edit" strings to make sure that the UI still works in those cases.  I am currently planning to use Turkish and German since those are languages where we have previously been burnt.


:fabrice/triage-friends, this is a 1.3 targeted late-ish feature that we determined as a necessary follow-on landing for our POP3 implementation that has landed in v1.3.  We believe this would end up a 1.3 blocker eventually if we don't address this now.


I will be implementing and landing this as my highest priority.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

The amazing transvision tool at http://transvision-beta.mozfr.org says:

* Empty Trash in German from TB: Papierkorb leeren
http://transvision-beta.mozfr.org/?repo=central&sourcelocale=en-US&locale=de&search_type=entities&recherche=mail%2Fchrome%2Fmessenger%2Fmessenger.dtd%3AemptyTrashCmd.label

* Empty trash in Turkish from TB: Çöpü boşalt
http://transvision-beta.mozfr.org/?repo=central&sourcelocale=en-US&locale=tr&search_type=entities&recherche=mail%2Fchrome%2Fmessenger%2Fmessenger.dtd%3AemptyTrashCmd.label

* Empty trash in Hungarian from TB: Törölt elemek mappa ürítése
http://transvision-beta.mozfr.org/?repo=central&sourcelocale=en-US&locale=hu&search_type=entities&recherche=mail%2Fchrome%2Fmessenger%2Fmessenger.dtd%3AemptyTrashCmd.label

The Hungarian translates back to "Emptying the Deleted Items folder" from Google Translate.  Google translate has no good ideas on how to shorten this.


While looking into existing translations of 'edit' I realized that I had forgotten that we are doing a plural numbers hack for the edit header.  When there are zero messages in the header, we say "Edit", but if any messages are selected, we say "{{ n }} selected".

Translations of selected seem to be mercifully shorter; of the 3 above, German seems to be the longest and gives us "{{ n }} ausgewählt".  French is longer at "{{ n }} sélectionnés". (And its empty trash is "Vider la corbeille")


Because of cases like Hungarian and the potential fight with our existing "{{ n }} selected", I'm going to propose a rev to the UX and suggest we run with displaying an inline blurb like we have for "load more messages", but we do this at the top of the message list.  The option would collapse if the folder is empty or we enter edit mode.  (And would always be collapsed for non-POP3 accounts.)

I'm going to tentatively prepare the patch assuming this course of action.  Apart from the HTML, the implementation will basically look the same so I'll try and prepare a screenshot for that too.

:robmac, I am needinfo-ing you as a proxy for Esther since I cannot find a bugzilla account for her.  I will point her at this comment, but want it to be clear that there's an outstanding UX question.

Comment 2

[Tony Chung [:tchung]]

Discussed in Productivity Team meeting today.   Nominating for blocking=1.3, as without this feature, pop3 trashed emails cant delete, fill up the internal storage, and possibly OOM or brick the device.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Attachment: GELAM implementation with test

There were more testing complications with POP3 than I was hoping for.  These are due to some of the testing hacks we landed to support POP3 passing a bunch of IMAP and ActiveSync's tests despite some semantic incompatibility.  I filed bug 945669 on trying to clear up some of the uglier stuff, including the unhappy problem where opening the trash folder can cause us to try and establish a connection, potentially interfering with Inbox sync, etc.

I need to crash now, but next up is completing the gaia implementation and writing a JS marionette test.  (We might also be able to get away with a unit test for sufficient coverage, but it might be nice to have the higher level marionette coverage.  I'll think on this more, but either way there is enough complexity in deciding whether to show the empty trash button/bar that we do want a UI test.)

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Screenshot of Hungarian "Empty Trash" string using Andrew's proposed solution to deal with huge strings

As noted in the bug, the Hungarian string for 'empty trash' is realllly huge.  By showing a scrollable bar/button at the top of the message list in the POP3 trash folder, we manage to fit the string in.  We actually even have a little space to spare.

To be very honest, I marked the button with 'danger' so it was red simply because I am reusing the delete icon and the white icon on the grey button doesn't work.  But it is arguably appropriate.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Screenshot of en-US "Empty Trash" string using Andrew's proposed solution to deal with huge strings

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

Attachment: empty trash confirmation page screenshot, ZTE Open

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

Attachment: The folder after emptying the trash; just the empty folder string, no button.

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8341663 [details] [review]
GELAM implementation with test

Back-end should be good to go for review now.  The only additional change from when I put this up is that I added 'accountType' to the MailFolder instances so that we can identify the POP3 trash folder without doing any complicated inferences, etc.  Also it's rebased to tip; thanks much for fixing that sliceOpenMostRecent thing so I could avoid an XXX about triggering sync! :)

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Attachment: abandoned GAIA PR for ugly red button

Here's the Gaia pull request.  It has the gaia changes and the install GELAM changes in separate commits for ease of reviewing / testing.

Thing to do still:

- Write the JS Marionette test.  I was conflicted about making this a gaia unit test, but we don't have any coverage of deletion right now in the marionette tests, so I think it's worth handling this using marionette coverage.

- Reach consensus with Esther on the button in the list versus the edit bar header.  I pinged Esther and she will look at this tonight.  An initial suggestion is to just use "Empty" for the edit menu, although unfortunately I'm not qualified to directly determine whether Hungarian could provide a shorter translation for that with appropriate semantics/context.  I'll try and see if I can find a comparable string context in Thunderbird for that...

Comment 10

[Marcus Cavanaugh [:mcav]]

Comment on attachment 8341663 [details] [review]
GELAM implementation with test

Looks flawless to me. I haven't tested on-device yet, but I'll do so shortly and "no news is good news" type of thing. Test case looks really thorough, I like it. Deleted messages have no chance of survival against it.

Comment 11

[Andrew Sutherland [:asuth] (he/him)]

Attachment: GAIA PR for trash button in folder picker / folder list

After discussion with Esther we've lost the horrible red button in favor of the previous next-rev goal of putting the empty-trash button in the folder picker.  We are reusing the existing delete icon.

Code-wise this is much cleaner.  The only real sadness with this is that we can't easily grey out the button when there are no messages in the trash folder.  That requires MailFolder to have (live-updating) statistics that it does not possess and which are unfortunately too complex to pursue for a late-1.3 feature.

I'll upload some screenshots of this, then finish the marionette test.  Will request review once the marionette test is good.

Comment 12

[Andrew Sutherland [:asuth] (he/him)]

Attachment: folder-list-inbox-selected.png

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

Attachment: folder-list-trash-active.png

Comment 14

[Andrew Sutherland [:asuth] (he/him)]

Attachment: folder-list-empty-trash-active.png

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

Attachment: folder-list-trash-is-current-folder.png

Comment 16

[Andrew Sutherland [:asuth] (he/him)]

Forgot an outstanding implementation question.  I'm waiting to hear back from Esther about showing a status message that we emptied the trash.  Our current visual presentation assumes we are in the folder list when we show such a message.  The grey on grey drawer may look dumb if we show it without returning to the message list, especially since it will partially fall on the edge of the message list visible because of the tray mechanism.  If we just return to the message list (of whatever the current folder was) when we empty, we can avoid that.

Comment 17

[Andrew Sutherland [:asuth] (he/him)]

The implementation question was resolved.  We will transition to the (now empty) trash folder after confirming the desire to empty the trash.  I've updated the implementation to do this.

The marionette test is still in progress.  In a good news/bad news situation, I've cleaned up a lot of things while adding the required functionality for appropriate test coverage of the empty trash mechanism, it's just taken a bit more work to propagate the sanity and document what the methods are doing.  I'm expecting the final patch to go up for review much later today.

Comment 18

[Tony Chung [:tchung]]

in-moztrap? to Whsu to include some testcases.

Comment 19

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Tony Chung [:tchung] from comment #18)
> in-moztrap? to Whsu to include some testcases.

We're actually going to have what amounts to full coverage of empty trash between the GELAM back-end tests and the front-end JS Marionette tests, so we won't need any moztrap cases.  What do we set in-moztrap to for that?

Comment 20

[Tony Chung [:tchung]]

(In reply to Andrew Sutherland (:asuth) from comment #19)
> (In reply to Tony Chung [:tchung] from comment #18)
> > in-moztrap? to Whsu to include some testcases.
> 
> We're actually going to have what amounts to full coverage of empty trash
> between the GELAM back-end tests and the front-end JS Marionette tests, so
> we won't need any moztrap cases.  What do we set in-moztrap to for that?

while im happy to hear that, there's a sense of testing on device (different screen sizes, resolution), that human eyes can catch.  We'll likely be adding Gaia python tests on device tests as usual.

I'm asking for a set of minimum trash tests that would get executed during the end to end test runs.  Thats really what in-moztrap? tries to catch when needed; and of course the other bugs that "cannot" be easily automated.

Comment 21

[Dylan Oliver [:doliver]]

At Productivity team meeting today we agreed to hold this feature until v1.4.

Comment 22

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Tony Chung [:tchung] from comment #20)
> while im happy to hear that, there's a sense of testing on device (different
> screen sizes, resolution), that human eyes can catch.  We'll likely be
> adding Gaia python tests on device tests as usual.
> 
> I'm asking for a set of minimum trash tests that would get executed during
> the end to end test runs.  Thats really what in-moztrap? tries to catch when
> needed; and of course the other bugs that "cannot" be easily automated.

Yes, having a human run a simple empty-trash test and verifying there is no visual breakage seems suitable, although once we run perceptual diffs on screenshots of the test process, we can likely reduce this further to just having a human check when things change on a number of screen resolutions based on shipping devices or for occasional total shake-down smoke-tests.

My concern is about creating manually run test-cases that are better done in an automated fashion and *are* covered by existing automated tests.  Especially the test cases that seem particularly labor-intensive and where false failures have and will occur as a result of human tester error.  Based on bugs filed thus far, there is a non-trivial time and frustration cost for us to triage bugs where the filer fails to provide a logcat as requested in the component filing notes and then when the log is provided it is clear from the log that they made a typo, did not understand what a text/plain message was, etc.  Moztrap cases that were created against UX wire-frames that have no relationship with what gets implemented are a particularly frustrating variant.


Eschewing generalities, the currently proposed list of moztrap cases for empty trash that I was pointed at on a non-public thread are at http://goo.gl/vxeTnq and many of them don't seem like they are contributing to the quality of the software but seem like they are going to waste the time of the people who run the tests and the people who triage the bugs.

== I think the following should be removed:

* Select all / deselect are not implemented and will not be implemented and so cases that explicitly reference them as part of the goal of the test are moot because they will never pass:
https://moztrap.mozilla.org/manage/case/11085/
https://moztrap.mozilla.org/manage/case/11084/
https://moztrap.mozilla.org/manage/case/11081/
https://moztrap.mozilla.org/manage/case/11080/
https://moztrap.mozilla.org/manage/case/11079/
https://moztrap.mozilla.org/manage/case/11078/ (similar to 11021)
https://moztrap.mozilla.org/manage/case/11027/ (note: IMAP case; needs no revised case since we don't support empty trash for IMAP.)
https://moztrap.mozilla.org/manage/case/11024/

* Tests deleting various numbers of messages are labor intensive, the dominion of the back-end tests, and also the way POP3 trash emptying is implemented and will always be implemented has no scaling issues:
https://moztrap.mozilla.org/manage/case/11083/
https://moztrap.mozilla.org/manage/case/11082/

* Testing trash while offline.  All POP3 operations are offline and the back-end unit tests ensure and verify that we don't try and establish or otherwise use a back-end connection.
https://moztrap.mozilla.org/manage/case/11025/

* Verifying deleted mails get deleted.  This test seems to depend on a UI bug that lets us enter the search mode when the folder is empty and we definitely don't want to codify that. But in general, the back-end tests very extensively check to make sure that deletion works, and the JS Marionette tests already cover this too.
https://moztrap.mozilla.org/manage/case/11023/

* Greying out the icons.  This is largely covered by the automated test, but I think it makes sense to just fold the check for the state of the icons into the 11020 case.

== I think we want to keep / have merged:

* Cancellation works (This is covered by an automated test but is fast and easy to test if you're already running any manual test, so why not keep it).  Although maybe we could just fold this as a step into 11020 too since it doesn't really require any independent setup?
https://moztrap.mozilla.org/manage/case/11021/

* General trash emptying.  Note that this will need to be revised to account for the UX change that we automatically switch to the trash folder.
https://moztrap.mozilla.org/manage/case/11020/

Comment 23

[William Hsu [:whsu]]

Hi, Tony,

I have created the test cases. But the feature is postponed to V1.4.
Many thanks!

Comment 24

[William Hsu [:whsu]]

(In reply to Andrew Sutherland (:asuth) from comment #22)
> (In reply to Tony Chung [:tchung] from comment #20)
> > while im happy to hear that, there's a sense of testing on device (different
> > screen sizes, resolution), that human eyes can catch.  We'll likely be
> > adding Gaia python tests on device tests as usual.
> > 
> > I'm asking for a set of minimum trash tests that would get executed during
> > the end to end test runs.  Thats really what in-moztrap? tries to catch when
> > needed; and of course the other bugs that "cannot" be easily automated.
> 
> Yes, having a human run a simple empty-trash test and verifying there is no
> visual breakage seems suitable, although once we run perceptual diffs on
> screenshots of the test process, we can likely reduce this further to just
> having a human check when things change on a number of screen resolutions
> based on shipping devices or for occasional total shake-down smoke-tests.
> 
> My concern is about creating manually run test-cases that are better done in
> an automated fashion and *are* covered by existing automated tests. 
> Especially the test cases that seem particularly labor-intensive and where
> false failures have and will occur as a result of human tester error. Based

If we don't do the force error testing, how could you know the user's feeling
while they encounter the same problem?
Please keep in mind. All the manual test can do that mean the FxOS user may
suffer the bugs there. So, QA needs to test the cases on the first run then
remove it if we don't need to deliver these test cases to QAnalysis.

> on bugs filed thus far, there is a non-trivial time and frustration cost for
> us to triage bugs where the filer fails to provide a logcat as requested in
> the component filing notes and then when the log is provided it is clear
> from the log that they made a typo, did not understand what a text/plain

So, why not raise the problem to stakeholders instead of discussing
the problem in private?

> message was, etc.  Moztrap cases that were created against UX wire-frames
> that have no relationship with what gets implemented are a particularly
> frustrating variant.

The other question is if QA doesn't follow the UX wire-frames to design
test cases, why do we need UX document?
 
> Eschewing generalities, the currently proposed list of moztrap cases for
> empty trash that I was pointed at on a non-public thread are at
> http://goo.gl/vxeTnq and many of them don't seem like they are contributing
> to the quality of the software but seem like they are going to waste the
> time of the people who run the tests and the people who triage the bugs.
> == I think the following should be removed:
> 
> * Select all / deselect are not implemented and will not be implemented and
> so cases that explicitly reference them as part of the goal of the test are
> moot because they will never pass:
> https://moztrap.mozilla.org/manage/case/11085/
> https://moztrap.mozilla.org/manage/case/11084/
> https://moztrap.mozilla.org/manage/case/11081/
> https://moztrap.mozilla.org/manage/case/11080/
> https://moztrap.mozilla.org/manage/case/11079/
> https://moztrap.mozilla.org/manage/case/11078/ (similar to 11021)
> https://moztrap.mozilla.org/manage/case/11027/ (note: IMAP case; needs no
> revised case since we don't support empty trash for IMAP.)
> https://moztrap.mozilla.org/manage/case/11024/

These test cases seem out-of-date. This is because 
(1) I got the final specification from Esther and the UX document mentioned
    the "Select All" button
(2) Esther said we hit some issues with localization and couldn't make the
    string "Empty Trash" work.
(3) I cannot get the final implementation from feature owner/developer while
    I designed test cases.
 
> * Tests deleting various numbers of messages are labor intensive, the
> dominion of the back-end tests, and also the way POP3 trash emptying is
> implemented and will always be implemented has no scaling issues:
> https://moztrap.mozilla.org/manage/case/11083/
> https://moztrap.mozilla.org/manage/case/11082/

But, do you know how long will the email client spend?
This relates to performance. I created the two test cases here just because
I need QAnalysis to pay attention on performance although I don't specifically
describe it.

> * Testing trash while offline.  All POP3 operations are offline and the
> back-end unit tests ensure and verify that we don't try and establish or
> otherwise use a back-end connection.
> https://moztrap.mozilla.org/manage/case/11025/
> 
> * Verifying deleted mails get deleted.  This test seems to depend on a UI
> bug that lets us enter the search mode when the folder is empty and we
> definitely don't want to codify that. But in general, the back-end tests
> very extensively check to make sure that deletion works, and the JS
> Marionette tests already cover this too.
> https://moztrap.mozilla.org/manage/case/11023/
> 
> * Greying out the icons.  This is largely covered by the automated test, but
> I think it makes sense to just fold the check for the state of the icons
> into the 11020 case.
> 
> == I think we want to keep / have merged:
> 
> * Cancellation works (This is covered by an automated test but is fast and
> easy to test if you're already running any manual test, so why not keep it).
> Although maybe we could just fold this as a step into 11020 too since it
> doesn't really require any independent setup?
> https://moztrap.mozilla.org/manage/case/11021/
> 
> * General trash emptying.  Note that this will need to be revised to account
> for the UX change that we automatically switch to the trash folder.
> https://moztrap.mozilla.org/manage/case/11020/

By the way, did there have any official automation test case review?
If so, please also share it to B2G-QA. We are glad to see that.
If not, how do you make sure the automation test of email app is robust?
Could you please share the test 
after you automate the test cases?

The other thing I want to mentioned here is the test cases design and refinement.
The test cases you saw/review are not the final one. QA will reduce it or refine
it after a test run. That's why you may saw some invalid bug in early stage.
But I think developer can help on this. If we can have transparent communication and
know the specific design in early stage, the invalid bug can be prevented.
Regarding the test case design, QA need to deliver our test cases to QAnalysis or
OEM vendors. It is impossible to design or describe a test cases too hard or unclear.
Also, I don't think the automation tests of JS marionette can cover the OEM build.
So, that's why we need to design test cases and test it manually.

My two cents!
Many thanks!

Comment 25

[Andrew Sutherland [:asuth] (he/him)]

(In reply to William Hsu [:whsu] from comment #24)
> If we don't do the force error testing, how could you know the user's feeling
> while they encounter the same problem?

I'm not sure what you mean by feeling in this case.  In general, we rely on UX for user-empathy ;)  More seriously, all the error cases and their impact on user-flow should be explicitly considered by UX/IX.  In the event the UX wire-frames/plans don't cover all failure cases discovered during implementation, the developer implementing the patch brings it up with UX to help determine a reasonable solution for the problem.

The best time for anyone, whether QA, developer, or otherwise to give input on error cases is during the design phase when the wire-frames are being discussed and iterated on.

> Please keep in mind. All the manual test can do that mean the FxOS user may
> suffer the bugs there. So, QA needs to test the cases on the first run then
> remove it if we don't need to deliver these test cases to QAnalysis.

Very thorough acceptance tests as part of review/landing or immediately after landing sounds like a great thing to me!  Having these duplicate what the automated tests to and exploratory testing to try and break the functionality also sounds good and fine since this helps verify that there are no gaps/major bugs in the automated testing.  I just want to make sure that we're not running unnecessary/redundant tests after that point.

> > on bugs filed thus far, there is a non-trivial time and frustration cost for
> > us to triage bugs where the filer fails to provide a logcat as requested in
> > the component filing notes and then when the log is provided it is clear
> > from the log that they made a typo, did not understand what a text/plain
> 
> So, why not raise the problem to stakeholders instead of discussing
> the problem in private?

Bugzilla isn't private; I've been assuming that QA/the people writing the moztrap cases either monitor the bugs that QAnalaydocs files and/or that there is a process for QAnalydocs to effect feedback.  As far as I can tell, Naoki and Jason monitor or have monitored the component.  If you are writing the moztrap cases but not watching the component, it seems like maybe we should re-align things so that whoever writes the cases also watches the component, be that you or someone else.  We definitely do have bugs in the e-mail component and I think it helps people write better targeted cases if they know what types of bugs are occurring.  (Also, it helps make sure that those writing the test-cases are affected by invalid bugs that get filed too! ;)

> The other question is if QA doesn't follow the UX wire-frames to design
> test cases, why do we need UX document?

At the current time, they are exclusively an input for engineering to implement functionality and become outdated immediately.  I would like it very much if UX could keep the wire-frames up-to-date, but UX hasn't had the time to do this so far and hasn't made it possible for engineering to update them.  It's possible with the new UX team assignments that Juwei will be able to keep the wire-frames up-to-date, which would be amazing.  I'll keep my fingers crossed, but let's not depend on that happening.

In other words, it's best to think of the UX wire-frames as comments on a bug or in an e-mail thread.  They're graphical depictions of suggestions, but they become stale just like the harder-to-understand textual description equivalents would be.

> These test cases seem out-of-date. This is because 
> (1) I got the final specification from Esther and the UX document mentioned
>     the "Select All" button
> (2) Esther said we hit some issues with localization and couldn't make the
>     string "Empty Trash" work.
> (3) I cannot get the final implementation from feature owner/developer while
>     I designed test cases.

And this is why I keep saying we should not write up the test cases until the implementation is completed.  The implementation is not done-done until it lands.  It's largely done by the time it is up for review, but there may be additional changes.  I think I have a good proposal to address this in a few paragraphs...

> But, do you know how long will the email client spend?
> This relates to performance. I created the two test cases here just because
> I need QAnalysis to pay attention on performance although I don't
> specifically
> describe it.

Performance without numbers/metrics tend not to be useful/actionable.  I think this is best left to automated tests since we can cheaply get multiple samples allowing us to specify tighter time-constraint assertions, and can plot them on datazilla to track trends.  We admittedly don't have any meaningful automated tests for the e-mail app (or almost any other applications) right now, but we're getting closer to be able to do this and it's a goal of the productivity engineering peoples and the performance (engineering) peoples.  Any help QA can provide in writing JS marionette tests or improving tooling to this end will help us get there faster.

Having said that, I think it's reasonable to put ballpark figures on the general operation of empty trash, but I would keep the human labor to a minimum.  For example, having the general smoke-test case involve deleting say 5 messages and then verifying that the deletion doesn't take longer than ~10 seconds would be appropriate.  That way the tester knows if it's taking 30 seconds that something is wrong and they should get our eyes on it.  (The 10 seconds helps allow for operations still completing in the background to complete, like large database commits, etc.)

I guess I should elaborate that while the e-mail app's implementation is constant-time and can't be made faster (we just delete the folder and replace it), there are fundamental underlying IndexedDB scaling realities and things that might be impacted by our IndexedDB schema.  But IndexedDB's implementation won't be changed without serious numbers demonstrating improvement, nor will our IndexedDB schema.

> By the way, did there have any official automation test case review?
> If so, please also share it to B2G-QA. We are glad to see that.
> If not, how do you make sure the automation test of email app is robust?
> Could you please share the test 
> after you automate the test cases?

This patch hasn't landed yet.  I agree it makes sense for QA to review the automated tests.  I think it also makes sense to have QA formally review the automated test cases prior to landing since follow-up test-writing bugs historically have low rates of timely resolution within Mozilla.  As part of the review I think QA requiring additional automated test cases to be added as long as they don't require massive amounts of additional tooling would be beneficial.  For example, performance tests currently require large testing infrastructure changes, so are out of scope.  But additional assertion-checks or test permutations that are not already well-covered by existing/added tests would be perfect.

Would you like me to mark you as a reviewer of the patch for the test-cases when I put it up for review or is there someone else on QA who should review?

This also seems like the best time for you/QA to write up the manual test cases, which I think also addresses your next paragraph.  At this time, you can:
- run the app in its actual implemented form or likely to be implemented form.  If you wait for the engineering review and any relevant ux-review to complete, then you can be much more sure of this.  I could mark you/QA as a reviewer only after those reviews complete if preferred.
- perform exploratory/acceptance testing without needing to write up the cases formally in moztrap.  It'd still be handy for you to file on the bug what types of testing you ran, but it probably takes less time than writing up all those moztrap cases!
- know what the automated tests cover so you don't need to write-up manual tests for those

If we treat QA's review and creation of manual test cases as a blocking step for landing, that hopefully also takes some time pressure off of you/QA and allows you to have the test cases ready when the functionality lands without having to speculatively create test cases ahead of time.
 
> My two cents!
> Many thanks!

Thanks for discussing this too!  I feel like we're on the road to an improved engineering/QA work-flow that should help us all improve the quality of the product and spending less time to do that!

Comment 26

[Brian Huang [:brianhuang][:brhuang]]

(In reply to Andrew Sutherland (:asuth) from comment #25)
> (In reply to William Hsu [:whsu] from comment #24)
> > If we don't do the force error testing, how could you know the user's feeling
> > while they encounter the same problem?
> 
> I'm not sure what you mean by feeling in this case.  In general, we rely on
> UX for user-empathy ;)  More seriously, all the error cases and their impact
> on user-flow should be explicitly considered by UX/IX.  In the event the UX
> wire-frames/plans don't cover all failure cases discovered during
> implementation, the developer implementing the patch brings it up with UX to
> help determine a reasonable solution for the problem.
> 
> The best time for anyone, whether QA, developer, or otherwise to give input
> on error cases is during the design phase when the wire-frames are being
> discussed and iterated on.
> 
> > Please keep in mind. All the manual test can do that mean the FxOS user may
> > suffer the bugs there. So, QA needs to test the cases on the first run then
> > remove it if we don't need to deliver these test cases to QAnalysis.
> 
> Very thorough acceptance tests as part of review/landing or immediately
> after landing sounds like a great thing to me!  Having these duplicate what
> the automated tests to and exploratory testing to try and break the
> functionality also sounds good and fine since this helps verify that there
> are no gaps/major bugs in the automated testing.  I just want to make sure
> that we're not running unnecessary/redundant tests after that point.
> 
> > > on bugs filed thus far, there is a non-trivial time and frustration cost for
> > > us to triage bugs where the filer fails to provide a logcat as requested in
> > > the component filing notes and then when the log is provided it is clear
> > > from the log that they made a typo, did not understand what a text/plain
> > 
> > So, why not raise the problem to stakeholders instead of discussing
> > the problem in private?
> 
> Bugzilla isn't private; I've been assuming that QA/the people writing the
> moztrap cases either monitor the bugs that QAnalaydocs files and/or that
> there is a process for QAnalydocs to effect feedback.  As far as I can tell,
> Naoki and Jason monitor or have monitored the component.  If you are writing
> the moztrap cases but not watching the component, it seems like maybe we
> should re-align things so that whoever writes the cases also watches the
> component, be that you or someone else.  We definitely do have bugs in the
> e-mail component and I think it helps people write better targeted cases if
> they know what types of bugs are occurring.  (Also, it helps make sure that
> those writing the test-cases are affected by invalid bugs that get filed
> too! ;)
> 
> > The other question is if QA doesn't follow the UX wire-frames to design
> > test cases, why do we need UX document?
> 
> At the current time, they are exclusively an input for engineering to
> implement functionality and become outdated immediately.  I would like it
> very much if UX could keep the wire-frames up-to-date, but UX hasn't had the
> time to do this so far and hasn't made it possible for engineering to update
> them.  It's possible with the new UX team assignments that Juwei will be
> able to keep the wire-frames up-to-date, which would be amazing.  I'll keep
> my fingers crossed, but let's not depend on that happening.
> 
> In other words, it's best to think of the UX wire-frames as comments on a
> bug or in an e-mail thread.  They're graphical depictions of suggestions,
> but they become stale just like the harder-to-understand textual description
> equivalents would be.
> 
> > These test cases seem out-of-date. This is because 
> > (1) I got the final specification from Esther and the UX document mentioned
> >     the "Select All" button
> > (2) Esther said we hit some issues with localization and couldn't make the
> >     string "Empty Trash" work.
> > (3) I cannot get the final implementation from feature owner/developer while
> >     I designed test cases.
> 
> And this is why I keep saying we should not write up the test cases until
> the implementation is completed.  The implementation is not done-done until
> it lands.  It's largely done by the time it is up for review, but there may
> be additional changes.  I think I have a good proposal to address this in a
> few paragraphs...
> 
> > But, do you know how long will the email client spend?
> > This relates to performance. I created the two test cases here just because
> > I need QAnalysis to pay attention on performance although I don't
> > specifically
> > describe it.
> 
> Performance without numbers/metrics tend not to be useful/actionable.  I
> think this is best left to automated tests since we can cheaply get multiple
> samples allowing us to specify tighter time-constraint assertions, and can
> plot them on datazilla to track trends.  We admittedly don't have any
> meaningful automated tests for the e-mail app (or almost any other
> applications) right now, but we're getting closer to be able to do this and
> it's a goal of the productivity engineering peoples and the performance
> (engineering) peoples.  Any help QA can provide in writing JS marionette
> tests or improving tooling to this end will help us get there faster.
> 
> Having said that, I think it's reasonable to put ballpark figures on the
> general operation of empty trash, but I would keep the human labor to a
> minimum.  For example, having the general smoke-test case involve deleting
> say 5 messages and then verifying that the deletion doesn't take longer than
> ~10 seconds would be appropriate.  That way the tester knows if it's taking
> 30 seconds that something is wrong and they should get our eyes on it.  (The
> 10 seconds helps allow for operations still completing in the background to
> complete, like large database commits, etc.)
> 
> I guess I should elaborate that while the e-mail app's implementation is
> constant-time and can't be made faster (we just delete the folder and
> replace it), there are fundamental underlying IndexedDB scaling realities
> and things that might be impacted by our IndexedDB schema.  But IndexedDB's
> implementation won't be changed without serious numbers demonstrating
> improvement, nor will our IndexedDB schema.
> 
> > By the way, did there have any official automation test case review?
> > If so, please also share it to B2G-QA. We are glad to see that.
> > If not, how do you make sure the automation test of email app is robust?
> > Could you please share the test 
> > after you automate the test cases?
> 
> This patch hasn't landed yet.  I agree it makes sense for QA to review the
> automated tests.  I think it also makes sense to have QA formally review the
> automated test cases prior to landing since follow-up test-writing bugs
> historically have low rates of timely resolution within Mozilla.  As part of
> the review I think QA requiring additional automated test cases to be added
> as long as they don't require massive amounts of additional tooling would be
> beneficial.  For example, performance tests currently require large testing
> infrastructure changes, so are out of scope.  But additional
> assertion-checks or test permutations that are not already well-covered by
> existing/added tests would be perfect.
> 
> Would you like me to mark you as a reviewer of the patch for the test-cases
> when I put it up for review or is there someone else on QA who should review?
> 
> This also seems like the best time for you/QA to write up the manual test
> cases, which I think also addresses your next paragraph.  At this time, you
> can:
> - run the app in its actual implemented form or likely to be implemented
> form.  If you wait for the engineering review and any relevant ux-review to
> complete, then you can be much more sure of this.  I could mark you/QA as a
> reviewer only after those reviews complete if preferred.
> - perform exploratory/acceptance testing without needing to write up the
> cases formally in moztrap.  It'd still be handy for you to file on the bug
> what types of testing you ran, but it probably takes less time than writing
> up all those moztrap cases!
> - know what the automated tests cover so you don't need to write-up manual
> tests for those
> 
> If we treat QA's review and creation of manual test cases as a blocking step
> for landing, that hopefully also takes some time pressure off of you/QA and
> allows you to have the test cases ready when the functionality lands without
> having to speculatively create test cases ahead of time.
>  
> > My two cents!
> > Many thanks!
> 
> Thanks for discussing this too!  I feel like we're on the road to an
> improved engineering/QA work-flow that should help us all improve the
> quality of the product and spending less time to do that!

As the members of the functional team, we hope this team have the best productivity include good quality. If the process between folks can be accepted, then we will have a good result. Since cases been automated must be later then manual testing, That's why we still need to have the manual test. If all cases can be automated, that will be great since we can have more runs.

Also, records on Moztrap will help other members to understand what will be tested for this feature. if there is something wrong that can't perform automation test, we can still get the reference from Moztrap to verify build.

Comment 27

[Dylan Oliver [:doliver]]

[priority] --> tracking-b2g:+ conversion

Comment 28

[BMO Automation]

Firefox OS is not being worked on