Bug 33200 – Generate list of Perl unit test files dynamically

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Chris Jerdonek]

As David K. suggested here:

https://bugs.webkit.org/show_bug.cgi?id=33124#c15

[Chris Jerdonek]

Created attachment 45869 [details]
Proposed patch

Is there a preferred ordering of the "use" statements at the top?

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45869 [details] without any errors.

[David Kilzer (:ddkilzer)]

Comment on attachment 45869 [details]
Proposed patch

These changes look fine, but I noticed that the script isn't using strict mode and doesn't have warnings enabled:

> Index: WebKitTools/Scripts/test-webkitperl

- #/usr/bin/perl
+ #/usr/bin/perl -w

> @@ -30,6 +30,7 @@
>  
>  # Runs unit tests of WebKit Perl code.

+ use strict;

> +use File::Spec;
>  use FindBin;
>  use Test::Harness;
>  use lib $FindBin::Bin; # so this script can be run from any directory.

These two changes will require all variables to be declared (using 'my').  Sorry I didn't catch this earlier!

[Chris Jerdonek]

(In reply to comment #3)
> (From update of attachment 45869 [details])
> These changes look fine, but I noticed that the script isn't using strict mode
> and doesn't have warnings enabled:
> ...
> These two changes will require all variables to be declared (using 'my'). 
> Sorry I didn't catch this earlier!

No problem -- thanks for noticing it now!

[Chris Jerdonek]

Created attachment 45887 [details]
Proposed patch 2

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45887 [details] without any errors.

[David Kilzer (:ddkilzer)]

Comment on attachment 45887 [details]
Proposed patch 2

> Index: WebKitTools/Scripts/test-webkitperl
> ===================================================================
> --- WebKitTools/Scripts/test-webkitperl	(revision 52795)
> +++ WebKitTools/Scripts/test-webkitperl	(working copy)
> @@ -30,23 +30,20 @@
>  
>  # Runs unit tests of WebKit Perl code.
>  
> +use strict;
> +use warnings;

Adding "-w" to the "#!/usr/bin/perl" line actually catches slightly more issues than "use warnings" (IIRC), but I don't think it really matters for this script.

r=me

[WebKit Commit Bot]

Comment on attachment 45887 [details]
Proposed patch 2

Clearing flags on attachment: 45887

Committed r52817: <http://trac.webkit.org/changeset/52817>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Jerdonek]

(In reply to comment #7)

Sorry I missed that part of your comment.  Is "use warnings" redundant with "-w"?  The other Perl scripts seem to have both.

[David Kilzer (:ddkilzer)]

(In reply to comment #10)
> Sorry I missed that part of your comment.  Is "use warnings" redundant with
> "-w"?  The other Perl scripts seem to have both.

Having both is technically redundant, but doesn't hurt anything.  In this specific case, I think "-w" is equivalent to "use warnings", so it doesn't need both.

See "man warnings" and "man perllexwarn" for details.

[Eric Seidel (no email)]

http://build.webkit.org/builders/Leopard%20Intel%20Debug%20%28Tests%29/builds/8891 seems to think this change caused a regression.  I don't believe it.

[Eric Seidel (no email)]

Filed bug 33230 about the failures seen on the bot.  I don't think they're related to this change.