Attachment 343413 Details for Bug 186955 – Proposed patch

[patch] Proposed patch

ews_stylequeue.patch (text/plain), 19.28 KB, created by Aakash Jain on 2018-06-22 18:45:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Aakash Jain

Created: 2018-06-22 18:45:59 PDT

Size: 19.28 KB

patch

obsolete

>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 233118)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+2018-06-22  Aakash Jain  <aakash_jain@apple.com>
>+
>+        [ews-build] Add support for Style-EWS
>+        https://bugs.webkit.org/show_bug.cgi?id=186955
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * BuildSlaveSupport/ews-build/factories.py:
>+        (Factory): Base class for all the factory.
>+        (StyleFactory): Class CheckStyle build step.
>+        * BuildSlaveSupport/ews-build/loadConfig.py:
>+        (loadBuilderConfig): Initialize factory with required parameters.
>+        * BuildSlaveSupport/ews-build/runUnittests.py: Added, script to run all the unit tests.
>+        * BuildSlaveSupport/ews-build/steps.py: Added.
>+        * BuildSlaveSupport/ews-build/steps_unittest.py: Added unit-tests.
>+
> 2018-06-22  Brady Eidson  <beidson@apple.com>
> 
>         WKURLSchemeHandler doesn't handle sync XHR.
>Index: Tools/BuildSlaveSupport/ews-build/factories.py
>===================================================================
>--- Tools/BuildSlaveSupport/ews-build/factories.py	(revision 233117)
>+++ Tools/BuildSlaveSupport/ews-build/factories.py	(working copy)
>@@ -23,50 +23,61 @@
> 
> from buildbot.process import factory
> 
>+from steps import *
> 
>-class StyleFactory(factory.BuildFactory):
>-    pass
>+
>+class Factory(factory.BuildFactory):
>+    def __init__(self, platform, configuration=None, architectures=None, buildOnly=True, additionalArguments=None, **kwargs):
>+        factory.BuildFactory.__init__(self)
>+        self.addStep(ConfigureBuild(platform, configuration, architectures, buildOnly, additionalArguments))
>+        self.addStep(CheckOutSource())
>+
>+
>+class StyleFactory(Factory):
>+    def __init__(self, platform, configuration=None, architectures=None, additionalArguments=None, **kwargs):
>+        Factory.__init__(self, platform, configuration, architectures, False, additionalArguments)
>+        self.addStep(CheckStyle())
> 
> 
>-class GTKFactory(factory.BuildFactory):
>+class GTKFactory(Factory):
>     pass
> 
> 
>-class iOSFactory(factory.BuildFactory):
>+class iOSFactory(Factory):
>     pass
> 
> 
>-class iOSSimulatorFactory(factory.BuildFactory):
>+class iOSSimulatorFactory(Factory):
>     pass
> 
> 
>-class MacWK1Factory(factory.BuildFactory):
>+class MacWK1Factory(Factory):
>     pass
> 
> 
>-class MacWK2Factory(factory.BuildFactory):
>+class MacWK2Factory(Factory):
>     pass
> 
> 
>-class WindowsFactory(factory.BuildFactory):
>+class WindowsFactory(Factory):
>     pass
> 
> 
>-class WinCairoFactory(factory.BuildFactory):
>+class WinCairoFactory(Factory):
>     pass
> 
> 
>-class WPEFactory(factory.BuildFactory):
>+class WPEFactory(Factory):
>     pass
> 
> 
>-class JSCTestsFactory(factory.BuildFactory):
>+class JSCTestsFactory(Factory):
>     pass
> 
> 
>-class BindingsFactory(factory.BuildFactory):
>+class BindingsFactory(Factory):
>     pass
> 
> 
>-class WebkitpyFactory(factory.BuildFactory):
>+class WebkitpyFactory(Factory):
>     pass
>Index: Tools/BuildSlaveSupport/ews-build/loadConfig.py
>===================================================================
>--- Tools/BuildSlaveSupport/ews-build/loadConfig.py	(revision 233117)
>+++ Tools/BuildSlaveSupport/ews-build/loadConfig.py	(working copy)
>@@ -49,10 +49,13 @@ def loadBuilderConfig(c, use_localhost_w
>     for builder in config['builders']:
>         builder['tags'] = getTagsForBuilder(builder)
>         factory = globals()[builder['factory']]
>-        builder['factory'] = factory()
>-        del builder['platform']
>-        if 'configuration' in builder:
>-            del builder['configuration']
>+        factorykwargs = {}
>+        for key in ["platform", "configuration", "architectures", "triggers", "additionalArguments"]:
>+            value = builder.pop(key, None)
>+            if value:
>+                factorykwargs[key] = value
>+
>+        builder["factory"] = factory(**factorykwargs)
> 
>         if use_localhost_worker:
>             builder['workernames'].append("local-worker")
>Index: Tools/BuildSlaveSupport/ews-build/runUnittests.py
>===================================================================
>--- Tools/BuildSlaveSupport/ews-build/runUnittests.py	(nonexistent)
>+++ Tools/BuildSlaveSupport/ews-build/runUnittests.py	(working copy)
>@@ -0,0 +1,30 @@
>+#!/usr/bin/python
>+
>+import os
>+import sys
>+import unittest
>+
>+"""
>+This is the equivalent of running:
>+    python -m unittest discover --start-directory {test_discovery_path} --pattern {UNIT_TEST_PATTERN}
>+"""
>+
>+UNIT_TEST_PATTERN = '*_unittest.py'
>+
>+
>+def run_unittests(test_discovery_path):
>+    test_suite = unittest.defaultTestLoader.discover(test_discovery_path, pattern=UNIT_TEST_PATTERN)
>+    results = unittest.TextTestRunner(buffer=True).run(test_suite)
>+    if results.failures or results.errors:
>+        raise RuntimeError('Test failures or errors were generated during this test run.')
>+
>+
>+if __name__ == '__main__':
>+    try:
>+        relative_path = sys.argv[1]
>+    except IndexError:
>+        relative_path = os.path.dirname(__file__)
>+
>+    path = os.path.abspath(relative_path)
>+    assert os.path.isdir(path), '{} is not a directory. Please specify a valid directory'.format(path)
>+    run_unittests(path)
>
>Property changes on: Tools/BuildSlaveSupport/ews-build/runUnittests.py
>___________________________________________________________________
>Added: svn:executable
>## -0,0 +1 ##
>+*
>\ No newline at end of property
>Index: Tools/BuildSlaveSupport/ews-build/steps.py
>===================================================================
>--- Tools/BuildSlaveSupport/ews-build/steps.py	(nonexistent)
>+++ Tools/BuildSlaveSupport/ews-build/steps.py	(working copy)
>@@ -0,0 +1,73 @@
>+# Copyright (C) 2018 Apple Inc. All rights reserved.
>+#
>+# Redistribution and use in source and binary forms, with or without
>+# modification, are permitted provided that the following conditions
>+# are met:
>+# 1.  Redistributions of source code must retain the above copyright
>+#     notice, this list of conditions and the following disclaimer.
>+# 2.  Redistributions in binary form must reproduce the above copyright
>+#     notice, this list of conditions and the following disclaimer in the
>+#     documentation and/or other materials provided with the distribution.
>+#
>+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND
>+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
>+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
>+# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR
>+# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
>+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
>+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
>+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+
>+from buildbot.process import buildstep
>+from buildbot.process.results import Results, SUCCESS, FAILURE, WARNINGS, SKIPPED, EXCEPTION, RETRY
>+from buildbot.steps import shell
>+from buildbot.steps.source import svn
>+from twisted.internet import defer
>+
>+
>+class ConfigureBuild(buildstep.BuildStep):
>+    name = "configure-build"
>+    description = ["configuring build"]
>+    descriptionDone = ["configured build"]
>+
>+    def __init__(self, platform, configuration, architectures, buildOnly, additionalArguments):
>+        super(ConfigureBuild, self).__init__()
>+        self.platform = platform
>+        if platform != 'jsc-only':
>+            self.platform = platform.split('-', 1)[0]
>+        self.fullPlatform = platform
>+        self.configuration = configuration
>+        self.architecture = " ".join(architectures) if architectures else None
>+        self.buildOnly = buildOnly
>+        self.additionalArguments = additionalArguments
>+
>+    def start(self):
>+        self.setProperty("platform", self.platform)
>+        self.setProperty("fullPlatform", self.fullPlatform)
>+        self.setProperty("configuration", self.configuration)
>+        self.setProperty("architecture", self.architecture)
>+        self.setProperty("buildOnly", self.buildOnly)
>+        self.setProperty("additionalArguments", self.additionalArguments)
>+        self.finished(SUCCESS)
>+        return defer.succeed(None)
>+
>+
>+class CheckOutSource(svn.SVN):
>+    CHECKOUT_DELAY_AND_MAX_RETRIES_PAIR = (0, 2)
>+
>+    def __init__(self, **kwargs):
>+        self.repourl = 'https://svn.webkit.org/repository/webkit/trunk'
>+        super(CheckOutSource, self).__init__(repourl=self.repourl,
>+                                                retry=self.CHECKOUT_DELAY_AND_MAX_RETRIES_PAIR,
>+                                                preferLastChangedRev=True,
>+                                                **kwargs)
>+
>+
>+class CheckStyle(shell.ShellCommand):
>+    name = 'check-webkit-style'
>+    description = ['check-webkit-style running']
>+    descriptionDone = ['check-webkit-style']
>+    flunkOnFailure = True
>+    command = ['Tools/Scripts/check-webkit-style']
>Index: Tools/BuildSlaveSupport/ews-build/steps_unittest.py
>===================================================================
>--- Tools/BuildSlaveSupport/ews-build/steps_unittest.py	(nonexistent)
>+++ Tools/BuildSlaveSupport/ews-build/steps_unittest.py	(working copy)
>@@ -0,0 +1,231 @@
>+# Copyright (C) 2018 Apple Inc. All rights reserved.
>+#
>+# Redistribution and use in source and binary forms, with or without
>+# modification, are permitted provided that the following conditions
>+# are met:
>+# 1.  Redistributions of source code must retain the above copyright
>+#     notice, this list of conditions and the following disclaimer.
>+# 2.  Redistributions in binary form must reproduce the above copyright
>+#     notice, this list of conditions and the following disclaimer in the
>+#     documentation and/or other materials provided with the distribution.
>+#
>+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND
>+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
>+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
>+# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR
>+# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
>+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
>+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
>+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+
>+import operator
>+import os
>+import shutil
>+import tempfile
>+
>+from buildbot.test.fake.remotecommand import ExpectShell
>+from buildbot.test.util.steps import BuildStepMixin
>+from twisted.internet import error, reactor
>+from twisted.python import failure, log
>+from twisted.trial import unittest
>+
>+from steps import *
>+
>+
>+class BuildStepMixinAdditions(BuildStepMixin):
>+    def setUpBuildStep(self):
>+        self.patch(reactor, 'spawnProcess', lambda *args, **kwargs: self._checkSpawnProcess(*args, **kwargs))
>+        self._expected_local_commands = []
>+
>+        self._temp_directory = tempfile.mkdtemp()
>+        os.chdir(self._temp_directory)
>+        self._expected_uploaded_files = []
>+
>+        super(BuildStepMixinAdditions, self).setUpBuildStep()
>+
>+    def tearDownBuildStep(self):
>+        shutil.rmtree(self._temp_directory)
>+        super(BuildStepMixinAdditions, self).tearDownBuildStep()
>+
>+    def fakeBuildFinished(self, text, results):
>+        self.build.text = text
>+        self.build.results = results
>+
>+    def setupStep(self, step, *args, **kwargs):
>+        self.previous_steps = kwargs.get('previous_steps') or []
>+        if self.previous_steps:
>+            del kwargs['previous_steps']
>+
>+        super(BuildStepMixinAdditions, self).setupStep(step, *args, **kwargs)
>+        self.build.terminate = False
>+        self.build.stopped = False
>+        self.build.executedSteps = self.executedSteps
>+        self.build.buildFinished = self.fakeBuildFinished
>+        self._expected_added_urls = []
>+        self._expected_sources = None
>+
>+    @property
>+    def executedSteps(self):
>+        return filter(lambda step: not step.stopped, self.previous_steps)
>+
>+    def setProperty(self, name, value, source='Unknown'):
>+        self.properties.setProperty(name, value, source)
>+
>+    def expectAddedURLs(self, added_urls):
>+        self._expected_added_urls = added_urls
>+
>+    def expectUploadedFile(self, path):
>+        self._expected_uploaded_files.append(path)
>+
>+    def expectLocalCommands(self, *expected_commands):
>+        self._expected_local_commands.extend(expected_commands)
>+
>+    def expectRemoteCommands(self, *expected_commands):
>+        self.expectCommands(*expected_commands)
>+
>+    def expectSources(self, expected_sources):
>+        self._expected_sources = expected_sources
>+
>+    def _checkSpawnProcess(self, processProtocol, executable, args, env, path, usePTY, **kwargs):
>+        got = (executable, args, env, path, usePTY)
>+        if not self._expected_local_commands:
>+            self.fail('got local command {0} when no further commands were expected'.format(got))
>+        local_command = self._expected_local_commands.pop(0)
>+        try:
>+            self.assertEqual(got, (local_command.args[0], local_command.args, local_command.env, local_command.path, local_command.usePTY))
>+        except AssertionError:
>+            log.err()
>+            raise
>+        for name, value in local_command.logs:
>+            if name == 'stdout':
>+                processProtocol.outReceived(value)
>+            elif name == 'stderr':
>+                processProtocol.errReceived(value)
>+        if local_command.rc != 0:
>+            value = error.ProcessTerminated(exitCode=local_command.rc)
>+        else:
>+            value = error.ProcessDone(None)
>+        processProtocol.processEnded(failure.Failure(value))
>+
>+    def _added_files(self):
>+        results = []
>+        for dirpath, dirnames, filenames in os.walk(self._temp_directory):
>+            relative_root_path = os.path.relpath(dirpath, start=self._temp_directory)
>+            if relative_root_path == '.':
>+                relative_root_path = ''
>+            for name in filenames:
>+                results.append(os.path.join(relative_root_path, name))
>+        return results
>+
>+    def runStep(self):
>+        def check(result):
>+            self.assertEqual(self._expected_local_commands, [], 'assert all expected local commands were run')
>+            self.expectAddedURLs(self._expected_added_urls)
>+            self.assertEqual(self._added_files(), self._expected_uploaded_files)
>+            if self._expected_sources is not None:
>+                # Convert to dictionaries because assertEqual() only knows how to diff Python built-in types.
>+                actual_sources = sorted([source.asDict() for source in self.build.sources], key=operator.itemgetter('codebase'))
>+                expected_sources = sorted([source.asDict() for source in self._expected_sources], key=operator.itemgetter('codebase'))
>+                self.assertEqual(actual_sources, expected_sources)
>+        deferred_result = super(BuildStepMixinAdditions, self).runStep()
>+        deferred_result.addCallback(check)
>+        return deferred_result
>+
>+
>+class TestCheckStyle(BuildStepMixinAdditions, unittest.TestCase):
>+    def setUp(self):
>+        self.longMessage = True
>+        return self.setUpBuildStep()
>+
>+    def tearDown(self):
>+        return self.tearDownBuildStep()
>+
>+    def test_success_internal(self):
>+        self.setupStep(CheckStyle())
>+        self.setProperty('try-codebase', 'internal')
>+        self.setProperty('platform', 'mac')
>+        self.setProperty('configuration', 'Debug')
>+
>+        self.expectRemoteCommands(
>+            ExpectShell(workdir='wkdir',
>+                        command=['Tools/Scripts/check-webkit-style'],
>+                        )
>+            + 0,
>+        )
>+        self.expectOutcome(result=SUCCESS, state_string='check-webkit-style')
>+        return self.runStep()
>+
>+    def test_failure_unknown_try_codebase(self):
>+        self.setupStep(CheckStyle())
>+        self.setProperty('try-codebase', 'foo')
>+        self.setProperty('platform', 'mac')
>+        self.setProperty('configuration', 'Debug')
>+
>+        self.expectRemoteCommands(
>+            ExpectShell(workdir='wkdir',
>+                        command=['Tools/Scripts/check-webkit-style'],
>+                        )
>+            + 2,
>+        )
>+        self.expectOutcome(result=FAILURE, state_string='check-webkit-style (failure)')
>+        return self.runStep()
>+
>+    def test_failures_with_style_issues(self):
>+        self.setupStep(CheckStyle())
>+        self.setProperty('try-codebase', 'internal')
>+        self.setProperty('platform', 'mac')
>+        self.setProperty('configuration', 'Debug')
>+
>+        self.expectRemoteCommands(
>+            ExpectShell(workdir='wkdir',
>+                        command=['Tools/Scripts/check-webkit-style'],
>+                        )
>+            + ExpectShell.log('stdio', stdout='''ERROR: Source/WebCore/layout/FloatingContext.cpp:36:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
>+ERROR: Source/WebCore/layout/FormattingContext.h:94:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
>+ERROR: Source/WebCore/layout/LayoutContext.cpp:52:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
>+ERROR: Source/WebCore/layout/LayoutContext.cpp:55:  Extra space before last semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
>+ERROR: Source/WebCore/layout/LayoutContext.cpp:60:  Tab found; better to use spaces  [whitespace/tab] [1]
>+ERROR: Source/WebCore/layout/Verification.cpp:88:  Missing space before ( in while(  [whitespace/parens] [5]
>+Total errors found: 8 in 48 files''')
>+            + 2,
>+        )
>+        self.expectOutcome(result=FAILURE, state_string='check-webkit-style (failure)')
>+        return self.runStep()
>+
>+    def test_failures_no_style_issues(self):
>+        self.setupStep(CheckStyle())
>+        self.setProperty('try-codebase', 'internal')
>+        self.setProperty('platform', 'mac')
>+        self.setProperty('configuration', 'Debug')
>+
>+        self.expectRemoteCommands(
>+            ExpectShell(workdir='wkdir',
>+                        command=['Tools/Scripts/check-webkit-style'],
>+                        )
>+            + ExpectShell.log('stdio', stdout='Total errors found: 0 in 6 files')
>+            + 0,
>+        )
>+        self.expectOutcome(result=SUCCESS, state_string='check-webkit-style')
>+        return self.runStep()
>+
>+    def test_failures_no_changes(self):
>+        self.setupStep(CheckStyle())
>+        self.setProperty('try-codebase', 'internal')
>+        self.setProperty('platform', 'mac')
>+        self.setProperty('configuration', 'Debug')
>+
>+        self.expectRemoteCommands(
>+            ExpectShell(workdir='wkdir',
>+                        command=['Tools/Scripts/check-webkit-style'],
>+                        )
>+            + ExpectShell.log('stdio', stdout='Total errors found: 0 in 0 files')
>+            + 2,
>+        )
>+        self.expectOutcome(result=FAILURE, state_string='check-webkit-style (failure)')
>+        return self.runStep()
>+
>+
>+if __name__ == '__main__':
>+    unittest.main()

Flags:

lforschler: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 186955: 343413 | 343443