Bug 135660 – The support directory shouldn't be skipped unconditionally in test import

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

[Bem Jones-Bey]

The support directory shouldn't be skipped unconditionally in test import

[Bem Jones-Bey]

Created attachment 236116 [details]
Patch

[Bem Jones-Bey]

Comment on attachment 236116 [details]
Patch

I want to make a better solution.

[Bem Jones-Bey]

Created attachment 236163 [details]
Patch

[Ryosuke Niwa]

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 = ?

[Bem Jones-Bey]

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.

[Bem Jones-Bey]

Created attachment 236189 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 236189 [details]
Patch for landing

Clearing flags on attachment: 236189

Committed r172214: <http://trac.webkit.org/changeset/172214>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.