Currently run-javascriptcore-tests runs files wherever it finds them. We want it to be able to run tests on remote machines, which will require moving those test files around. A first step to this is for run-jsc-tests to package everything it needs into a tar file.
s/run-javascriptcore-tests/run-jsc-stress-tests/g
This feature will be enabled by the --tarball flag and will be disabled by default.
Created attachment 217363 [details] Patch
Comment on attachment 217363 [details] Patch r=me
Comment on attachment 217363 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217363&action=review R=me too > Tools/Scripts/run-jsc-stress-tests:318 > + self.addLayoutTestFiles These "self." things should be superfluous. Get rid of them if possible. > Tools/Scripts/run-jsc-stress-tests:330 > + system("mkdir", "-p", bundleTestPath.to_s) unless bundleTestPath.directory? Why not use the ruby mkdir API?
(In reply to comment #5) > (From update of attachment 217363 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=217363&action=review > > R=me too > > > Tools/Scripts/run-jsc-stress-tests:318 > > + self.addLayoutTestFiles > > These "self." things should be superfluous. Get rid of them if possible. Will do. > > > Tools/Scripts/run-jsc-stress-tests:330 > > + system("mkdir", "-p", bundleTestPath.to_s) unless bundleTestPath.directory? > > Why not use the ruby mkdir API? You mean the FileUtils mkdir_p thingy? No reason really.
Committed r159538: <http://trac.webkit.org/changeset/159538>
Re-opened since this is blocked by bug 124627
(In reply to comment #8) > Re-opened since this is blocked by bug 124627 Today is not my day :-(
Created attachment 217439 [details] Patch
(In reply to comment #10) > Created an attachment (id=217439) [details] > Patch The main difference is that the default Bundle class now uses WEBKIT_PATH as the base for all paths passed to localPathToBundlePath. This prevents run-jsc-stress-tests from getting confused when using LAYOUTTESTS_RELATIVE_PATH to setup the helper files for layout tests.
Comment on attachment 217439 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217439&action=review How does this handle extra files for Kraken, Octane, etc? > Tools/Scripts/run-jsc-stress-tests:289 > + attr_accessor :baseForExtraFiles > + def add(localTestPath, testName) I would put a blank line between attr_accessor and def add. > Tools/Scripts/run-jsc-stress-tests:317 > + self.addLayoutTestFiles Remove self. > Tools/Scripts/run-jsc-stress-tests:337 > + self.add(LAYOUTTESTS_RELATIVE_PATH + "resources", "standalone-pre.js") Remove self. > Tools/Scripts/run-jsc-stress-tests:338 > + self.add(LAYOUTTESTS_RELATIVE_PATH + "resources", "standalone-post.js") Remove self. > Tools/Scripts/run-jsc-stress-tests:344 > + self.add(@baseForExtraFiles, file) Remove self.
Am I understanding right that this is engineered around having all tests in a subdirectory of the WebKit checkout? That won't work for any of our internal tests.
(In reply to comment #13) > Am I understanding right that this is engineered around having all tests in a subdirectory of the WebKit checkout? > > That won't work for any of our internal tests. I don't think so. It's engineered around having all tests relative to the WebKit checkout, which I believe was already the case.
Created attachment 217452 [details] Patch
Comment on attachment 217452 [details] Patch After talking in person, this approach definitely still needs some work.
Created attachment 217698 [details] Patch
Comment on attachment 217698 [details] Patch We talked about it offline. Looks really good but it would be even better if run-jsc-stress-tests didn't hard-code yaml filenames.
Created attachment 217713 [details] Patch
Comment on attachment 217713 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217713&action=review > Tools/Scripts/run-jsc-stress-tests:355 > + if key == "DYLD_FRAMEWORK_PATH" > + script += "export #{key}=$(cd ../#{$frameworkPath.dirname}; pwd)\n" Change this to not be an IMPORTANT_ENV and handle it outside the loop. > Tools/Scripts/run-jsc-stress-tests:611 > + $extraFilesBaseDir = $extraFilesBaseDir.dirname unless $extraFilesBaseDir.basename == $benchmarkDirectory.basename Don't change this here.
I cc-ed some people responsible for other platforms since they'll need to make some adjustments if they want it to work for them.
Created attachment 217715 [details] Patch
Comment on attachment 217715 [details] Patch Clearing flags on attachment: 217715 Committed r159752: <http://trac.webkit.org/changeset/159752>
All reviewed patches have been landed. Closing bug.
(In reply to comment #23) > (From update of attachment 217715 [details]) > Clearing flags on attachment: 217715 > > Committed r159752: <http://trac.webkit.org/changeset/159752> FYI, it broke run-javascriptcore-tests script on all build.webkit.org bots: Error: #<LoadError: cannot load such file -- highline> Tools/Scripts/run-jsc-stress-tests:153:in `frameworkFromJSCPath': Unknown JSC path, cannot copy full framework: /Volumes/Data/slave/mavericks-release-tests-wk1/build/WebKitBuild/Release/jsc (RuntimeError) from Tools/Scripts/run-jsc-stress-tests:163:in `copyVMToBundle' from Tools/Scripts/run-jsc-stress-tests:822:in `prepareBundle' from Tools/Scripts/run-jsc-stress-tests:1056:in `<main>'
(In reply to comment #25) > (In reply to comment #23) > > (From update of attachment 217715 [details] [details]) > > Clearing flags on attachment: 217715 > > > > Committed r159752: <http://trac.webkit.org/changeset/159752> > > FYI, it broke run-javascriptcore-tests script on all build.webkit.org bots: > > Error: #<LoadError: cannot load such file -- highline> > Tools/Scripts/run-jsc-stress-tests:153:in `frameworkFromJSCPath': Unknown JSC path, cannot copy full framework: /Volumes/Data/slave/mavericks-release-tests-wk1/build/WebKitBuild/Release/jsc (RuntimeError) > from Tools/Scripts/run-jsc-stress-tests:163:in `copyVMToBundle' > from Tools/Scripts/run-jsc-stress-tests:822:in `prepareBundle' > from Tools/Scripts/run-jsc-stress-tests:1056:in `<main>' Fix is on the way.
> Fix is on the way. Fix landed in http://trac.webkit.org/changeset/159754.
Re-opened since this is blocked by bug 124847
(In reply to comment #28) > Re-opened since this is blocked by bug 124847 This broke the linux test bots. Ossy has agreed to help adjust the script so that it works for the linux bots as well.
I tried to debug r159752-r159754 changes on Linux, but it seems the problem is bigger than I thought before. The whole "framework" is a Mac only thing, it doesn't exist on Linux at all, that's why frameworkFromJSCPath() is broken, and then setting $frameworkPath and $jscPath based on the this premise that framework exists and jsc is placed in $frameworkPath/Resources/jsc. archiveBuiltProduct() in Tools/BuildSlaveSupport/built-product-archive would be a good start to bundle everything properly on all ports. Of course it packs everything for layout testing which we don't need for running only jsc tests. But it seems it works only on GTK now (but packs jsc-stress-results unnecessarily!), EFL doesn't use it, because they have builder-tester bots only, not separated builder and tester. But I still wondering how it works, because it doesn't pack the monumental but necessary WebKitBuild/Dependencies. It's more complex than I thought. :( I propose keeping the actual behaviour of jsc-stress-tests script and create bundle if only --tarball is passed until we find the proper way creating bundle on Linux. ( as Mark mentioned in https://bugs.webkit.org/show_bug.cgi?id=124549#c2 )
Created attachment 218202 [details] Patch
(In reply to comment #31) > Created an attachment (id=218202) [details] > Patch I modified the script to artificially create the directory structure it expects, even on platforms that don't normally have a JavaScriptCore.framework directory.
(In reply to comment #32) > (In reply to comment #31) > > Created an attachment (id=218202) [details] [details] > > Patch > > I modified the script to artificially create the directory structure it expects, even on platforms that don't normally have a JavaScriptCore.framework directory. For those reviewing, the changes are in copyVMToBundle and prepareFramework
Comment on attachment 218202 [details] Patch Clearing flags on attachment: 218202 Committed r159955: <http://trac.webkit.org/changeset/159955>