Closed Bug 688330 Opened 9 years ago Closed 9 years ago

Make special-powers install method a little less special

Categories

(Testing :: Mochitest, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla10

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

References

Details

(Whiteboard: [inbound])

Attachments

(1 file, 1 obsolete file)

Attached patch Patch, v1 (obsolete) — Splinter Review
I'd like to add an additonal test extension, much like special powers, but only for telephony stuff. The attached patch makes the special-powers install method more generic so that other test code can follow along. Needs the patch from bug 688300.
Attachment #561616 - Flags: review?(ted.mielczarek)
Comment on attachment 561616 [details] [diff] [review]
Patch, v1

Review of attachment 561616 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine. Can you make sure this passes Android mochitests on try? The Remote test harnesses can be fiddly.

::: testing/mochitest/runtests.py
@@ +832,5 @@
> +  def installExtensionFromPath(self, options, path, extensionID = None):
> +    extensionPath = self.getFullPath(path)
> +
> +    if extensionID is None and os.path.isdir(extensionPath):
> +      extensionID = self.getLeafNameForPath(extensionPath)

As I asked on the other bug, is it worth doing this, or should we just always parse the install.rdf to get the extension name?

@@ +838,5 @@
> +    self.automation.log.info("INFO | runtests.py | Installing extension at %s to %s." %
> +                            (extensionPath, options.profilePath))
> +    self.automation.installExtension(extensionPath, options.profilePath,
> +                                     extensionID)
> +    self.automation.log.info("INFO | runtests.py | Done installing extension.")

I don't think you need both of these lines, that's a bit verbose. (I know we had them both in the SpecialPowers code, but if we're going to do this more than once we should cut it down a bit.)

@@ +846,3 @@
>  
> +    extensionDirs = [
> +      # Testing extensions.

I might say instead "extensions distributed with the harness".

@@ +847,5 @@
> +    extensionDirs = [
> +      # Testing extensions.
> +      os.path.normpath(os.path.join(self.SCRIPT_DIRECTORY, "extensions")),
> +      # Extensions distributed with the application.
> +      os.path.join(options.app[ : options.app.rfind(os.sep)], "distribution", "extensions")

Do extensions in distribution/extensions not get automatically installed by the app?

::: testing/mochitest/specialpowers/Makefile.in
@@ +64,2 @@
>  libs::
> +	$(MKDIR) -p $(TEST_EXTENSIONS_DIR)

bug 680246 is going to re-land real soon, and you can do:
GENERATED_DIRS += $(TEST_EXTENSIONS_DIR)
libs:: $(TEST_EXTENSIONS_DIR)
...

::: testing/mochitest/specialpowers/install.rdf
@@ +3,5 @@
>  <RDF xmlns="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
>       xmlns:em="http://www.mozilla.org/2004/em-rdf#">
>  
>    <Description about="urn:mozilla:install-manifest">
> +#expand    <em:id>__XPI_NAME__</em:id>

Doesn't seem like a worthwhile change to me, even considering the duplication.
Attachment #561616 - Flags: review?(ted.mielczarek) → review+
Assignee: nobody → bent.mozilla
Attached patch Patch, v1.1Splinter Review
Updated to address review comments.
Attachment #561616 - Attachment is obsolete: true
Attachment #563079 - Flags: review+
Since bug 680246 keeps getting backed out I just went with "$(MKDIR) -p" for now.
No longer depends on: 680246
https://hg.mozilla.org/mozilla-central/rev/870bd2683c5e
https://hg.mozilla.org/mozilla-central/rev/e39e7e990e3a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.