Bug 135660

Summary: The support directory shouldn't be skipped unconditionally in test import
Product: WebKit Reporter: Bem Jones-Bey <bjonesbe>
Component: Tools / TestsAssignee: Bem Jones-Bey <bjonesbe>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dpranke, glenn, rhauck, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Bem Jones-Bey
Reported 2014-08-06 10:57:26 PDT
The support directory shouldn't be skipped unconditionally in test import
Attachments
Patch (1.71 KB, patch)
2014-08-06 11:00 PDT, Bem Jones-Bey 		no flags
DetailsFormatted DiffDiff
Patch (9.71 KB, patch)
2014-08-06 19:09 PDT, Bem Jones-Bey 		no flags
DetailsFormatted DiffDiff
Patch for landing (9.70 KB, patch)
2014-08-07 09:13 PDT, Bem Jones-Bey 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Bem Jones-Bey
Comment 1 2014-08-06 11:00:35 PDT
Created attachment 236116 [details] Patch
Bem Jones-Bey
Comment 2 2014-08-06 15:04:15 PDT
Comment on attachment 236116 [details] Patch I want to make a better solution.
Bem Jones-Bey
Comment 3 2014-08-06 19:09:02 PDT
Created attachment 236163 [details] Patch
Ryosuke Niwa
Comment 4 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 = ?
Bem Jones-Bey
Comment 5 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.
Bem Jones-Bey
Comment 6 2014-08-07 09:13:02 PDT
Created attachment 236189 [details] Patch for landing
WebKit Commit Bot
Comment 7 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>
WebKit Commit Bot
Comment 8 2014-08-07 09:38:19 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.