Bug 135660 - The support directory shouldn't be skipped unconditionally in test import
Summary: The support directory shouldn't be skipped unconditionally in test import
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Bem Jones-Bey
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-06 10:57 PDT by Bem Jones-Bey
Modified: 2014-08-07 09:38 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.71 KB, patch)
2014-08-06 11:00 PDT, Bem Jones-Bey
no flags Details | Formatted Diff | Diff
Patch (9.71 KB, patch)
2014-08-06 19:09 PDT, Bem Jones-Bey
no flags Details | Formatted Diff | Diff
Patch for landing (9.70 KB, patch)
2014-08-07 09:13 PDT, Bem Jones-Bey
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bem Jones-Bey 2014-08-06 10:57:26 PDT
The support directory shouldn't be skipped unconditionally in test import
Comment 1 Bem Jones-Bey 2014-08-06 11:00:35 PDT
Created attachment 236116 [details]
Patch
Comment 2 Bem Jones-Bey 2014-08-06 15:04:15 PDT
Comment on attachment 236116 [details]
Patch

I want to make a better solution.
Comment 3 Bem Jones-Bey 2014-08-06 19:09:02 PDT
Created attachment 236163 [details]
Patch
Comment 4 Ryosuke Niwa 2014-08-06 21:36:05 PDT
Comment on attachment 236163 [details]
Patch

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

> Tools/ChangeLog:12
> +        '.'.

Please put this in the previous line.
It's so awkward to start a new line with '.'.

> Tools/Scripts/webkitpy/w3c/test_importer.py:175
> +        should_skip = (subdir.startswith('.') or (root == self.source_directory and subdir in DIRS_TO_SKIP))

We don't need the outer parenthesis.

> Tools/Scripts/webkitpy/w3c/test_importer.py:186
> +            dirs[:] = [subdir for subdir in dirs if self.should_keep_subdir(root, subdir)]

why not just dirs = ?
Comment 5 Bem Jones-Bey 2014-08-07 09:08:50 PDT
Comment on attachment 236163 [details]
Patch

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

Thanks for the review!

>> Tools/ChangeLog:12
>> +        '.'.
> 
> Please put this in the previous line.
> It's so awkward to start a new line with '.'.

ok.

>> Tools/Scripts/webkitpy/w3c/test_importer.py:175
>> +        should_skip = (subdir.startswith('.') or (root == self.source_directory and subdir in DIRS_TO_SKIP))
> 
> We don't need the outer parenthesis.

ok.

>> Tools/Scripts/webkitpy/w3c/test_importer.py:186
>> +            dirs[:] = [subdir for subdir in dirs if self.should_keep_subdir(root, subdir)]
> 
> why not just dirs = ?

Because the API for os.walk is strange, and dirs is a special variable. If you want to exclude directories from the walk, you need to modify dirs in place.
Comment 6 Bem Jones-Bey 2014-08-07 09:13:02 PDT
Created attachment 236189 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2014-08-07 09:38:16 PDT
Comment on attachment 236189 [details]
Patch for landing

Clearing flags on attachment: 236189

Committed r172214: <http://trac.webkit.org/changeset/172214>
Comment 8 WebKit Commit Bot 2014-08-07 09:38:19 PDT
All reviewed patches have been landed.  Closing bug.