Summary: | Integrate v8 testing utility with webkit tests | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yaar Schnitman <yaar> | ||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, eric | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Yaar Schnitman
2010-04-16 15:15:14 PDT
Created attachment 53571 [details]
Patch
Comment on attachment 53571 [details]
Patch
Please cq+ if LGTM
Eric, this refactor of the original test utility can be run to see if output is different from the reference files and optionally, to update the reference files. I'll leave the hookup to EWS, commit queue, etc to you. Cheers, Yaar Comment on attachment 53571 [details]
Patch
+ os.system(' '.join(cmd))
os.system is a dangerous API. Please consider using popen or webkitpy.common.system.executive.
+ webkitRoot = os.path.join(os.path.dirname(__file__), '..', '..')
This seems very fragile. Please consider using checkout_root in webkitpy.common.checkout.scm.
+ workDir = tempfile.mkdtemp()
I don't understand why we need a working temp directory. Can't we just generate the new output into a memory buffer?
Created attachment 53687 [details]
Patch
Comment on attachment 53687 [details]
Patch
Ready for review.
(In reply to comment #4) > (From update of attachment 53571 [details]) > + os.system(' '.join(cmd)) > > os.system is a dangerous API. Please consider using popen or > webkitpy.common.system.executive. Fixed to use subprocess.call > + webkitRoot = os.path.join(os.path.dirname(__file__), '..', '..') > > This seems very fragile. Please consider using checkout_root in > webkitpy.common.checkout.scm. Fixed. > > + workDir = tempfile.mkdtemp() > > I don't understand why we need a working temp directory. Can't we just > generate the new output into a memory buffer? There are 3 reasons: 1) Its hard to run 'diff' on a memory buffer. 2) Its hard to predict pipe the output of the Perl generator into a memory buffer, as there is no agreed interface for how many files and what files it outputs. While I can hard code something for V8, I want this to generalize to other generators. 3) Its shorter, simpler code. (Please cq+ if ok) Comment on attachment 53687 [details]
Patch
Thanks for the patch! Glad to have this testing harness. :)
WebKitTools/Scripts/run-bindings-tests:105
+ overwrite = "--overwrite" in argv
We should probably use option parser here instead of doing this manually.
WebKitTools/Scripts/run-bindings-tests:82
+ workDir = tempfile.mkdtemp()
I still think we should do this in memory, but ok.
Comment on attachment 53687 [details] Patch Clearing flags on attachment: 53687 Committed r58143: <http://trac.webkit.org/changeset/58143> All reviewed patches have been landed. Closing bug. |