554846 - [mozmill] Common test module for Pause, Resume, Cancel, Retry a download

Status: RESOLVED WONTFIX

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

After some discussion, we have decided to merge Pause, Resume, Cancel and Retry Downloading BFTs into a single Mozmill test script.  These tests are originally documented in bug 524859, bug 536281, bug 524860, and bug 524856 respectively.

Comment 1

[u279076]

So I've been thinking about how we should implement this and I've come up with three options:

1. Have four test methods: testPauseDownload(), testResumeDownload(), testCancelDownload(), testRetryDownload()

2. Have two test methods: testPauseAndResumeDownload(), testCancelAndRetryDownload()

3. A single test method which follows the sequence: startDownload > pauseDownload > resumeDownload > cancelDownload > retryDownload

For code simplicity, #3 makes the most sense.

Do you have any thoughts on this Henrik?

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

That's correct. And given that we do not wanna create Mozmill tests 1:1 as the given Litmus test anymore, lets go with option 3.

Comment 3

[u279076]

So given that we are going with option 3 and this single test module will test 4 Litmus test cases, how should I handle the Litmus.meta tag at the end of the test?

Comment 4

[u279076]

Attachment: WIP Testcase

When running this test case the last test (checking for active download after clicking the Retry button) fails if I run from command line.  I can reproduce the failure in the extension, but only on the first execution.  If I keep re-running the test in the extension, it passes.

Subsequently, if I insert a controller.sleep(0); after each assertDownloadState() the test passes from command line.

I'm thinking we need some kind of timeout for assertDownloadState().

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

assertDownloadState should be removed! The API shouldn't have any assert functions. It's really up to the test. I will file a bug right now. After that you can use waitForEval.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #4)
> Subsequently, if I insert a controller.sleep(0); after each
> assertDownloadState() the test passes from command line.
> 
> I'm thinking we need some kind of timeout for assertDownloadState().

Looks like that some events are blocked and can't get processed. Looks like we will have to do a sleep before every user interaction. I will file a Mozmill bug.

For now, just move the declarations up, right before clicking pause. That will solve this problem.

Comment 7

[u279076]

Attachment: Patch (1.9.1)

Comment 8

[u279076]

Attachment: Patch initial actions [checked-in]

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 436271 [details] [diff] [review]
Patch (1.9.1)

Anthony, both patches contain "Mozilla Corporation" which has to be changed to "the Mozilla Foundation". I will do it this time but please be aware of it.

Why do you attach the same version of the patch for default and 1.9.1? Please do not do this. We only need an additional review if changes are required.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 436272 [details] [diff] [review]
Patch initial actions [checked-in]

r=me with the updated nits.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/5d5170429bc4
http://hg.mozilla.org/qa/mozmill-tests/rev/9dee33bb6857

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

While updating the Litmus tests I have noticed that the current tests only cover a part of all the given tests. We need additional test functions for actions like double click, space, or the context menu.

Comment 13

[u279076]

(In reply to comment #12)
> While updating the Litmus tests I have noticed that the current tests only
> cover a part of all the given tests. We need additional test functions for
> actions like double click, space, or the context menu.

This is more of FFT functionality than BFT functionality.

Comment 14

[u279076]

(In reply to comment #9)
> (From update of attachment 436271 [details] [diff] [review])
> Anthony, both patches contain "Mozilla Corporation" which has to be changed to
> "the Mozilla Foundation". I will do it this time but please be aware of it.
> 
This was not communicated to me at all.  When was this change decided upon?

> Why do you attach the same version of the patch for default and 1.9.1? Please
> do not do this. We only need an additional review if changes are required.
I was under the impression we need one for 1.9.1 and one for default.  Now that I know this is the case, I'll only make a 1.9.1 patch when there are changes required to the default patch.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #13)
> This is more of FFT functionality than BFT functionality.

It doesn't matter as long those tests are in the BFT area. We can't mark those as covered by Mozmill without having these tests inside. It's not that difficult.

(In reply to comment #14)
> > (From update of attachment 436271 [details] [diff] [review] [details])
> > Anthony, both patches contain "Mozilla Corporation" which has to be changed to
> > "the Mozilla Foundation". I will do it this time but please be aware of it.
> > 
> This was not communicated to me at all.  When was this change decided upon?

Now you are aware of. I read it lately in a posting from Gerv.

Comment 16

[u279076]

> > This is more of FFT functionality than BFT functionality.
> 
> It doesn't matter as long those tests are in the BFT area. We can't mark those
> as covered by Mozmill without having these tests inside. It's not that
> difficult.
> 
OK, that makes sense.  Having reviewed the Litmus tests, I'm not sure our decision to combine these into a single test was such a good idea.  It is going to make a bloated test if we check clickButton, doubleClick, contextMenu, and keyboardShortcut for pause, resume, cancel and retry.  This results in 4 tests being run on 4 actions, for a total of 16 tests in a single module.  This may make it pretty ambiguous when tracking down failures.  

It may be a good idea to back out testDownloadStates.js and reopen the 4 original bugs.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

It will not make the test bloated. Lets add 3 other test functions which run checks for the missing actions. Those tests are short and perfectly fit into one test module.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mass move of Mozmill Test related project bugs to newly created components. You can filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.

Comment 19

[u279076]

Probably goes without saying but I'm not working on this anymore.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

And it's nothing we want to have anymore given that we transition over to Marionette.