In update-w3c-test-expectations, handle failure to fetch results. (issue 2277303003 by qyearsley@chromium.org)

[qyea...@chromium.org]

Reviewers: Dirk Pranke
CL: https://codereview.chromium.org/2277303003/

Description:
In update-w3c-test-expectations, handle failure to fetch results.

Specifically, when no layout tests are fetched for a particular build,
log a warning and continue instead of raising an exception and aborting.

I noticed this error when running the script on my workstation.

This CL also adds several TODO notes.

BUG=629275

Base URL: https://chromium.googlesource.com/chromium/src.git@master

Affected files (+84, -4 lines):
M third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot_mock.py
M third_party/WebKit/Tools/Scripts/webkitpy/common/net/layouttestresults.py
M third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
M third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py


Index: third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot_mock.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot_mock.py b/third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot_mock.py
index bd4bdd8b9ebdd30d9936117331c11e89a635be8c..2ffdcfc8d029709f58d4ba8dce0e2a9ee75461c6 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot_mock.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot_mock.py
@@ -31,6 +31,9 @@ from webkitpy.common.net.layouttestresults import LayoutTestResults
from webkitpy.common.net.layouttestresults_unittest import LayoutTestResultsTest


+# TODO(qyearsley): Instead of canned results from another module, other unit
+# tests may be a little easier to understand if this returned None by default
+# when there are no canned results to return.
class MockBuildBot(BuildBot):

def __init__(self):
Index: third_party/WebKit/Tools/Scripts/webkitpy/common/net/layouttestresults.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/common/net/layouttestresults.py b/third_party/WebKit/Tools/Scripts/webkitpy/common/net/layouttestresults.py
index 4c6e324759b1b449f4751d49ce0c42e613bcc07a..50a3dd9c9eb1432365c27c1fd0d1e58bcdd049e3 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/common/net/layouttestresults.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/common/net/layouttestresults.py
@@ -51,9 +51,16 @@ class LayoutTestResult(object):
return 'PASS' in self.actual_results()

def did_run_as_expected(self):
+ # TODO(qyearsley): For correctness, this should be:
+ # return not self._result_dict.get('is_unexpected', False)
+ # (Right now for expected results it's not added to the result dict
+ # but in theory it could be present.)
return 'is_unexpected' not in self._result_dict

def is_missing_image(self):
+ # TODO(qyearsley): Change this to:
+ # self._result_dict.get('is_missing_image', False)
+ # The following method should likewise be changed.
return 'is_missing_image' in self._result_dict

def is_missing_text(self):
Index: third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
index ab69b282d2ddef7c4fc5387e83e3829cad6572ae..f00f016497ba194efe3d59ca1d53c19d1791ef33 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
@@ -8,14 +8,20 @@ based on layout test failures in try jobs.
This script is used as part of the w3c test auto-import process.
"""

+import logging
+
from webkitpy.common.net.rietveld import Rietveld
-from webkitpy.common.net.buildbot import BuildBot
+from webkitpy.common.net.buildbot import BuildBot, Build
from webkitpy.common.net.git_cl import GitCL
from webkitpy.common.webkit_finder import WebKitFinder
from webkitpy.w3c.test_parser import TestParser

+_log = logging.getLogger(__name__)
+

def main(host):
+ # TODO(qyearsley): Add a "main" function to W3CExpectationsLineAdder
+ # and move most or all of this logic in there.
host.initialize_scm()
port = host.port_factory.get()
expectations_file = port.path_to_generic_test_expectations_file()
@@ -72,7 +78,11 @@ class W3CExpectationsLineAdder(object):
Collects the builder name, platform and a list of tests that did not
run as expected.

+ TODO(qyearsley): Rather than taking a BuildBot object, this should use
+ the Host's BuildBot object.
+
Args:
+ buildbot: A BuildBot object.
builder: A Builder object.
build: A Build object.

@@ -84,9 +94,13 @@ class W3CExpectationsLineAdder(object):
'bug': 'crbug.com/11111'
}
}
+ If there are no failing results or no results could be fetched,
+ this will return an empty dict.
"""
- results_url = buildbot.results_url(builder_name, build_number)
- layout_test_results = buildbot.fetch_layout_test_results(results_url)
+ layout_test_results = buildbot.fetch_results(Build(builder_name, build_number))
+ if layout_test_results is None:
+ _log.warning('No results for builder %s, build %s', builder_name, build_number)
+ return {}
platform = self._host.builders.port_name_for_builder_name(builder_name)
result_list = layout_test_results.didnt_run_as_expected_results()
failing_results_dict = self._generate_results_dict(platform, result_list)
Index: third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py
index 018ec8f571be94bb09a718fce5b1bc2ac9d503f4..77fcd0c7161665c87b414261d7d45fd1076d8d45 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py
@@ -5,8 +5,11 @@
import unittest

from webkitpy.common.host_mock import MockHost
+from webkitpy.common.net.buildbot import Build
+from webkitpy.common.net.buildbot_mock import MockBuildBot
+from webkitpy.common.net.layouttestresults import LayoutTestResult, LayoutTestResults
from webkitpy.common.webkit_finder import WebKitFinder
-from webkitpy.common.net.layouttestresults import LayoutTestResult
+from webkitpy.layout_tests.builder_list import BuilderList
from webkitpy.w3c.update_w3c_test_expectations import W3CExpectationsLineAdder


@@ -36,6 +39,59 @@ class UpdateW3CTestExpectationsTest(unittest.TestCase, W3CExpectationsLineAdder)
'one': {'expected': 'FAIL', 'actual': 'TIMEOUT', 'bug': 'crbug.com/626703'}
}
}
+ self.host.builders = BuilderList({
+ 'mac': {'port_name': 'test-mac'},
+ })
+
+ def tearDown(self):
+ self.host = None
+
+ def test_get_failing_results_dict_only_passing_results(self):
+ self.host.buildbot.set_results(Build('mac', 123), LayoutTestResults({
+ 'tests': {
+ 'fake': {
+ 'test.html': {
+ 'passing-test.html': {
+ 'expected': 'PASS',
+ 'actual': 'PASS',
+ },
+ },
+ },
+ },
+ }))
+ line_adder = W3CExpectationsLineAdder(self.host)
+ self.assertEqual(line_adder.get_failing_results_dict(self.host.buildbot, 'mac', 123), {})
+
+ def test_get_failing_results_dict_no_results(self):
+ self.host.buildbot = MockBuildBot()
+ self.host.buildbot.set_results(Build('mac', 123), None)
+ line_adder = W3CExpectationsLineAdder(self.host)
+ self.assertEqual(line_adder.get_failing_results_dict(self.host.buildbot, 'mac', 123), {})
+
+ def test_get_failing_results_dict_some_failing_results(self):
+ self.host.buildbot.set_results(Build('mac', 123), LayoutTestResults({
+ 'tests': {
+ 'fake': {
+ 'test.html': {
+ 'failing-test.html': {
+ 'expected': 'PASS',
+ 'actual': 'IMAGE',
+ 'is_unexpected': True,
+ },
+ },
+ },
+ },
+ }))
+ line_adder = W3CExpectationsLineAdder(self.host)
+ self.assertEqual(line_adder.get_failing_results_dict(self.host.buildbot, 'mac', 123), {
+ 'fake/test.html/failing-test.html': {
+ 'Mac': {
+ 'actual': 'IMAGE',
+ 'expected': 'PASS',
+ 'bug': 'crbug.com/626703',
+ },
+ },
+ })

def test_merge_same_valued_keys(self):
self.assertEqual(

[qyea...@chromium.org]

https://codereview.chromium.org/2277303003/diff/1/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py
File
third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py
(right):

https://codereview.chromium.org/2277303003/diff/1/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py#newcode16
third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py:16:
class UpdateW3CTestExpectationsTest(unittest.TestCase,
W3CExpectationsLineAdder):
Another TODO which I plan to do as an immediate follow-up to this CL:

Make this class not subclass W3CExpectationsLineAdder.

https://codereview.chromium.org/2277303003/

[dpr...@chromium.org]

lgtm



https://codereview.chromium.org/2277303003/

[commit-bot@chromium.org via codereview.chromium.org]

CQ is trying da patch. Follow status at

https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2277303003/1


https://codereview.chromium.org/2277303003/

[commit-bot@chromium.org via codereview.chromium.org]

Committed patchset #1 (id:1)

https://codereview.chromium.org/2277303003/

[commit-bot@chromium.org via codereview.chromium.org]

Patchset 1 (id:??) landed as
https://crrev.com/0f77323cdf30b116f8055b11ef890d6a835b2044
Cr-Commit-Position: refs/heads/master@{#414497}

https://codereview.chromium.org/2277303003/