Integration test script for jetpack SDK

RESOLVED FIXED

Status

Add-on SDK
General
--
enhancement
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: ashah, Assigned: ashah)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

8 years ago
Please post your requests/feedback here.
Documentation attached.
(Assignee)

Comment 1

8 years ago
Created attachment 457371 [details]
Documentation
(Assignee)

Comment 2

8 years ago
Created attachment 457373 [details]
Integration script
This is great stuff!  It makes it very simple to download and test a build, and I was able to use it to test the tip (in addition to a release build) by pointing the script at the magic "tip of tree as ZIP archive" URL that Mercurial provides:

  python integration_check.py -u http://hg.mozilla.org/labs/jetpack-sdk/archive/tip.zip

A couple of suggestions for further development:

1. I'd prefer the script to work in the current directory rather than the desktop and to write the log file to the current directory as well, so that once you have run the script, the current directory contains the following files:

  jetpack-sdk-[version].zip
  jetpack-sdk-[version]/
  tests.log

That way I can determine where these files get written by simply changing to the right directory beforehand.  Also, because the log file doesn't get written to the SDK directory, there's no chance that its presence alters the behavior of the tests (which is really unlikely to happen anyway, but this way we can be sure of it).

2. Support for Windows and testing tarballs (on Mac/Linux) in addition to ZIP archives would complete the coverage of package formats and platforms.

3. The ability to specify the binary with which to run tests would be handy.  That way I can test against both Firefox 3.6 and the trunk build of Firefox without having to employ a workaround like the following:

  env PATH=/path/to/binary:$PATH python integration_check.py ...

The workaround isn't too onerous, but it'd be easier to just say:

  python integration_check.py -b /path/to/binary/firefox ...
(Assignee)

Comment 4

8 years ago
Created attachment 460314 [details]
Updated script
(Assignee)

Comment 5

8 years ago
(In reply to comment #3)
 
> A couple of suggestions for further development:
> 
> 1. I'd prefer the script to work in the current directory rather than the
> desktop and to write the log file to the current directory as well, so that
> once you have run the script, the current directory contains the following
> files:
> 
>   jetpack-sdk-[version].zip
>   jetpack-sdk-[version]/
>   tests.log
> 
> That way I can determine where these files get written by simply changing to
> the right directory beforehand.  Also, because the log file doesn't get written
> to the SDK directory, there's no chance that its presence alters the behavior
> of the tests (which is really unlikely to happen anyway, but this way we can be
> sure of it).
 Done.

> 2. Support for Windows and testing tarballs (on Mac/Linux) in addition to ZIP
> archives would complete the coverage of package formats and platforms.
 Now supports tarball as well. Windows support will be in next version.

> 3. The ability to specify the binary with which to run tests would be handy. 
> That way I can test against both Firefox 3.6 and the trunk build of Firefox
> without having to employ a workaround like the following:
> 
>   env PATH=/path/to/binary:$PATH python integration_check.py ...
> 
> The workaround isn't too onerous, but it'd be easier to just say:
> 
>   python integration_check.py -b /path/to/binary/firefox ...
Next version.


Myk: 
Based on your suggestions in the comment 3, I have patched the script with the change # 1 and #2(except windows).

The ability to specify the binary and windows support will be a part of the next version. Until then, this script can be made live.
I used the previous version of the script to test the 0.6rc1 builds, and I just used the latest version to test the 0.6rc2 builds.  It's really helpful, saving me a bunch of headache and worry.

We should get this script into version control so we can keep working on it there.  It's not a perfect fit for the jetpack-sdk repository, but at the same time I can't think of a better place for it.  So perhaps some kind of tools/ subdirectory?  I think Brian has been thinking about something of the sort, so cc:ing him for his thoughts.
Comment on attachment 460314 [details]
Updated script

As a prerequisite to getting this script checked in, it should undergo review.  Brian, as one of our Pythoniest hackers, would be a good reviewer for this.  Brian, are you game?
Attachment #460314 - Attachment mime type: text/x-python-script → text/plain
Attachment #460314 - Flags: review?(warner-bugzilla)

Comment 8

8 years ago
Note that in some sense, cfx is a tool, as is mozrunner and hgall, all of which are in the 'bin' directory of the jetpack-sdk and thereby added to the user's PATH after they do 'source bin/activate'. It seems like it would be appropriate to put this script in 'bin', too, but I could be wrong.
in other projects, I've had a top-level "misc/" directory for stuff like this. My only concern with putting it in our bin/ directory is that it should have a name that won't be too likely to clash with other tools the user might have installed: if they use their cfx "bin/activate" environment for everything, then it'd be nice if a fairly-infrequently-used tool like this did not steal namespace from more-frequently used commands. Also, this is not a tool that anyone who had already done bin/activate would ever use, right? This tool would specifically be run by people who had *not* already activated an environment.

I'll look at the script now..
Comment on attachment 460314 [details]
Updated script

If this script works, it can go in without major changes, but there are a
couple of style/idiom improvements that would make this script easier to
maintain in the long run:

(Warning: I can be pretty picky about python scripts, especially about style)

 * four-space indents
 * "x = 1", not "x=1" (see PEP8 for some basic guidelines)
 * "# comment", not "#comment"
 * use os.getcwd() instead of spawning subprocess.Popen('pwd')
 * use os.path.expanduser("~") instead of spawning a subprocess
 * you use subprocess.Popen() enough to justify a "get_output_from" helper
   function
 * the try/except clauses don't terminate the program: printing an
   extra-informative error message is great, but silently continuing as if
   nothing had gone wrong is less great. Wrap as little as possible in the
   try/except block (to avoid mis-representing errors that occur in
   unexpected places). Use things like:

     try:
         f = urllib2.urlopen(url)
     except urllib2.URLError:
         print "URL not correct, check again"
         raise
     out = open(fpath,'w')
     out.write(f.read())
     out.close()

 * download() should only download. extract() should only extract. There
   should be a main() function that invokes download(), then extract(),
   then run_testall().
 * you can probably avoid using global variables so much: pass parameters
   from one step to another
 * the kill_process() watchdog should be factored out into a helper function
Attachment #460314 - Flags: review?(warner-bugzilla) → review+
(Assignee)

Updated

8 years ago
Blocks: 584479

Updated

8 years ago
Status: NEW → ASSIGNED
I talked to Atul about where this should get checked in, and he decided that we should place it in bin/ until we figure out a better place for it.  I have done so, making only two changes in the process: I removed the ".py" extension from the file, since the other Python scripts in bin/ don't have one.  And I changed the underscore to a dash, which is consistent with the names of other multi-word files in the repository (like quick-start).

I also set the permissions on the file to 0755 before checking it in.

Fixed by changeset https://hg.mozilla.org/labs/jetpack-sdk/rev/5649f2a796c7 (although sadly I failed to set the author appropriately for the checkin--sorry Ayan!).
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
No longer blocks: 584479
(Assignee)

Updated

7 years ago
Blocks: 584479
The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".
Component: Jetpack SDK → General
Product: Mozilla Labs → Add-on SDK
QA Contact: ayanshah62 → general
You need to log in before you can comment on or make changes to this bug.