Bug 37786

Summary: new-run-webkit-tests: checking the build should be optional
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: Tools / TestsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cjerdonek, eric, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
eric: review+
revise w/ ojan's feedback (rename --check-build to --build)
none
tweak the help text, changelog eric: review+

Description Dirk Pranke 2010-04-18 17:51:11 PDT
Checking whether or not a build is up-to-date can take 5-10 seconds or longer, and sometime you may wish to run the tests without rebuilding, so there should be a way to skip the check_build step.
Comment 1 Dirk Pranke 2010-04-18 18:01:30 PDT
Created attachment 53648 [details]
Patch
Comment 2 Eric Seidel (no email) 2010-04-18 22:25:19 PDT
Comment on attachment 53648 [details]
Patch

Two notes:
1.  Sad that we need this.  Seems like a hack around build system lameness.
2.  I hate that optparse doesn't have built in support for --no-.  We need to make our own make_option wrapper which does automatic --no- generation as well as puts the default value in the help text.
Comment 3 Ojan Vafai 2010-04-19 08:03:21 PDT
(In reply to comment #2)
> 2.  I hate that optparse doesn't have built in support for --no-.  We need to
> make our own make_option wrapper which does automatic --no- generation as well
> as puts the default value in the help text.

Why do we need to have both flags? Since it defaults to True, don't we only need --no-check-build?

It seems like it's the actual rebuilding that slows things down. It's a shame to get rid of all the other checks in check_build just to avoid the rebuilding. How about adding a --no-build instead? I also think that would be more intuitive to users. If I didn't know the code, it wouldn't be clear to me that check-build is the thing that causes DRT to be built.
Comment 4 Dirk Pranke 2010-04-19 10:30:27 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > 2.  I hate that optparse doesn't have built in support for --no-.  We need to
> > make our own make_option wrapper which does automatic --no- generation as well
> > as puts the default value in the help text.
> 
> Why do we need to have both flags? Since it defaults to True, don't we only
> need --no-check-build?

This may be a personal thing, but I don't like having boolean command line flags where both versions of the flag aren't legal.
 
> It seems like it's the actual rebuilding that slows things down. It's a shame
> to get rid of all the other checks in check_build just to avoid the rebuilding.
> How about adding a --no-build instead? I also think that would be more
> intuitive to users. If I didn't know the code, it wouldn't be clear to me that
> check-build is the thing that causes DRT to be built.

This would be fine, and is probably safer.
Comment 5 Dirk Pranke 2010-04-19 15:16:55 PDT
Created attachment 53727 [details]
revise w/  ojan's feedback (rename --check-build to --build)
Comment 6 Ojan Vafai 2010-04-19 15:26:00 PDT
LGTM.
Comment 7 Eric Seidel (no email) 2010-04-19 16:12:37 PDT
Comment on attachment 53727 [details]
revise w/  ojan's feedback (rename --check-build to --build)

The option name and help are misleading.  This will only make sure that DumpRenderTree is built.  Not webkit itself.

I've been trying to use full sentences in option help (start with a capital, end with a period).

I think  the option names can stay, but we should fix the help to explain it only builds the helper tools, not WebKit.
Comment 8 Dirk Pranke 2010-04-19 16:45:56 PDT
Created attachment 53738 [details]
tweak the help text, changelog
Comment 9 Eric Seidel (no email) 2010-04-19 16:48:56 PDT
Comment on attachment 53738 [details]
tweak the help text, changelog

OK.
Comment 10 Dirk Pranke 2010-04-19 17:33:14 PDT
Committed r57858: <http://trac.webkit.org/changeset/57858>