Commit 972ae12f authored by eseidel@chromium.org's avatar eseidel@chromium.org

Teach webkit-patch update-flaky-tests how to upload a change

I accidentally already landed part of this support
in an earlier change, but this makes --upload actually
work.  You can see an example change here:
https://codereview.chromium.org/308743011/
(which again accidentally includes this change).

This also adds a hack-and-slash version of sheriff-calendar
code which I and Eric Boren originally wrote for the AutoRollBot
but that lives in a different repository:
https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/tools/blink_roller/auto_roll.py

This combined with a cron job should make
it easy for gardeners to keep FlakyTests up to date.

Looking at 308743011, we clearly need to address some
of this marauding flakiness as FlakyTests should be
a very stable file.  Right now it is changing by the hour.

BUG=242366

Review URL: https://codereview.chromium.org/307183002

git-svn-id: svn://svn.chromium.org/blink/trunk@175228 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 1ceee695
...@@ -107,3 +107,6 @@ class MockSCM(object): ...@@ -107,3 +107,6 @@ class MockSCM(object):
def move(self, origin, destination): def move(self, origin, destination):
if self._filesystem: if self._filesystem:
self._filesystem.move(self.absolute_path(origin), self.absolute_path(destination)) self._filesystem.move(self.absolute_path(origin), self.absolute_path(destination))
def changed_files(self):
return []
# Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import re
import urllib2
# This is based on code from:
# https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/tools/blink_roller/auto_roll.py
# Ideally we should share code between these.
# FIXME: This probably belongs in config.py?
BLINK_SHERIFF_URL = (
'http://build.chromium.org/p/chromium.webkit/sheriff_webkit.js')
# Does not support unicode or special characters.
VALID_EMAIL_REGEXP = re.compile(r'^[A-Za-z0-9\.&\'\+-/=_]+@[A-Za-z0-9\.-]+$')
def _complete_email(name):
"""If the name does not include '@', append '@chromium.org'."""
if '@' not in name:
return name + '@chromium.org'
return name
def _names_from_sheriff_js(sheriff_js):
match = re.match(r'document.write\(\'(.*)\'\)', sheriff_js)
emails_string = match.group(1)
# Detect 'none (channel is sheriff)' text and ignore it.
if 'channel is sheriff' in emails_string.lower():
return []
return map(str.strip, emails_string.split(','))
def _email_is_valid(email):
"""Determines whether the given email address is valid."""
return VALID_EMAIL_REGEXP.match(email) is not None
def _filter_emails(emails):
"""Returns the given list with any invalid email addresses removed."""
rv = []
for email in emails:
if _email_is_valid(email):
rv.append(email)
else:
print 'WARNING: Not including %s (invalid email address)' % email
return rv
def _emails_from_url(sheriff_url):
sheriff_js = urllib2.urlopen(sheriff_url).read()
return map(_complete_email, _names_from_sheriff_js(sheriff_js))
def current_gardener_emails():
return _emails_from_url(BLINK_SHERIFF_URL)
# Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
# This is based on code from:
# https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/tools/blink_roller/auto_roll_test.py
# Ideally we should share code between these.
from webkitpy.common.system.outputcapture import OutputCaptureTestCaseBase
import sheriff_calendar as calendar
class SheriffCalendarTest(OutputCaptureTestCaseBase):
def test_complete_email(self):
expected_emails = ['foo@chromium.org', 'bar@google.com', 'baz@chromium.org']
names = ['foo', 'bar@google.com', 'baz']
self.assertEqual(map(calendar._complete_email, names), expected_emails)
def test_emails(self):
expected_emails = ['foo@bar.com', 'baz@baz.com']
calendar._emails_from_url = lambda urls: expected_emails
self.assertEqual(calendar.current_gardener_emails(), expected_emails)
def _assert_parse(self, js_string, expected_emails):
self.assertEqual(calendar._names_from_sheriff_js(js_string), expected_emails)
def test_names_from_sheriff_js(self):
self._assert_parse('document.write(\'none (channel is sheriff)\')', [])
self._assert_parse('document.write(\'foo, bar\')', ['foo', 'bar'])
def test_email_regexp(self):
self.assertTrue(calendar._email_is_valid('somebody@example.com'))
self.assertTrue(calendar._email_is_valid('somebody@example.domain.com'))
self.assertTrue(calendar._email_is_valid('somebody@example-domain.com'))
self.assertTrue(calendar._email_is_valid('some.body@example.com'))
self.assertTrue(calendar._email_is_valid('some_body@example.com'))
self.assertTrue(calendar._email_is_valid('some+body@example.com'))
self.assertTrue(calendar._email_is_valid('some+body@com'))
self.assertTrue(calendar._email_is_valid('some/body@example.com'))
# These are valid according to the standard, but not supported here.
self.assertFalse(calendar._email_is_valid('some~body@example.com'))
self.assertFalse(calendar._email_is_valid('some!body@example.com'))
self.assertFalse(calendar._email_is_valid('some?body@example.com'))
self.assertFalse(calendar._email_is_valid('some" "body@example.com'))
self.assertFalse(calendar._email_is_valid('"{somebody}"@example.com'))
# Bogus.
self.assertFalse(calendar._email_is_valid('rm -rf /#@example.com'))
self.assertFalse(calendar._email_is_valid('some body@example.com'))
self.assertFalse(calendar._email_is_valid('[some body]@example.com'))
def test_filter_emails(self):
input_emails = ['foo@bar.com', 'baz@baz.com', 'bogus email @ !!!']
expected_emails = ['foo@bar.com', 'baz@baz.com']
self.assertEquals(calendar._filter_emails(input_emails), expected_emails)
self.assertStdout('WARNING: Not including bogus email @ !!! (invalid email address)\n')
...@@ -26,10 +26,12 @@ ...@@ -26,10 +26,12 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
import os
import optparse import optparse
from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand
from webkitpy.layout_tests.layout_package.bot_test_expectations import BotTestExpectationsFactory from webkitpy.layout_tests.layout_package.bot_test_expectations import BotTestExpectationsFactory
from webkitpy.layout_tests.models.test_expectations import TestExpectationParser, TestExpectationsModel, TestExpectations from webkitpy.layout_tests.models.test_expectations import TestExpectationParser, TestExpectationsModel, TestExpectations
from webkitpy.common.net import sheriff_calendar
class FlakyTests(AbstractDeclarativeCommand): class FlakyTests(AbstractDeclarativeCommand):
...@@ -37,46 +39,85 @@ class FlakyTests(AbstractDeclarativeCommand): ...@@ -37,46 +39,85 @@ class FlakyTests(AbstractDeclarativeCommand):
help_text = "Update FlakyTests file from the flakiness dashboard" help_text = "Update FlakyTests file from the flakiness dashboard"
show_in_main_help = True show_in_main_help = True
ALWAYS_CC = [
'ojan@chromium.org',
'dpranke@chromium.org',
'eseidel@chromium.org',
]
def __init__(self): def __init__(self):
options = [ options = [
optparse.make_option('--upload', action='store_true', optparse.make_option('--upload', action='store_true',
help='upload the changed FlakyTest file for review'), help='upload the changed FlakyTest file for review'),
optparse.make_option('--reviewers', action='store',
help='comma-separated list of reviewers, defaults to blink gardeners'),
] ]
AbstractDeclarativeCommand.__init__(self, options=options) AbstractDeclarativeCommand.__init__(self, options=options)
# This is sorta silly, but allows for unit testing:
self.expectations_factory = BotTestExpectationsFactory
def execute(self, options, args, tool): def _collect_expectation_lines(self, port_names, factory):
port = tool.port_factory.get()
model = TestExpectationsModel() model = TestExpectationsModel()
for port_name in tool.port_factory.all_port_names(): for port_name in port_names:
expectations = BotTestExpectationsFactory().expectations_for_port(port_name) expectations = factory.expectations_for_port(port_name)
for line in expectations.expectation_lines(only_ignore_very_flaky=True): for line in expectations.expectation_lines(only_ignore_very_flaky=True):
model.add_expectation_line(line) model.add_expectation_line(line)
# FIXME: We need an official API to get all the test names or all test lines. # FIXME: We need an official API to get all the test names or all test lines.
lines = model._test_to_expectation_line.values() return model._test_to_expectation_line.values()
def _commit_and_upload(self, tool, options):
files = tool.scm().changed_files()
flaky_tests_path = 'LayoutTests/FlakyTests'
if flaky_tests_path not in files:
print "%s is not changed, not uploading." % flaky_tests_path
return 0
if options.reviewers:
# FIXME: Could validate these as emails. sheriff_calendar has some code for that.
reviewer_emails = options.reviewers.split(',')
else:
reviewer_emails = sheriff_calendar.current_gardener_emails()
if not reviewer_emails:
print "No gardener, and --reviewers not specified, not bothering."
return 1
commit_message = """Update FlakyTests to match current flakiness dashboard results
Automatically generated using:
webkit-patch update-flaky-tests
R=%s
""" % ','.join(reviewer_emails)
git_cmd = ['git', 'commit', '-m', commit_message,
tool.filesystem.join(tool.scm().checkout_root, flaky_tests_path)]
tool.executive.run_and_throw_if_fail(git_cmd)
# FIXME: There must be a cleaner way to avoid the editor!
# Silence the editor.
os.environ['EDITOR'] = 'true'
git_cmd = ['git', 'cl', 'upload', '--send-mail',
'--cc', ','.join(self.ALWAYS_CC)]
tool.executive.run_and_throw_if_fail(git_cmd)
def execute(self, options, args, tool):
port = tool.port_factory.get()
port_names = tool.port_factory.all_port_names()
factory = self.expectations_factory()
lines = self._collect_expectation_lines(port_names, factory)
lines.sort(key=lambda line: line.path) lines.sort(key=lambda line: line.path)
# Skip any tests which are mentioned in the dashboard but not in our checkout: # Skip any tests which are mentioned in the dashboard but not in our checkout:
fs = tool.filesystem fs = tool.filesystem
lines = filter(lambda line: fs.exists(fs.join(port.layout_tests_dir(), line.path)), lines) lines = filter(lambda line: fs.exists(fs.join(port.layout_tests_dir(), line.path)), lines)
flaky_tests_path = fs.join(port.layout_tests_dir(), 'FlakyTests')
# Note: This includes all flaky tests from the dashboard, even ones mentioned # Note: This includes all flaky tests from the dashboard, even ones mentioned
# in existing TestExpectations. We could certainly load existing TestExpecations # in existing TestExpectations. We could certainly load existing TestExpecations
# and filter accordingly, or update existing TestExpectations instead of FlakyTests. # and filter accordingly, or update existing TestExpectations instead of FlakyTests.
with open(flaky_tests_path, 'w') as flake_file: flaky_tests_path = fs.join(port.layout_tests_dir(), 'FlakyTests')
flake_file.write(TestExpectations.list_to_string(lines)) fs.write_text_file(flaky_tests_path, TestExpectations.list_to_string(lines))
print "Updated %s" % flaky_tests_path
if not options.upload:
return 0
files = tool.scm().changed_files()
flaky_tests_path = 'LayoutTests/FlakyTests'
if flaky_tests_path not in files:
print "%s is not changed, not uploading." % flaky_tests_path
return 0
commit_message = "Update FlakyTests" if options.upload:
git_cmd = ['git', 'commit', '-m', commit_message, flaky_tests_path] return self._commit_and_upload(tool, options)
tool.executive.run_command(git_cmd)
git_cmd = ['git', 'cl', 'upload', '--use-commit-queue', '--send-mail']
tool.executive.run_command(git_cmd)
# If there are changes to git, upload.
# Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
from webkitpy.tool.commands.commandtest import CommandsTest
from webkitpy.tool.mocktool import MockTool, MockOptions
import flakytests
class FakeBotTestExpectations(object):
def expectation_lines(self, only_ignore_very_flaky=False):
return []
class FakeBotTestExpectationsFactory(object):
def expectations_for_port(self, port_name):
return FakeBotTestExpectations()
class FlakyTestsTest(CommandsTest):
def test_simple(self):
command = flakytests.FlakyTests()
factory = FakeBotTestExpectationsFactory()
lines = command._collect_expectation_lines(['foo'], factory)
self.assertEqual(lines, [])
def test_integration(self):
command = flakytests.FlakyTests()
command.expectations_factory = FakeBotTestExpectationsFactory
options = MockOptions(upload=True)
expected_stdout = """Updated /mock-checkout/third_party/WebKit/LayoutTests/FlakyTests
LayoutTests/FlakyTests is not changed, not uploading.
"""
self.assert_execute_outputs(command, options=options, expected_stdout=expected_stdout)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment