Bug 142012

Summary: W3C importer should use filesystem instead of shutil/host
Product: WebKit Reporter: youenn fablet <youennf>
Component: Tools / TestsAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: bjonesbe, commit-queue, glenn
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 142085    
Attachments:
Description Flags
Patch
none
Patch for landing none

youenn fablet
Reported 2015-02-25 01:23:32 PST
Tools/Scripts/w3c/test_importer.py should use Host abstraction in lieu of shutil, os...
Attachments
Patch (9.71 KB, patch)
2015-02-25 01:27 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for landing (9.39 KB, patch)
2015-02-28 13:52 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
youenn fablet
Comment 1 2015-02-25 01:27:28 PST
Created attachment 247322 [details] Patch
Bem Jones-Bey
Comment 2 2015-02-25 10:08:11 PST
Comment on attachment 247322 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=247322&action=review Does this enable the creation of any new unit tests, or do we have to wait until os.walk is replaced with something mockable? > Tools/Scripts/webkitpy/w3c/test_importer.py:94 > + if not FileSystem().exists(import_dir): Nit: put FileSystem() in a local variable and use that here.
youenn fablet
Comment 3 2015-02-25 12:23:26 PST
(In reply to comment #2) > Comment on attachment 247322 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=247322&action=review > > Does this enable the creation of any new unit tests, or do we have to wait > until os.walk is replaced with something mockable? Good point. We could already test quite some functionality by creating dedicated TestImporter.import_list and running TestImporter.import_tests. But these tests may need to be updated whenever we change import_list/import_tests. I would tend to wait until replacing os.walk to create black-box tests. > > Tools/Scripts/webkitpy/w3c/test_importer.py:94 > > + if not FileSystem().exists(import_dir): > > Nit: put FileSystem() in a local variable and use that here. OK
youenn fablet
Comment 4 2015-02-27 07:48:48 PST
(In reply to comment #3) > (In reply to comment #2) > > Comment on attachment 247322 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=247322&action=review > > > > Does this enable the creation of any new unit tests, or do we have to wait > > until os.walk is replaced with something mockable? > > Good point. > We could already test quite some functionality by creating dedicated > TestImporter.import_list and running TestImporter.import_tests. > But these tests may need to be updated whenever we change > import_list/import_tests. > I would tend to wait until replacing os.walk to create black-box tests. With patches from bug 142084 and bug 142085, test importer becomes testable with the mock system. Patch in bug 142085 adds one such unit test to ensure that.
Bem Jones-Bey
Comment 5 2015-02-27 17:25:49 PST
Comment on attachment 247322 [details] Patch Good stuff. r=me.
youenn fablet
Comment 6 2015-02-28 13:52:35 PST
Created attachment 247612 [details] Patch for landing
WebKit Commit Bot
Comment 7 2015-02-28 14:48:01 PST
Comment on attachment 247612 [details] Patch for landing Clearing flags on attachment: 247612 Committed r180844: <http://trac.webkit.org/changeset/180844>
WebKit Commit Bot
Comment 8 2015-02-28 14:48:05 PST
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.