Bug 61939 - Print error message when there is no httpd present on the system
Summary: Print error message when there is no httpd present on the system
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Andras Becsi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-02 10:01 PDT by Andras Becsi
Modified: 2011-06-03 10:32 PDT (History)
6 users (show)

See Also:


Attachments
proposed patch (2.10 KB, patch)
2011-06-02 10:03 PDT, Andras Becsi
no flags Details | Formatted Diff | Diff
proposed patch v2 (1.83 KB, patch)
2011-06-03 05:06 PDT, Andras Becsi
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from cr-jail-7 (190.58 KB, application/zip)
2011-06-03 06:45 PDT, WebKit Commit Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andras Becsi 2011-06-02 10:01:06 PDT
Skip the http tests when no httpd could be found on the system instead of crashing at the very end of a frustatingly long testing session.
Comment 1 Andras Becsi 2011-06-02 10:03:10 PDT
Created attachment 95773 [details]
proposed patch
Comment 2 Alexey Proskuryakov 2011-06-02 22:51:15 PDT
Is this a configuration we would ever desire to run the tests in? Perhaps run-webkit-tests should just refuse to run instead?
Comment 3 Andras Becsi 2011-06-03 05:06:35 PDT
Created attachment 95892 [details]
proposed patch v2
Comment 4 Csaba Osztrogonác 2011-06-03 06:11:55 PDT
Comment on attachment 95892 [details]
proposed patch v2

Great fix, r=me
Comment 5 WebKit Commit Bot 2011-06-03 06:45:54 PDT
Comment on attachment 95892 [details]
proposed patch v2

Rejecting attachment 95892 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'build-..." exit_code: 2

Last 500 characters of output:
/VCSUtils_unittest/runPatchCommand.....................................ok
Tools/Scripts/webkitperl/VCSUtils_unittest/setChangeLogDateAndReviewer.........................ok
All tests successful.
Files=19, Tests=363,  2 wallclock secs ( 1.03 cusr +  0.26 csys =  1.29 CPU)
syntax error at Tools/Scripts/old-run-webkit-tests line 528, near "else if"
syntax error at Tools/Scripts/old-run-webkit-tests line 540, near "}"
Execution of Tools/Scripts/old-run-webkit-tests aborted due to compilation errors.

Full output: http://queues.webkit.org/results/8757760
Comment 6 WebKit Commit Bot 2011-06-03 06:45:58 PDT
Created attachment 95907 [details]
Archive of layout-test-results from cr-jail-7

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: cr-jail-7  Port: Mac  Platform: Mac OS X 10.6.7
Comment 7 Csaba Osztrogonác 2011-06-03 06:50:15 PDT
Comment on attachment 95892 [details]
proposed patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=95892&action=review

> Tools/Scripts/old-run-webkit-tests:528
> +} else if (!hasHTTPD()) {

Please s/else if/elsif before landing. ;)
Comment 8 Andras Becsi 2011-06-03 07:19:48 PDT
(In reply to comment #7)
> (From update of attachment 95892 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=95892&action=review
> 
> > Tools/Scripts/old-run-webkit-tests:528
> > +} else if (!hasHTTPD()) {
> 
> Please s/else if/elsif before landing. ;)

D'oh!

My hugger-mugger trivial patches can make the whole tree red.
Don't trust me blindly ;)
Comment 9 Eric Seidel (no email) 2011-06-03 07:22:57 PDT
Why not use the commit-queue?  And why not CC anyone who has worked on this perl before?
Comment 10 Andras Becsi 2011-06-03 07:31:07 PDT
(In reply to comment #9)
> Why not use the commit-queue?  And why not CC anyone who has worked on this perl before?

Sorry for the noise, my fault.
Comment 11 Alexey Proskuryakov 2011-06-03 08:59:12 PDT
Looking good. Do we need the same change for new-run-webkit-tests?
Comment 12 Dirk Pranke 2011-06-03 09:36:01 PDT
Yeah, looks like new-run-webkit-tests will want something similar.
Comment 13 Daniel Bates 2011-06-03 10:31:43 PDT
For completeness, this was committed in <http://trac.webkit.org/changeset/88012>.
Comment 14 Daniel Bates 2011-06-03 10:32:49 PDT
Comment on attachment 95892 [details]
proposed patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=95892&action=review

I know this patch was already reviewed and landed. I have some remarks.

> Tools/ChangeLog:9
> +        * Scripts/webkitperl/httpd.pm:

It is convention that we either list or add a remark here about the function(s) that we added/modified for Perl changes since prepare-ChangeLog doesn't know how to generate the list of added and modified functions for Perl scripts. We should fix this.

> Tools/Scripts/old-run-webkit-tests:529
> +    print "\nNo httpd found. Cannot run http tests.\nPlease use --no-http if you do not want to run http tests.\n"

You're missing a semicolon ';' at the end of this line. I assume you tested this script with your changes. I'm not sure why this wouldn't generate a syntax error.

ah, you caught this and fixed it in <http://trac.webkit.org/changeset/88013>.

> Tools/Scripts/webkitperl/httpd.pm:93
> +    return system(@command) == 0;

This should read:

return exitStatus(system(@command)) == 0;

By definition of system(), "The return value [of system()] is the exit status of the program as returned by the wait call. To get the actual exit value, shift right by eight". We have a convenience function, called exitStatus(), that does this in a portable way. See <http://perldoc.perl.org/functions/system.html> for more details.