Summary: | W3C test importer should use filesystem instead of os.walk | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||
Component: | Tools / Tests | Assignee: | youenn fablet <youennf> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bjonesbe, buildbot, commit-queue, glenn, rniwa | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 142012, 142084 | ||||||||||
Bug Blocks: | 134764 | ||||||||||
Attachments: |
|
Description
youenn fablet
2015-02-27 07:16:56 PST
Created attachment 247514 [details]
Patch
Comment on attachment 247514 [details] Patch Attachment 247514 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4770151262060544 New failing tests: fast/css/object-fit/object-fit-canvas.html Created attachment 247656 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
(In reply to comment #2) > Comment on attachment 247514 [details] > Patch > > Attachment 247514 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.appspot.com/results/4770151262060544 > > New failing tests: > fast/css/object-fit/object-fit-canvas.html Putting r?/cq as this patch should have no impact on this particular test. Comment on attachment 247514 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=247514&action=review > Tools/Scripts/webkitpy/common/system/filesystem.py:80 > + def dirs_under(self, path, dir_filter=None): It would be nice to have a unit test for this. > Tools/Scripts/webkitpy/common/system/filesystem.py:93 > + for (dirpath, dirnames, filenames) in os.walk(path): If you wrap os.walk in a method (like _os_walk), then you only have to mock that method instead of copying this entire method into the MockFileSystem. Also, you'll be able to write unit tests for this method. > Tools/Scripts/webkitpy/common/system/filesystem_mock.py:179 > + def getsize(self, path): > + if not self.isfile(path): > + raise OSError("%s is not a directory" % path) > + return len(self.files[path]) Why did you add this? (In reply to comment #5) > Comment on attachment 247514 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=247514&action=review > > > Tools/Scripts/webkitpy/common/system/filesystem.py:80 > > + def dirs_under(self, path, dir_filter=None): > > It would be nice to have a unit test for this. Ok > > Tools/Scripts/webkitpy/common/system/filesystem.py:93 > > + for (dirpath, dirnames, filenames) in os.walk(path): > > If you wrap os.walk in a method (like _os_walk), then you only have to mock > that method instead of copying this entire method into the MockFileSystem. > Also, you'll be able to write unit tests for this method. I am not sure to understand. MockFileSystem is totally independent of FileSystem. We need to add MockFileSystem.dirs_under anyway. > > Tools/Scripts/webkitpy/common/system/filesystem_mock.py:179 > > + def getsize(self, path): > > + if not self.isfile(path): > > + raise OSError("%s is not a directory" % path) > > + return len(self.files[path]) > > Why did you add this? It is needed to run the unit test as it is used in TestImporter. Rereading it, there is obviously a typo in the exception description. Created attachment 247753 [details]
Adding unit tests
Comment on attachment 247753 [details]
Adding unit tests
Looks good. r=me.
Comment on attachment 247753 [details] Adding unit tests Clearing flags on attachment: 247753 Committed r181014: <http://trac.webkit.org/changeset/181014> All reviewed patches have been landed. Closing bug. |