Closed Bug 932948 Opened 6 years ago Closed 6 years ago

Running mach mochitest-a11y with installed extension


(Testing :: Mochitest, defect)

Not set


(Not tracked)



(Reporter: kamilm, Assigned: kamilm)



(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130911155223

Steps to reproduce:

It is impossible to run 'mach mochitest-a11y' with extension installed. There is a support for this feature in testing/mochitest/ (see bug, but 'mach mochitest-a11y' command doesn't provide --install-extension attribute.

Actual results:

'mach mochitest-a11y 'command doesn't provide --install-extension attribute. 

Expected results:

'mach mochitest-a11y 'command should provide --install-extension attribute - code responsible for actual extension installing is already present in testing/mochitest/
Attached patch extension.diff (obsolete) — Splinter Review
Proposed patch that adds --install-extension functionality to mach mochitest-a11y
Attachment #824834 - Flags: review?(ted)
I've found --install-extension feature really useful when debugging xul tests with DomInspector extension, so I thought it would be nice to have it available directly from mach mochitest-a11y command.
Comment on attachment 824834 [details] [diff] [review]

Review of attachment 824834 [details] [diff] [review]:

My apologies for the extremely long review delay, that's unacceptable.

r=me with two nits fixed.

::: testing/mochitest/
@@ +287,4 @@
>          options.thisChunk = this_chunk
>          options.failureFile = failure_file_path
> +        if install_extension != None:

if install extension is not None:

@@ +413,4 @@
>              'executed.')
>      func = path(func)
> +    install_extension = CommandArgument('--install-extension', 

nit: trailing whitespace here

@@ +415,5 @@
> +    install_extension = CommandArgument('--install-extension', 
> +        help='Install given extension before running selected tests. ' \
> +            'Parameter is a path to xpi file.')
> +    func = install_extension(func)

I wonder if we shouldn't support a comma-delimited list of XPI files, since I don't think CommandArgument allows you to create an argument that appends to a list. Not really necessary for this to land though.
Attachment #824834 - Flags: review?(ted) → review+
Changes after the last review, patch formatted according to the 'Using Mercurial' guide from MDN
Attachment #824834 - Attachment is obsolete: true
Attachment #8348290 - Flags: review?(ted)
Ted, I've recreated the patch according to this guideline.

I've removed that trailing space. 
And as you said, this patch will only support one extension to be installed. I can try to figure out how to support more extensions at once, but I don't think that's really necessary. 

Can you do this commit for me? As a Mozilla newbie, I have no access rights to do this.
Flags: needinfo?(ted)
Attachment #8348290 - Flags: review?(ted) → review+
Once your patch is reviewed you just need to add the checkin-needed keyword and someone will take care of that. Thanks for the patch!
Assignee: nobody → muszynski.kamil
Ever confirmed: true
Flags: needinfo?(ted)
Keywords: checkin-needed
No problem, I hope someone will find this addition useful ;)
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.