Add support for remotes file for EWS builders
Created attachment 382925 [details] Patch
Comment on attachment 382925 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382925&action=review Logic looks fine. Please fix the unit-tests as well. You can run them using 'python Tools/BuildSlaveSupport/ews-build/runUnittests.py' > Tools/BuildSlaveSupport/ews-build/steps.py:929 > + command = ['perl', 'Tools/Scripts/run-javascriptcore-tests', '--no-build', '--no-fail-fast', '--json-output={0}'.format(jsonFileName), WithProperties('--%(configuration)s')] + [WithProperties('--remote-config-file=%(remotes)s')] if WithProperties('%(remotes)s') != '' else [] is this if-else correct? Please check what does command contains in case of else.
Thanks for the comments - I will take a look at the unit tests. Didn't know we had them.
python Tools/BuildSlaveSupport/ews-build/runUnittests.py E....................... ====================================================================== ERROR: steps_unittest (unittest.loader.ModuleImportFailure) ---------------------------------------------------------------------- ImportError: Failed to import test module: steps_unittest Traceback (most recent call last): File "/usr/lib64/python2.7/unittest/loader.py", line 254, in _find_tests module = self._get_module_from_name(name) File "/usr/lib64/python2.7/unittest/loader.py", line 232, in _get_module_from_name __import__(name) File "/home/pmatos/Projects/igalia/WebKit/Tools/BuildSlaveSupport/ews-build/steps_unittest.py", line 30, in <module> from buildbot.test.fake.remotecommand import Expect, ExpectRemoteRef, ExpectShell ImportError: No module named test.fake.remotecommand Are you using buildbot 1.8.2?
(In reply to Aakash Jain from comment #2) > Comment on attachment 382925 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=382925&action=review > > Logic looks fine. > > Please fix the unit-tests as well. You can run them using 'python > Tools/BuildSlaveSupport/ews-build/runUnittests.py' > No updates seem to be required as the unittests don't call into ConfigureBuild.
Created attachment 383035 [details] Patch
Created attachment 383037 [details] Patch
Created attachment 383038 [details] Patch
I am having some issues getting ahold of the property in the RunJavaScriptCoreTests step. Need to understand the best way to do this.
Comment on attachment 383038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383038&action=review > Tools/BuildSlaveSupport/ews-build/steps.py:937 > + # add remotes configuration file path to the command line if needed start() is a better place to add parameters to self.command. If I remember correctly, you will need to use self.getProperty('remotes') instead of WithProperties/util.Property().
(In reply to Paulo Matos from comment #4) > Are you using buildbot 1.8.2? Yes, see https://ews-build.webkit.org/#/about Did you used '--no-binary :all:' while installing buildbot, as per: https://trac.webkit.org/wiki/EarlyWarningSystem#Installingrequiredpackages: (In reply to Paulo Matos from comment #5) > No updates seem to be required as the unittests don't call into ConfigureBuild. The unit-test failures are in RunJavaScriptCoreTests (and related) unit-tests. See the red services ews bubble https://ews-build.webkit.org/#/builders/20/builds/7508
(In reply to Paulo Matos from comment #9) > I am having some issues getting ahold of the property in the > RunJavaScriptCoreTests step. Need to understand the best way to do this. s __init__ is called at the very beginning, when buildbot starts. While start() is called after the build, and the corresponding build-step has started. So at the time start() is called, all the build properties are already set, and you can simply use self.getProperty(). We modify self.command inside start() in our current code as well, e.g.: in CompileWebKit::start(), and even inside RunJavaScriptCoreTests:: start() using appendCustomBuildFlags().
Created attachment 383042 [details] Patch
Created attachment 383044 [details] Patch
(In reply to Aakash Jain from comment #12) > (In reply to Paulo Matos from comment #9) > > I am having some issues getting ahold of the property in the > > RunJavaScriptCoreTests step. Need to understand the best way to do this. > s > __init__ is called at the very beginning, when buildbot starts. While > start() is called after the build, and the corresponding build-step has > started. So at the time start() is called, all the build properties are > already set, and you can simply use self.getProperty(). > > We modify self.command inside start() in our current code as well, e.g.: in > CompileWebKit::start(), and even inside RunJavaScriptCoreTests:: start() > using appendCustomBuildFlags(). Thanks for the quick review and comments. r+ and cq+ pls if all is good.
(In reply to Paulo Matos from comment #15) > Thanks for the quick review and comments. r+ and cq+ pls if all is good. Code looks good. Can you just move the unit-tests inside TestRunJavaScriptCoreTests? Usually we have a class in steps_unittest corresponding to a class in steps.
Created attachment 383047 [details] Patch
Comment on attachment 383047 [details] Patch Clearing flags on attachment: 383047 Committed r252184: <https://trac.webkit.org/changeset/252184>
All reviewed patches have been landed. Closing bug.
<rdar://problem/56983361>
We need to pass 'remotes' parameter in JSCTestsFactory.__init__() as well. Also, it's probably better to use named arguments while using Factory.__init(). e.g.: class JSCTestsFactory(Factory): def __init__(self, platform, configuration='release', architectures=None, remotes=None, additionalArguments=None, runTests='true', **kwargs): Factory.__init__(self, platform=platform, configuration=configuration, architectures=architectures, buildOnly=False, remotes=remotes, additionalArguments=additionalArguments, checkRelevance=True)
(In reply to Aakash Jain from comment #21) > We need to pass 'remotes' parameter in JSCTestsFactory.__init__() as well. Done in https://trac.webkit.org/changeset/252454/webkit