Integrate v8 testing utility with webkit tests
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.