BuildAndTestFactory in Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg contains code duplication from TestFactory, and BuildAndTestWebKit2Factory is unused. Patch Forthcoming.
Created attachment 247779 [details] Patch
Comment on attachment 247779 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=247779&action=review Some comments added, otherwise looks good to me. > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:806 > self.addStep(RunEflAPITests) RunEflAPITests() similar to CompileClass() below. > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:-811 > - CompileClass = CompileWebKit > - LayoutTestClass = RunWebKitTests > - ExtractTestResultsClass = ExtractTestResults Good catch, it seems CompileClass and ExtractTestResultsClass are really useless. > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:813 > + self.addStep(CompileWebKit) Use self.addStep(CompileWebKit()) , because passing BuildStep subclass is deprecated. > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:815 > + def __init__(self, *args, triggers=None, **kwargs): self, *args, triggers=None, **kwargs --> self, triggers=None, *args, **kwargs > Tools/ChangeLog:15 > + (BuildAndTestWebKit2Factory.getProduct): Deleted. Wasn't used anywhere. getProduct? I know it is generated, but it can be fixed manually.
(In reply to comment #2) > > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:815 > > + def __init__(self, *args, triggers=None, **kwargs): > > self, *args, triggers=None, **kwargs --> self, triggers=None, *args, **kwargs Is triggers being used as a keyword argument? If so, putting it at the beginning will disable any non keyword arguments being passed in. Otherwise, if it isn't, this change doesn't work at all, since it changes the order of args.
Created attachment 247785 [details] Patch
(In reply to comment #2) > > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:815 > > + def __init__(self, *args, triggers=None, **kwargs): > > self, *args, triggers=None, **kwargs --> self, triggers=None, *args, **kwarg I decided to just revert this.
Comment on attachment 247785 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=247785&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:814 > + self.addStep(CompileWebKit()) > def __init__(self, platform, configuration, architectures, triggers=None, additionalArguments=None, SVNMirror=None, **kwargs): please add a newline > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:815 > + TestFactory.__init__(self, platform, configuration, architectures, False, additionalArguments, SVNMirror, **kwargs) without False, TestFactory.__init__ has only 6 arguments.
Created attachment 247798 [details] Patch
Comment on attachment 247798 [details] Patch Rejecting attachment 247798 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 247798, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: fs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 180958 = ba5f5701f3d812202858234b65effa39dbd3461b r180959 = 0011a228a1f200b552f03bd2fcd3b09c02fc00a3 r180960 = a0a2f39cf8dd053e9648572ffaa30ae0cf423d0d r180961 = 06addba3300bbc41107c7a4233ed9bac2bb510bf Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.appspot.com/results/6625932274040832
It seems it needs to be rebased before landing.
(In reply to comment #9) > It seems it needs to be rebased before landing. Are you going to rebase and land it in the near future?
(In reply to comment #10) > (In reply to comment #9) > > It seems it needs to be rebased before landing. > > Are you going to rebase and land it in the near future? Yes, I didn't notice this didn't get commit queued.
(In reply to comment #10) > (In reply to comment #9) > > It seems it needs to be rebased before landing. > > Are you going to rebase and land it in the near future? (In reply to comment #10) > (In reply to comment #9) > > It seems it needs to be rebased before landing. > > Are you going to rebase and land it in the near future? nvm, I had already landed it, and forgot to close it. http://trac.webkit.org/changeset/180999
Comment on attachment 247798 [details] Patch Landed in http://trac.webkit.org/changeset/180999