Bug 15002 – Script to automatically search nightly builds for regressions (bisect-builds)

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

It would be useful to have a utility script that would automatically search nightly builds for regressions (to the extent that it would download images, mount them, launch Safari, let the user test, ask if the bug occurred or not, then narrow the binary search results based on the answer until two adjacent builds were found (one that doesn't reproduce the bug, and one that does).

[David Kilzer (:ddkilzer)]

Created attachment 16018 [details]
WIP v1

Here's a work-in-progress script I hacked together today.

Features:
- You may use -l or --local to only search nightlies on disk (no Internet access).
- Revisions may be specified in a multitude of ways:  -rM:N; -rM -rN; -rM:rN; -rM
- Sanity check (verify starting revision works, ending revision fails) using -s or --sanity-check.
- See "autospade -h" for help.
- Does NOT invoke WebKit.app, but instead invokes Safari.app with a DYLD_FRAMEWORK_PATH set to the Resources directory on the mounted image.

Warts:
- Won't work on Windows.
- Doesn't use webkitdirs.pm or any other scripts in WebKitTools/Scripts.
- You must modify $nightlyDirectory to the location where you keep your nightly builds.
- Creates too many temp files (only one is needed per run).
- Potentially dodgy code (hacked together today).
- Older WebKit nightlies (on OS X) do not work with Safari 3 Public Beta due to missing API.  Testing (spading?) works much better with Safari 2.

Do we want to check this in so others may hack on it quickly as well?

[David Kilzer (:ddkilzer)]

Invalid image (remove from __DATA__ segment later):

WebKit-SVN-r17208.dmg

[David Kilzer (:ddkilzer)]

Created attachment 16019 [details]
Updated revision list

NOTE: Some files were missing in the __DATA__ section, so delete the list before using the script or use this list as a replacement.

[Mark Rowe (bdash)]

Fwiw:
        # We know the last page is 'r12166-r11976', so make some dangerous assumptions

That was only true until a new nightly build was uploaded.  Adding a new nightly at the front of the list bumped every build down one in the listing, which leaves a new build at the top of the last page.

What is the purpose of the hard-coded list of builds anyway?

[David Kilzer (:ddkilzer)]

(In reply to comment #4)
> What is the purpose of the hard-coded list of builds anyway?

The idea was to save on bandwidth/server load by caching a list of builds (since they don't change historically), and only load web pages until we found duplicate entries.  It also made for faster development since the script didn't have to hit the server more than two times every time I ran it.

If there is a single URL that will provide a list of files for downloading, this pretty much all goes away, assuming we don't have hundreds of developers using the tool at the same time.  :)

Like the comment said, this was just a hack to get it working.

[David Kilzer (:ddkilzer)]

Created attachment 16186 [details]
WIP v2

Changes since WIP v1 (still no love for Windows, sorry):

- Added r13688 to %knownBadBuilds.
- Added r15860 to %knownBadBuilds.
- Moved %knownBadBuilds from with makeNightlyList() to a global variable.
- Updated local file code path in makeNightlyList() to obey %knownBadBuilds.
- Added r14901 to %knownBadBuilds.
- Extracted printStatus() subroutine from body of main script.
- Added newline to beginning of printStatus() print statement (which explains previous dangling '\').
- Call printStatus() after sanity check but before spading in case there are no revisions between start and end.
- Removed extra blank lines between subroutine definitions.
- Changed makeNightlyList() to use new URL (http://nightly.webkit.org/builds/mac/all) that lists all available builds.
- Added %knownBadRevisions hash to makeNightlyList().
- Removed now-obsolete DATA segment at end of script.
- Added -p|--progression switch to test for progressions.
- Extracted createTempFile($) subroutine so that only one temp file is created per run.
- Removed redundant message at end of run; the Fails/Works line is sufficient.
- Removed WebKit-SVN-r16390.dmg from the DATA segment because it caused a crash on my system.

[David Kilzer (:ddkilzer)]

Created attachment 16424 [details]
Patch v1

I think this has baked long enough.  Providing patch for review.  No Windows support as I don't have a Windows box set up to play with.

Changes since WIP v2:

- Add description of script and instructions on how to create ~/.autospaderc to override values.
- Created loadConfiguration() subroutine to load variable overrides from a ~/.autospaderc file.
- Renamed $nightlyDirectory to $nightlyDownloadDirectory.
- Made $nightlyDownloadDirectory and $safariPath overrideable via ~/.autospaderc.
- Added $safariPath argument to mountAndRunNightly() subroutine.
- Moved subroutine declarations to immediately follow use statements.
- Fixed URLs to list of files and individual nightlies for trunk builds after feature-branch builds were added.
- Removed r16390 from %knownBadBuilds because it doesn't always crash (www.google.com works).
- Added r17208 to %knownBadBuilds because that DMG doesn't mount.
- Verified other DMG revisions in %knownBadBuilds either don't mount (r14901, r15860) or have an unexpected directory structure (r13688).
- Changed behavior for finding starting/ending revisions with --sanity to expand the range rather than to constrict the range could lead to an invalid search, and we pay so little for testing slightly larger ranges.
- Fixed a warning about an uninitialized variable in mountAndRunNightly() if no test URL is given.

Comment on attachment 16424 [details]
Patch v1

+        * Scripts/autospade: Added.

I think the script should have a descriptive name, like the rest of WebKitTools/Scripts. Codenames, Apple-internal or otherwise, just make things harder for newcomers.

[Mark Rowe (bdash)]

Dave, you might want to revisit knownBadBuilds -- I think I purged most, if not all, of the broken builds from nightly.webkit.org recently during a maintenance spree I went on.

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> (From update of attachment 16424 [details] [edit])
> +        * Scripts/autospade: Added.
> 
> I think the script should have a descriptive name, like the rest of
> WebKitTools/Scripts. Codenames, Apple-internal or otherwise, just make things
> harder for newcomers.

Yeah, I was thinking about this today.  Suggestions?

find-regressions (with a symlink to find-progressions :)
search-nightly-webkit-builds-for-regressions
perform-binary-search-of-nightly-webkit-builds-for-regressions-or-progressions

[David Kilzer (:ddkilzer)]

(In reply to comment #9)
> Dave, you might want to revisit knownBadBuilds -- I think I purged most, if not
> all, of the broken builds from nightly.webkit.org recently during a maintenance
> spree I went on.

So you rebuilt the bad ones, or just removed them?  :)  If they were removed, it won't make much difference if I leave the builds in there or not, but I'll check them before I commit this.

Other ideas for command-line switches (which aren't implemented yet):
--branch=trunk|feature
--safari=/path/to/Safari.app
--downloadDirectory=/path/to/download/dir

[Mark Rowe (bdash)]

(In reply to comment #11)
> (In reply to comment #9)
> > Dave, you might want to revisit knownBadBuilds -- I think I purged most, if not
> > all, of the broken builds from nightly.webkit.org recently during a maintenance
> > spree I went on.
> 
> So you rebuilt the bad ones, or just removed them?  :)  If they were removed,
> it won't make much difference if I leave the builds in there or not, but I'll
> check them before I commit this.

I just removed the bad builds.

> Other ideas for command-line switches (which aren't implemented yet):
> --branch=trunk|feature
> --safari=/path/to/Safari.app
> --downloadDirectory=/path/to/download/dir

For simplicity and consistency with nightly.webkit.org, I think having --branch accept either trunk or feature-branch would be better.  It should also be --download-directory :)

[Mark Rowe (bdash)]

(In reply to comment #10)
> (In reply to comment #8)
> > (From update of attachment 16424 [details] [edit] [edit])
> > +        * Scripts/autospade: Added.
> > 
> > I think the script should have a descriptive name, like the rest of
> > WebKitTools/Scripts. Codenames, Apple-internal or otherwise, just make things
> > harder for newcomers.
> 
> Yeah, I was thinking about this today.  Suggestions?
> 
> find-regressions (with a symlink to find-progressions :)
> search-nightly-webkit-builds-for-regressions
> perform-binary-search-of-nightly-webkit-builds-for-regressions-or-progressions

bisect-nightly-builds?

[Eric Seidel (no email)]

(In reply to comment #13)
> > search-nightly-webkit-builds-for-regressions

A couple thoughts:

1.  Why do you need a -progression flag?  I didn't look that closely, but I would assume in the end you're just looking for changes.

2.  Thus, the name could be "search-builds-for-change" or just "search-builds".  I think "nightly" is misleading.

Oh... and eeewwe it's perl. ;p  Perhaps I'll read it more closely later.

[David Kilzer (:ddkilzer)]

(In reply to comment #14)
> 1.  Why do you need a -progression flag?  I didn't look that closely, but I
> would assume in the end you're just looking for changes.

Why not?  Sometimes you may want to search for a progression.  This flag makes it easier for the user to simply answer the question after each run instead of reversing the logic in their head (which would be required to use the tool to find progressions without the regression tag).

> 2.  Thus, the name could be "search-builds-for-change" or just "search-builds".
>  I think "nightly" is misleading.

I agree, although "nightly builds" is what we've been using to refer to them for a while now.  I like Mark's 'bisect-nightly-builds' or, combining Mark's and your idea, 'bisect-builds'.

> Oh... and eeewwe it's perl. ;p  Perhaps I'll read it more closely later.

I realize that not everyone is a Perl guru, but it's the scripting language I know best, and I thought it would be better to get the tool into the hands of the masses than for me to spend time learning a new scripting language (at least for this project).

[David Kilzer (:ddkilzer)]

Comment on attachment 16424 [details]
Patch v1

I'm going to spin another patch with a new script name, and add some more features.

[David Kilzer (:ddkilzer)]

(In reply to comment #15)
> Why not?  Sometimes you may want to search for a progression.  This flag makes
> it easier for the user to simply answer the question after each run instead of
> reversing the logic in their head (which would be required to use the tool to
> find progressions without the regression tag).

Not "regression tag" but "--progression flag".  Not sure which wires got crossed there!

[Mark Rowe (bdash)]

Keeping the downloaded nightlies on the desktop by default seems rather strange.  In normal use of this script I would consider them to be a local cache of the available builds, which suggests that ~/Library/Cache/ would be an ideal location for them.  I don't see any harm in making it configurable, I just think the default should be more appropriate.

[David Kilzer (:ddkilzer)]

Created attachment 16480 [details]
Patch v2

Changes since Patch v1 (Attachment 16424 [details]):

- Made --branch=feature work as expected (equivalent to --branch=feature-branch).
- Added arguments to new command-line switches in usage text.
- Fixed creation of $nightlyBuildsURLBase and $nightlyFilesURLBase.
- Fixed directory creation of $nightlyDownloadDirectory.
- Added --branch command-line switch to test different branches of nightly builds.
- Removed %knownBadBuilds hash map since bad builds have been removed from the web site.
- Added --download-directory and --safari-path command-line switches to override default or config file settings.
- Renamed loadConfiguration() to loadSettings(), and cleaned up the code that set defaults.
- Replaced 'use warnings' with -w switch.
- Added 'h' shortcut to help switch.
- Moved default location for WebKit nightly builds from ~/Desktop to ~/Library/Caches.
- Renamed autospade to bisect-builds.
- Changed "autospade" to "bisect-builds" in source.
- Added description of functionality to help text.

[David Kilzer (:ddkilzer)]

Created attachment 16512 [details]
Patch v3

Changes since Patch v2:

- Rewrite the code in makeNightlyList() that reads the list of local nightly builds from disk to be more readable.
- Fix code formatting: put else/elsif on the same line as the closing brace.
- Print the entire help message to STDERR.

[Eric Seidel (no email)]

Comment on attachment 16512 [details]
Patch v3

Looks *totally* fine for a "first pass".  Best to just get this in the hands of the people!  A (small) wiki article might be helpful too.

[David Kilzer (:ddkilzer)]

Committed revision 26653.

Changes since Patch v3 (which I felt were needed before committing the tool):

- When using Safari 3.0.x, don't use nightly builds older than r19992 since they'll always crash.  (Yes, I used bisect-builds itself to find this!)
- Attempt to unmount a disk image before mounting a new one for up 10 seconds before quitting with an error.  This happens if you're regressing or progressing a crasher.
- Don't die if there are non-fatal errors parsing ~/.bisect-buildsrc.  (Can't determine the difference between a file  with no statements and a file with warnable syntax.)